Move setSelect into the Jcrop init callback - #9270
Open
c-tonneslan wants to merge 1 commit into
Open
Conversation
Jcrop's init is async: the second argument is a callback that runs
once the plugin has wired itself up and the api object becomes 'this'.
The setSelect call sat outside that callback, so on a fresh page load
jcrop_api was still undefined and the browser threw
Uncaught TypeError: Cannot read properties of undefined (reading 'setSelect')
which left the page without the initial centred crop box. Drag-select
still worked, which is why this slipped through, but the default
selection was gone.
Closes mysociety#9196
Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relevant issue(s)
Closes #9196.
What does this do?
Moves the
jcrop_api.setSelect(...)call into the Jcrop init callback so the initial centred crop box appears again on the profile photo crop page.Why was this needed?
Jcrop's init is async, so on a fresh load
jcrop_apiwas still undefined whensetSelectran on the next line, throwingUncaught TypeError: Cannot read properties of undefined (reading 'setSelect'). Drag-select still worked, which is why the bug was minor in practice, but the default selection was missing.Implementation notes
Just moved the single line inside the callback that already assigns
jcrop_api = this.Screenshots
n/a, console-only
Notes to reviewer
Added a CHANGES.md entry under develop. Happy to drop the author attribution if you prefer a different format.