Ensure the bbox input is in the correct form #322
Merged
Conversation
This file contains 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
markstuart
commented
Nov 13, 2023
| }); | ||
| // Add necessary field to the search form if not already created | ||
| if ($("#" + bbox_input_id).length === 0) { | ||
| $('<input type="hidden" />').attr({'id': bbox_input_id, 'name': bbox_input_id}).appendTo(form); |
There was a problem hiding this comment.
Since the form was actually 2 forms in our case due to the jquery lookup by class, the appendTo(form) was adding the hidden input to both forms. Then the later lookup and value setting ($('#ext_bbox').val) only found the first input on the page with that ID and set the val on that. Not the right form in this case.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In our particular customisation of CKAN we have 2 forms with the class of 'search-form', and this class is used to ensure styling consistency.
With the way this was set up before, both forms were getting a ext_bbox input injected into them, but then only the first one on the page was getting the bbox value set on it. This unfortunately was not the search form that actually needed the bbox value.
With these changes, I'm trying to scope the ext_bbox input to only the actual dataset search form.
In general, I think actions like this should try to target elements by ID's where possible, rather than using classes, which should be used more for defining styling to be applied to that class, but unsure what the CKAN team preferences are on this.