Skip to content

Commit

Permalink
Ensure button plugin sets/removes active class correctly on page load (
Browse files Browse the repository at this point in the history
…#28952)

* Ensure correct active class is set on button toggles/checkboxes/radios on page load

Sanity check, ensures that the UI visually matches the actual values/states of controls. Also ensures that if any autocomplete/autofill happened, this is visually accounted for
by having the correct class set.

Includes unit tests (and `autocomplete` has been removed from these as it's no longer necessary)

* Remove now unnecessary autocomplete attribute

As the attribute was there to force/ensure that the visual presentation matched the state, and this is now taken care of programmatically, there's no need to unnecessarily suppress autocomplete...let them autocomplete if they want to...
  • Loading branch information
patrickhlauke committed Jun 24, 2019
1 parent 7a7a2b9 commit e38d9fa
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 30 deletions.
44 changes: 37 additions & 7 deletions js/src/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,20 @@ const ClassName = {
}

const Selector = {
DATA_TOGGLE_CARROT : '[data-toggle^="button"]',
DATA_TOGGLE : '[data-toggle="buttons"]',
INPUT : 'input:not([type="hidden"])',
ACTIVE : '.active',
BUTTON : '.btn'
DATA_TOGGLE_CARROT : '[data-toggle^="button"]',
DATA_TOGGLES : '[data-toggle="buttons"]',
DATA_TOGGLE : '[data-toggle="button"]',
DATA_TOGGLES_BUTTONS : '[data-toggle="buttons"] .btn',
INPUT : 'input:not([type="hidden"])',
ACTIVE : '.active',
BUTTON : '.btn'
}

const Event = {
CLICK_DATA_API : `click${EVENT_KEY}${DATA_API_KEY}`,
FOCUS_BLUR_DATA_API : `focus${EVENT_KEY}${DATA_API_KEY} ` +
`blur${EVENT_KEY}${DATA_API_KEY}`
`blur${EVENT_KEY}${DATA_API_KEY}`,
LOAD_DATA_API : `load${EVENT_KEY}${DATA_API_KEY}`
}

/**
Expand All @@ -63,7 +66,7 @@ class Button {
let triggerChangeEvent = true
let addAriaPressed = true
const rootElement = $(this._element).closest(
Selector.DATA_TOGGLE
Selector.DATA_TOGGLES
)[0]

if (rootElement) {
Expand Down Expand Up @@ -167,6 +170,33 @@ $(document)
$(button).toggleClass(ClassName.FOCUS, /^focus(in)?$/.test(event.type))
})

$(window).on(Event.LOAD_DATA_API, () => {
// ensure correct active class is set to match the controls' actual values/states

// find all checkboxes/readio buttons inside data-toggle groups
let buttons = [].slice.call(document.querySelectorAll(Selector.DATA_TOGGLES_BUTTONS))
for (let i = 0, len = buttons.length; i < len; i++) {
const button = buttons[i]
const input = button.querySelector(Selector.INPUT)
if (input.checked || input.hasAttribute('checked')) {
button.classList.add(ClassName.ACTIVE)
} else {
button.classList.remove(ClassName.ACTIVE)
}
}

// find all button toggles
buttons = [].slice.call(document.querySelectorAll(Selector.DATA_TOGGLE))
for (let i = 0, len = buttons.length; i < len; i++) {
const button = buttons[i]
if (button.getAttribute('aria-pressed') === 'true') {
button.classList.add(ClassName.ACTIVE)
} else {
button.classList.remove(ClassName.ACTIVE)
}
}
})

/**
* ------------------------------------------------------------------------
* jQuery
Expand Down
80 changes: 70 additions & 10 deletions js/tests/unit/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,73 @@ $(function () {
assert.strictEqual($btn.attr('aria-pressed'), 'true', 'btn aria-pressed state is true')
})

QUnit.test('should assign active class on page load to buttons with aria-pressed="true"', function (assert) {
assert.expect(1)
var done = assert.async()
var $btn = $('<button class="btn" data-toggle="button" aria-pressed="true">mdo</button>')
$btn.appendTo('#qunit-fixture')
$(window).trigger($.Event('load'))
setTimeout(function () {
assert.ok($btn.hasClass('active'), 'button with aria-pressed="true" has been given class active')
done()
}, 5)
})

QUnit.test('should assign active class on page load to button checkbox with checked attribute', function (assert) {
assert.expect(1)
var done = assert.async()
var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
'<label class="btn btn-primary">' +
'<input type="checkbox" id="radio" checked> Checkbox' +
'</label>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')
var $btn = $group.children().eq(0)

$(window).trigger($.Event('load'))
setTimeout(function () {
assert.ok($btn.hasClass('active'), 'checked checkbox button has been given class active')
done()
}, 5)
})

QUnit.test('should remove active class on page load from buttons without aria-pressed="true"', function (assert) {
assert.expect(1)
var done = assert.async()
var $btn = $('<button class="btn active" data-toggle="button" aria-pressed="false">mdo</button>')
$btn.appendTo('#qunit-fixture')
$(window).trigger($.Event('load'))
setTimeout(function () {
assert.ok(!$btn.hasClass('active'), 'button without aria-pressed="true" has had active class removed')
done()
}, 5)
})

QUnit.test('should remove active class on page load from button checkbox without checked attribute', function (assert) {
assert.expect(1)
var done = assert.async()
var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
'<label class="btn btn-primary active">' +
'<input type="checkbox" id="radio"> Checkbox' +
'</label>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')
var $btn = $group.children().eq(0)

$(window).trigger($.Event('load'))
setTimeout(function () {
assert.ok(!$btn.hasClass('active'), 'unchecked checkbox button has had active class removed')
done()
}, 5)
})

QUnit.test('should trigger input change event when toggled button has input field', function (assert) {
assert.expect(1)
var done = assert.async()

var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
'<label class="btn btn-primary">' +
'<input type="radio" id="radio" autocomplete="off">Radio' +
'<input type="radio" id="radio">Radio' +
'</label>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')
Expand Down Expand Up @@ -158,8 +218,8 @@ $(function () {
QUnit.test('should not add aria-pressed on labels for radio/checkbox inputs in a data-toggle="buttons" group', function (assert) {
assert.expect(2)
var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
'<label class="btn btn-primary"><input type="checkbox" autocomplete="off"> Checkbox</label>' +
'<label class="btn btn-primary"><input type="radio" name="options" autocomplete="off"> Radio</label>' +
'<label class="btn btn-primary"><input type="checkbox"> Checkbox</label>' +
'<label class="btn btn-primary"><input type="radio" name="options"> Radio</label>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')

Expand All @@ -177,7 +237,7 @@ $(function () {
assert.expect(4)
var groupHTML = '<div class="btn-group disabled" data-toggle="buttons" aria-disabled="true" disabled>' +
'<label class="btn btn-danger disabled">' +
'<input type="checkbox" aria-disabled="true" autocomplete="off" disabled>' +
'<input type="checkbox" aria-disabled="true" disabled>' +
'</label>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')
Expand All @@ -196,7 +256,7 @@ $(function () {
assert.expect(4)
var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
'<label class="btn btn-danger">' +
'<input type="checkbox" autocomplete="off" disabled>' +
'<input type="checkbox" disabled>' +
'</label>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')
Expand All @@ -215,7 +275,7 @@ $(function () {
assert.expect(4)
var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
'<label class="btn">' +
'<input type="checkbox" autocomplete="off">' +
'<input type="checkbox">' +
'</label>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')
Expand All @@ -234,7 +294,7 @@ $(function () {
assert.expect(4)
var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
'<div class="btn">' +
'<input type="checkbox" autocomplete="off" aria-label="Check">' +
'<input type="checkbox" aria-label="Check">' +
'</div>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')
Expand All @@ -253,7 +313,7 @@ $(function () {
assert.expect(4)
var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
'<label class="btn">' +
'<input type="checkbox" autocomplete="off">' +
'<input type="checkbox">' +
'</label>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')
Expand All @@ -272,7 +332,7 @@ $(function () {
assert.expect(2)
var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
'<label class="btn">' +
'<input type="hidden" autocomplete="off">' +
'<input type="hidden">' +
'</label>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')
Expand All @@ -289,7 +349,7 @@ $(function () {
assert.expect(2)
var groupHTML = '<div class="btn-group" data-toggle="buttons">' +
'<label class="btn">' +
'<input type="text" autocomplete="off">' +
'<input type="text">' +
'</label>' +
'</div>'
var $group = $(groupHTML).appendTo('#qunit-fixture')
Expand Down
14 changes: 7 additions & 7 deletions js/tests/visual/button.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<div class="container">
<h1>Button <small>Bootstrap Visual Test</small></h1>

<button type="button" class="btn btn-primary" data-toggle="button" aria-pressed="false" autocomplete="off">
<button type="button" class="btn btn-primary" data-toggle="button" aria-pressed="false">
Single toggle
</button>

Expand All @@ -19,27 +19,27 @@ <h1>Button <small>Bootstrap Visual Test</small></h1>

<div class="btn-group" data-toggle="buttons">
<label class="btn btn-primary active">
<input type="checkbox" checked autocomplete="off"> Checkbox 1 (pre-checked)
<input type="checkbox" checked> Checkbox 1 (pre-checked)
</label>
<label class="btn btn-primary">
<input type="checkbox" autocomplete="off"> Checkbox 2
<input type="checkbox"> Checkbox 2
</label>
<label class="btn btn-primary">
<input type="checkbox" autocomplete="off"> Checkbox 3
<input type="checkbox"> Checkbox 3
</label>
</div>

<p>Navigate to the radio button group with the keyboard (generally, using <kbd>TAB</kbd> / <kbd>SHIFT + TAB</kbd>). If no radio button was initially set to be selected, the first/last radio button should receive focus (depending on whether you navigated "forward" to the group with <kbd>TAB</kbd> or "backwards" using <kbd>SHIFT + TAB</kbd>). If a radio button was already selected, navigating with the keyboard should set focus to that particular radio button. Only one radio button in a group should receive focus at any given time. Ensure that the selected radio button can be changed by using the <kbd></kbd> and <kbd></kbd> arrow keys. Click on one of the radio buttons with the mouse, ensure that focus was correctly set on the actual radio button, and that <kbd></kbd> and <kbd></kbd> change the selected radio button again.</p>

<div class="btn-group" data-toggle="buttons">
<label class="btn btn-primary active">
<input type="radio" name="options" id="option1" autocomplete="off" checked> Radio 1 (preselected)
<input type="radio" name="options" id="option1" checked> Radio 1 (preselected)
</label>
<label class="btn btn-primary">
<input type="radio" name="options" id="option2" autocomplete="off"> Radio 2
<input type="radio" name="options" id="option2"> Radio 2
</label>
<label class="btn btn-primary">
<input type="radio" name="options" id="option3" autocomplete="off"> Radio 3
<input type="radio" name="options" id="option3"> Radio 3
</label>
</div>
</div>
Expand Down
10 changes: 5 additions & 5 deletions site/docs/4.3/components/buttons.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ Do more with buttons. Control button states or create groups of buttons for more
Add `data-toggle="button"` to toggle a button's `active` state. If you're pre-toggling a button, you must manually add the `.active` class **and** `aria-pressed="true"` to the `<button>`.

{% capture example %}
<button type="button" class="btn btn-primary" data-toggle="button" aria-pressed="false" autocomplete="off">
<button type="button" class="btn btn-primary" data-toggle="button" aria-pressed="false">
Single toggle
</button>
{% endcapture %}
Expand All @@ -134,7 +134,7 @@ Note that pre-checked buttons require you to manually add the `.active` class to
{% capture example %}
<div class="btn-group-toggle" data-toggle="buttons">
<label class="btn btn-secondary active">
<input type="checkbox" checked autocomplete="off"> Checked
<input type="checkbox" checked> Checked
</label>
</div>
{% endcapture %}
Expand All @@ -143,13 +143,13 @@ Note that pre-checked buttons require you to manually add the `.active` class to
{% capture example %}
<div class="btn-group btn-group-toggle" data-toggle="buttons">
<label class="btn btn-secondary active">
<input type="radio" name="options" id="option1" autocomplete="off" checked> Active
<input type="radio" name="options" id="option1" checked> Active
</label>
<label class="btn btn-secondary">
<input type="radio" name="options" id="option2" autocomplete="off"> Radio
<input type="radio" name="options" id="option2"> Radio
</label>
<label class="btn btn-secondary">
<input type="radio" name="options" id="option3" autocomplete="off"> Radio
<input type="radio" name="options" id="option3"> Radio
</label>
</div>
{% endcapture %}
Expand Down
2 changes: 1 addition & 1 deletion site/docs/4.3/components/progress.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ The striped gradient can also be animated. Add `.progress-bar-animated` to `.pro
<div class="progress">
<div class="progress-bar progress-bar-striped" role="progressbar" aria-valuenow="75" aria-valuemin="0" aria-valuemax="100" style="width: 75%"></div>
</div>
<button type="button" class="btn btn-secondary bd-toggle-animated-progress" data-toggle="button" aria-pressed="false" autocomplete="off">
<button type="button" class="btn btn-secondary bd-toggle-animated-progress" data-toggle="button" aria-pressed="false">
Toggle animation
</button>
</div>
Expand Down

0 comments on commit e38d9fa

Please sign in to comment.