Button: remove ui-state-active on button disable. Fixed #9602 - ui-state-active remains after disable#1151
Button: remove ui-state-active on button disable. Fixed #9602 - ui-state-active remains after disable#1151robotdan wants to merge 8 commits into
Conversation
…ate-active class remains after disabling a button
|
Added testcase from bug report to use this branch bug_9602. |
|
There's one problem with this approach that I didn't think about earlier. We can't remove |
|
I'll fix that and add some additional test cases to cover those scenarios. |
|
Checked in some additional code to only remove the These two test cases should now show the same behavior for a checkbox
And the test case for issue 9602 |
There was a problem hiding this comment.
We also support turning <a> elements and even generic elements like <div>s into buttons. It might be better to flip the logic so you explicitly check for radio buttons and checkboxes.
There was a problem hiding this comment.
That is a good point, I did prototype that logic initially but I had a few problems with it. But that is probably a better strategy, I'll take another look at it.
…ate-active remains after disable Refactor unit tests after flipping logic to specifically look for radio and checkbox, also added div, and a elements for unit test.
|
I updated the logic as suggested to specifically check for I updated these two jsfiddle test cases to include more button options.
|
|
👍 |
There was a problem hiding this comment.
:checkbox and :radio are discouraged; use [type=...] selectors.
There was a problem hiding this comment.
Store the result of this check in a variable since you need to know this later too.
|
Please move the test to |
…ate-active class remains after disabling a button Few small formatting changes based upon comments. Moved unit test to button_options.js from button_methods.js.
|
Resolved comments from @scottgonzalez and moved unit test to |
|
A few minor issues, but other than that this looks good to go. Thanks. |
…ate-active class remains after disabling a button use .button( “widget” ) instead of checking for radio or checkbox.
I saw in the style guidelines to use nodeName over tagName. https://contribute.jquery.org/style-guide/js/#dom-node-rules
|
@scottgonzalez there were updates, but no comment, after your last comment - can you review this again? |
There was a problem hiding this comment.
Everything is indented too much.
|
Thanks, I squashed everything and landed this. |
Button: remove ui-state-active on button disable. Fixed #9602 - ui-state-active class remains after disabling a button