-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Watch disabled property of option/optgroup elements #4537
Watch disabled property of option/optgroup elements #4537
Conversation
It looks like this doesn't cover the case when an already selected option is disabled. |
src/js/select2/data/select.js
Outdated
@@ -203,6 +203,14 @@ define([ | |||
data = $.data($option[0], 'data'); | |||
|
|||
if (data != null) { | |||
// always update the value of `disabled` property of the option element | |||
data.disabled = $option.prop('disabled'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally the solution to this should be very similar, if not the same as, the solution to us handling the cached selected
property on elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We now have a PR that is similar to the mentioned commit: #4818
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@denilsonsa I don't think that #4818 handles optgroup
elements. You'll notice that @letrunghieu's solution does a bit more. However, neither of these PRs have any tests yet 😦
@letrunghieu are you still interested in working on this? I'd like to merge it, but a few things need to be addressed:
Thanks! |
Hi @alexweissman I am sure interested in working on this issue :) I'll review the code. |
Hi @kevin-brown , @alexweissman , could you review this PR again please? |
Looking good. Can we avoid the |
@alexweissman So how to we make sure that the type of the |
Is |
Should be, as it is part of JavaScript since (basically) forever: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean (Hi! I haven't contributed to this project yet; I just wanted to share this relevant link!) |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This pull request includes a
The following changes were made
disabled
property onoptgroup
elementsdisabled
property onoptgroup
andoption
elements so that changes of this property are respected.Fix #3347, #2306
Demo: https://jsbin.com/moduda/3/edit?html,js,output