Prevent .selectize() updates from destroying .data() and event listeners
#3923
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
This reverts commit eddc304.
|
Talked though this in person with @gadenbuie |
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.
This PR reverts #3890 (which closed #3889) and instead reverts a recent change made to selectize.js to fix that same issue. As this comment highlights, that selectize.js change removes
.data()and event listeners attached to the<select>tag (or any of it's children), which is problematic for ourselectInputbinding since bindings are storied/acquired via.data()(and it also relies onchangeevents to set values back to the server).#3890 originally worked around the issue by unbinding before and binding after any update occurs (akin to how dynamic UI works), but after #3904, to do that properly would further complicate an already complex
selectInputbinding. Also, once I came back to look more deeply at this, it felt like this destruction action has a pretty good chance of breaking 3rd party code. And, since it seems the selectize.js change is solely about performance, it seems best to just revert it (it feels like a future release of selectize.js might have to do something similar -- or at least change that section in such a way that we'll get alerted by it).