Fix #16703. Prevent disabled buttons set to active on click. #16707
Conversation
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: e475549 (Please note that this is a fully automated comment.) |
|
Any way to kick off savage build test again? Looking at logs and it doesn't seem like the code in the pull request is the cause of the failed test. Looks like the tunnel to sauce labs was closed in the middle of testing, so a POST couldn't be done and threw an error. |
|
@twbs-savage retry |
|
Also, the addition of a new unit test is required in order to merge this. See https://github.com/twbs/bootstrap/tree/master/js/tests for some instructions. |
|
ah, okay. I'll get started on that then. |
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: e475549 (Please note that this is a fully automated comment.) |
|
Tests passed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED Commit: 9358008 (Please note that this is a fully automated comment.) |
|
The cause of #16703 seems to be that this event handler is called (it shouldn't be for disabled buttons) not that this event handler sets the class to active. |
| @@ -110,6 +110,18 @@ $(function () { | |||
| assert.ok($btn.hasClass('active'), 'btn has class active') | |||
| }) | |||
|
|
|||
| QUnit.test('should prevent toggle active when disabled btn children are clicked', function (assert) { | |||
| assert.expect(2) | |||
| var $btn = $('<button class="btn disabled" data-toggle="button">mdo</button>') | |||
There was a problem hiding this comment.
Could we please use a checkbox/radio button group here, since that's what the issue is about?
There was a problem hiding this comment.
you are correct. I will update this test.
Though I also have am not sure if this is the correct way to solve this PR. I will continue discussion in #16703.
|
We should also add a disabled button to each of the checkbox/radio examples in https://github.com/twbs/bootstrap/blob/master/docs/_includes/js/buttons.html for good measure. |
|
Improved tests to include label > input format as well as singular I'm unable to get jekyll to install correctly right now on my machine, so I'm unable to properly update documentation. I will work on twbs/bootlint#292 next to help improve linter for disabled attributes and classes together on an element. |
|
Tests passed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED Commit: 97eb7f6 (Please note that this is a fully automated comment.) |
|
Bootstrap 3 is no longer being officially developed or supported. All work has moved onto our next major release, v4. As such, this issue or pull request is being closed as a "won't fix." For additional help and support, we recommend utilizing our community resources. Thanks for your understanding, and see you on the other side of v4! <3, |
Resolves #16703. Conditional added in click event to check if the target button has the class of 'disabled'. If it does not have 'disabled' it calls the 'toggle' function. Otherwise, if button has 'disabled' class, toggle does not run.