Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove checkbox/radio toggle from button plugin in favor of a CSS only solution #30650

Merged
merged 3 commits into from
Jun 16, 2020
+124 −318
Merged

Remove checkbox/radio toggle from button plugin in favor of a CSS only solution #30650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0b3115a
Remove checkbox/radio toggle from button plugin in favor of a CSS onl…
MartijnCuppens May 2, 2020
2dbaa05
Switch to btn-check
MartijnCuppens Jun 4, 2020
586d1e6
Clarify removal of border radii
MartijnCuppens Jun 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 5 additions & 73 deletions js/src/button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import { getjQuery } from './util/index'
import Data from './dom/data'
import EventHandler from './dom/event-handler'
import SelectorEngine from './dom/selector-engine'

/**
* ------------------------------------------------------------------------
Expand All @@ -23,18 +22,10 @@ const EVENT_KEY = `.${DATA_KEY}`
const DATA_API_KEY = '.data-api'

const CLASS_NAME_ACTIVE = 'active'
const CLASS_NAME_DISABLED = 'disabled'
const CLASS_NAME_FOCUS = 'focus'

const SELECTOR_DATA_TOGGLE_CARROT = '[data-toggle^="button"]'
const SELECTOR_DATA_TOGGLE = '[data-toggle="buttons"]'
const SELECTOR_INPUT = 'input:not([type="hidden"])'
const SELECTOR_ACTIVE = '.active'
const SELECTOR_BUTTON = '.btn'
const SELECTOR_DATA_TOGGLE = '[data-toggle="button"]'

const EVENT_CLICK_DATA_API = `click${EVENT_KEY}${DATA_API_KEY}`
const EVENT_FOCUS_DATA_API = `focus${EVENT_KEY}${DATA_API_KEY}`
const EVENT_BLUR_DATA_API = `blur${EVENT_KEY}${DATA_API_KEY}`

/**
* ------------------------------------------------------------------------
Expand All @@ -57,51 +48,8 @@ class Button {
// Public

toggle() {
let triggerChangeEvent = true
let addAriaPressed = true

const rootElement = this._element.closest(SELECTOR_DATA_TOGGLE)

if (rootElement) {
const input = SelectorEngine.findOne(SELECTOR_INPUT, this._element)

if (input && input.type === 'radio') {
if (input.checked &&
this._element.classList.contains(CLASS_NAME_ACTIVE)) {
triggerChangeEvent = false
} else {
const activeElement = SelectorEngine.findOne(SELECTOR_ACTIVE, rootElement)

if (activeElement) {
activeElement.classList.remove(CLASS_NAME_ACTIVE)
}
}

if (triggerChangeEvent) {
if (input.hasAttribute('disabled') ||
rootElement.hasAttribute('disabled') ||
input.classList.contains(CLASS_NAME_DISABLED) ||
rootElement.classList.contains(CLASS_NAME_DISABLED)) {
return
}

input.checked = !this._element.classList.contains(CLASS_NAME_ACTIVE)
EventHandler.trigger(input, 'change')
}

input.focus()
addAriaPressed = false
}
}

if (addAriaPressed) {
this._element.setAttribute('aria-pressed',
!this._element.classList.contains(CLASS_NAME_ACTIVE))
}

if (triggerChangeEvent) {
this._element.classList.toggle(CLASS_NAME_ACTIVE)
}
// Toggle class and sync the `aria-pressed` attribute with the return value of the `.toggle()` method
this._element.setAttribute('aria-pressed', this._element.classList.toggle(CLASS_NAME_ACTIVE))
}

dispose() {
Expand Down Expand Up @@ -136,10 +84,10 @@ class Button {
* ------------------------------------------------------------------------
*/

EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE_CARROT, event => {
EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE, event => {
event.preventDefault()

const button = event.target.closest(SELECTOR_BUTTON)
const button = event.target.closest(SELECTOR_DATA_TOGGLE)

let data = Data.getData(button, DATA_KEY)
if (!data) {
Expand All @@ -149,22 +97,6 @@ EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DATA_TOGGLE_CARROT, eve
data.toggle()
})

EventHandler.on(document, EVENT_FOCUS_DATA_API, SELECTOR_DATA_TOGGLE_CARROT, event => {
const button = event.target.closest(SELECTOR_BUTTON)

if (button) {
button.classList.add(CLASS_NAME_FOCUS)
}
})

EventHandler.on(document, EVENT_BLUR_DATA_API, SELECTOR_DATA_TOGGLE_CARROT, event => {
const button = event.target.closest(SELECTOR_BUTTON)

if (button) {
button.classList.remove(CLASS_NAME_FOCUS)
}
})

const $ = getjQuery()

/**
Expand Down
161 changes: 0 additions & 161 deletions js/tests/unit/button.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import Button from '../../src/button'
import EventHandler from '../../src/dom/event-handler'

/** Test helpers */
import {
getFixture,
clearFixture,
createEvent,
jQueryMock
} from '../helpers/fixture'

Expand Down Expand Up @@ -51,144 +49,6 @@ describe('Button', () => {

expect(btnTestParent.classList.contains('active')).toEqual(true)
})

it('should trigger input change event when toggled button has input field', done => {
fixtureEl.innerHTML = [
'<div class="btn-group" data-toggle="buttons">',
' <label class="btn btn-primary">',
' <input type="radio" id="radio" autocomplete="off"> Radio',
' </label>',
'</div>'
].join('')

const input = fixtureEl.querySelector('input')
const label = fixtureEl.querySelector('label')

input.addEventListener('change', () => {
expect().nothing()
done()
})

label.click()
})

it('should not trigger input change event when input already checked and button is active', () => {
fixtureEl.innerHTML = [
'<button type="button" class="btn btn-primary active" data-toggle="buttons">',
' <input type="radio" id="radio" autocomplete="off" checked> Radio',
'</button>'
].join('')

const button = fixtureEl.querySelector('button')

spyOn(EventHandler, 'trigger')

button.click()

expect(EventHandler.trigger).not.toHaveBeenCalled()
})

it('should remove active when an other radio button is clicked', () => {
fixtureEl.innerHTML = [
'<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',
' </label>',
' <label class="btn btn-secondary">',
' <input type="radio" name="options" id="option2" autocomplete="off"> Radio',
' </label>',
' <label class="btn btn-secondary">',
' <input type="radio" name="options" id="option3" autocomplete="off"> Radio',
' </label>',
'</div>'
].join('')

const option1 = fixtureEl.querySelector('#option1')
const option2 = fixtureEl.querySelector('#option2')

expect(option1.checked).toEqual(true)
expect(option1.parentElement.classList.contains('active')).toEqual(true)

const clickEvent = createEvent('click')

option2.dispatchEvent(clickEvent)

expect(option1.checked).toEqual(false)
expect(option1.parentElement.classList.contains('active')).toEqual(false)
expect(option2.checked).toEqual(true)
expect(option2.parentElement.classList.contains('active')).toEqual(true)
})

it('should do nothing if the child is not an input', () => {
fixtureEl.innerHTML = [
'<div class="btn-group btn-group-toggle" data-toggle="buttons">',
' <label class="btn btn-secondary active">',
' <span id="option1">el 1</span>',
' </label>',
' <label class="btn btn-secondary">',
' <span id="option2">el 2</span>',
' </label>',
' <label class="btn btn-secondary">',
' <span>el 3</span>',
' </label>',
'</div>'
].join('')

const option2 = fixtureEl.querySelector('#option2')
const clickEvent = createEvent('click')

option2.dispatchEvent(clickEvent)

expect().nothing()
})

it('should add focus class on focus event', () => {
fixtureEl.innerHTML = '<button class="btn" data-toggle="button"><input type="text"></button>'

const btn = fixtureEl.querySelector('.btn')
const input = fixtureEl.querySelector('input')

const focusEvent = createEvent('focus')
input.dispatchEvent(focusEvent)

expect(btn.classList.contains('focus')).toEqual(true)
})

it('should not add focus class', () => {
fixtureEl.innerHTML = '<button data-toggle="button"><input type="text"></button>'

const btn = fixtureEl.querySelector('button')
const input = fixtureEl.querySelector('input')

const focusEvent = createEvent('focus')
input.dispatchEvent(focusEvent)

expect(btn.classList.contains('focus')).toEqual(false)
})

it('should remove focus class on blur event', () => {
fixtureEl.innerHTML = '<button class="btn focus" data-toggle="button"><input type="text"></button>'

const btn = fixtureEl.querySelector('.btn')
const input = fixtureEl.querySelector('input')

const focusEvent = createEvent('blur')
input.dispatchEvent(focusEvent)

expect(btn.classList.contains('focus')).toEqual(false)
})

it('should not remove focus class on blur event', () => {
fixtureEl.innerHTML = '<button class="focus" data-toggle="button"><input type="text"></button>'

const btn = fixtureEl.querySelector('button')
const input = fixtureEl.querySelector('input')

const focusEvent = createEvent('blur')
input.dispatchEvent(focusEvent)

expect(btn.classList.contains('focus')).toEqual(true)
})
})

describe('toggle', () => {
Expand All @@ -206,27 +66,6 @@ describe('Button', () => {
expect(btnEl.getAttribute('aria-pressed')).toEqual('true')
expect(btnEl.classList.contains('active')).toEqual(true)
})

it('should handle disabled attribute on non-button elements', () => {
fixtureEl.innerHTML = [
'<div class="btn-group disabled" data-toggle="buttons" aria-disabled="true" disabled>',
' <label class="btn btn-danger disabled" aria-disabled="true" disabled>',
' <input type="checkbox" aria-disabled="true" autocomplete="off" disabled class="disabled">',
' </label>',
'</div>'
].join('')

const btnGroupEl = fixtureEl.querySelector('.btn-group')
const btnDanger = fixtureEl.querySelector('.btn-danger')
const input = fixtureEl.querySelector('input')

const button = new Button(btnGroupEl)

button.toggle()

expect(btnDanger.hasAttribute('disabled')).toEqual(true)
expect(input.checked).toEqual(false)
})
})

describe('dispose', () => {
Expand Down
50 changes: 16 additions & 34 deletions scss/_button-group.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,17 @@
> .btn {
position: relative;
flex: 1 1 auto;
}

// Bring the hover, focused, and "active" buttons to the front to overlay
// the borders properly
&:hover,
&:focus,
&:active,
&.active {
z-index: 1;
}
// Bring the hover, focused, and "active" buttons to the front to overlay
// the borders properly
> .btn-check:checked + .btn,
> .btn-check:focus + .btn,
> .btn:hover,
> .btn:focus,
> .btn:active,
> .btn.active {
z-index: 1;
}
}

Expand Down Expand Up @@ -46,7 +48,12 @@
@include border-right-radius(0);
}

> .btn:not(:first-child),
// The left radius should be 0 if the button is:
// - the "third or more" child
// - the second child and the previous element isn't `.btn-check` (making it the first child visually)
// - part of a btn-group which isn't the first child
> .btn:nth-child(n + 3),
mdo marked this conversation as resolved.
Show resolved Hide resolved
> :not(.btn-check) + .btn,
> .btn-group:not(:first-child) > .btn {
@include border-left-radius(0);
}
Expand Down Expand Up @@ -132,28 +139,3 @@
@include border-top-radius(0);
}
}


// Checkbox and radio options
//
// In order to support the browser's form validation feedback, powered by the
// `required` attribute, we have to "hide" the inputs via `clip`. We cannot use
// `display: none;` or `visibility: hidden;` as that also hides the popover.
// Simply visually hiding the inputs via `opacity` would leave them clickable in
// certain cases which is prevented by using `clip` and `pointer-events`.
// This way, we ensure a DOM element is visible to position the popover from.
//
// See https://github.com/twbs/bootstrap/pull/12794 and
// https://github.com/twbs/bootstrap/pull/14559 for more information.

.btn-group-toggle {
> .btn,
> .btn-group > .btn {
input[type="radio"],
input[type="checkbox"] {
position: absolute;
clip: rect(0, 0, 0, 0);
pointer-events: none;
}
}
}
9 changes: 5 additions & 4 deletions scss/_buttons.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@
text-decoration: if($link-hover-decoration == underline, none, null);
}

&:focus,
&.focus {
.btn-check:focus + &,
&:focus {
outline: 0;
box-shadow: $btn-focus-box-shadow;
}

.btn-check:checked + &,
.btn-check:active + &,
&:active,
&.active {
@include box-shadow($btn-active-box-shadow);
Expand Down Expand Up @@ -81,8 +83,7 @@
text-decoration: $link-hover-decoration;
}

&:focus,
&.focus {
&:focus {
text-decoration: $link-hover-decoration;
}

Expand Down
Loading