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.

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

Proposal: use button for toggles instead of checkbox #6576

Closed
janhassel opened this issue Jul 29, 2020 · 5 comments
Closed

Proposal: use button for toggles instead of checkbox #6576

janhassel opened this issue Jul 29, 2020 · 5 comments
Labels
component: toggle hacktoberfest See https://hacktoberfest.com/ proposal: accepted This request has gone through triaging and we are accepting PR's against it. type: a11y ♿ type: enhancement 💡

Comments

@janhassel
Copy link
Member

janhassel commented Jul 29, 2020

Currently, the Toggle and ToggleSmall components use <input type="checkbox"> under the hood. I'd like to propose switching this implementation to <button role="switch"> for the following two reasons:

Accessibility

The WAI-ARIA Authoring Practices 1.1 describe the switch role as "A type of checkbox that represents on/off values, as opposed to checked/unchecked values".
Compared to a checkbox, a button with role="switch" cannot have a mixed state, which a toggle should never have either. According to the W3C, assistive technologies will "present the widget in a fashion consistent with its on-screen appearance".

Related guidance on MDN: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/Switch_role

Preventing bugs

A checkbox cannot be toggled by pressing the enter key by default, which led to a PR last year that added this functionality by using the keyup event. When using a toggle as the first focused element in a modal or overflow menu (by using props.selectorPrimaryFocus), it will automatically be triggered if the modal or overflow menu is opened by pressing the enter key instead of space. This is not expected by the user and can lead to confusion and unintended changes in a user's settings.

Demo: https://bc02h.csb.app/

The issue is only noticeable with the modal when the enter key is pressed for a bit longer and released as soon as the modal is visible. This therefore affects mostly users with motion impairment. The effect on an overflow menu is instant and affects all users.

@joshblack
Copy link
Contributor

@janhassel thanks for taking the time to make this proposal! Makes a lot of sense to me, cc @dakahn I'm curious what you think too.

@designertyler
Copy link
Contributor

We marked this as "needs more research" in our backlog grooming session to see how this may be a breaking change for teams who have used toggles in their forms.

@janhassel
Copy link
Member Author

I agree this would probably be a breaking change as some teams might be relying on the checkbox element directly instead of just the component's API.

I'm wondering if this would be something that...
a) could be a refactor that ships with v11
b) could be made available earlier as a new component suffixed by "v2" (or something else). I think this was the approach around Dropdowns and DataTables before, right? There was a Dropdown component that at some point was marked deprecated and the DropdownV2 componentn was the one carried over as the default for the next major release.

Both mentioned options obviously assume this refactor would indeed improve the user's and developer's experience based on the two reasons I posted in the original issue comment (bug preventing and accessiblity). /cc @dakahn

@dakahn
Copy link
Contributor

dakahn commented Aug 19, 2020

Great research @janhassel. I'm all for making this a v11 breaking change. This would go a long way to improve usability.

@joshblack joshblack added proposal: accepted This request has gone through triaging and we are accepting PR's against it. and removed proposal: needs more research 🕵️‍♀️ proposal: open This request has gone through triaging. We're determining whether we take this on or not. labels Sep 11, 2020
@tw15egan tw15egan added hacktoberfest See https://hacktoberfest.com/ and removed hacktoberfest See https://hacktoberfest.com/ labels Sep 29, 2020
@andreancardona andreancardona linked a pull request Apr 29, 2021 that will close this issue
@tw15egan tw15egan removed a link to a pull request Apr 29, 2021
@janhassel
Copy link
Member Author

Should this be closed as the proposal was implemented under the v11 feature flag?

@dakahn dakahn closed this as completed Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: toggle hacktoberfest See https://hacktoberfest.com/ proposal: accepted This request has gone through triaging and we are accepting PR's against it. type: a11y ♿ type: enhancement 💡
Projects
None yet
Development

No branches or pull requests

5 participants