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

[base-ui][Switch] Missing accessibility features #40615

Closed
DiegoAndai opened this issue Jan 15, 2024 · 8 comments · Fixed by #40907
Closed

[base-ui][Switch] Missing accessibility features #40615

DiegoAndai opened this issue Jan 15, 2024 · 8 comments · Fixed by #40907
Assignees
Labels
accessibility a11y bug 🐛 Something doesn't work component: switch This is the name of the generic UI component, not the React module! package: base-ui Specific to @mui/base

Comments

@DiegoAndai
Copy link
Member

DiegoAndai commented Jan 15, 2024

Steps to reproduce

Link to live example: https://mui.com/base-ui/react-switch/#introduction

Current behavior

The Switch component and useSwitch hook are missing some important accessibility features:

  • Root element's role should be switch
  • Root element should have aria-checked attribute
  • The Enter key should toggle the switch

Expected behavior

For the component and hook to comply with the authoring practices.

Benchmarks

Your environment

npx @mui/envinfo
  System:
    OS: macOS 13.4.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 344.23 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.19.0 - ~/.nvm/versions/node/v18.19.0/bin/node
    npm: 10.2.3 - ~/.nvm/versions/node/v18.19.0/bin/npm
    pnpm: 8.13.1 - ~/.nvm/versions/node/v18.19.0/bin/pnpm
  Managers:
    Homebrew: 4.2.0 - /usr/local/bin/brew
    pip3: 21.2.4 - /usr/bin/pip3
    RubyGems: 3.0.3.1 - /usr/bin/gem
  Utilities:
    Make: 3.81 - /usr/bin/make
    GCC: 14.0.3 - /usr/bin/gcc
    Git: 2.39.2 - /usr/bin/git
    Clang: 14.0.3 - /usr/bin/clang
    Curl: 7.88.1 - /usr/bin/curl
  Servers:
    Apache: 2.4.56 - /usr/sbin/apachectl
  IDEs:
    VSCode: 1.85.1 - /usr/local/bin/code
    Vim: 9.0 - /usr/bin/vim
    Xcode: /undefined - /usr/bin/xcodebuild
  Languages:
    Bash: 3.2.57 - /bin/bash
    Perl: 5.30.3 - /usr/bin/perl
    Python3: 3.9.6 - /usr/bin/python3
    Ruby: 2.6.10 - /usr/bin/ruby
  Databases:
    SQLite: 3.39.5 - /usr/bin/sqlite3
  Browsers:
    Chrome: 120.0.6099.216
    Safari: 16.5.1
  Monorepos:
    Lerna: 8.0.2

Search keywords: base-ui switch accessibility

@DiegoAndai DiegoAndai added bug 🐛 Something doesn't work accessibility a11y labels Jan 15, 2024
@DiegoAndai DiegoAndai self-assigned this Jan 15, 2024
@DiegoAndai
Copy link
Member Author

@michaldudak I discovered these while preparing a demo of why I think the hooks are a great pattern for Base UI (instead of one component per node). I'm spending some time on Friday on this, so I can take this issue at that time as well.

@DiegoAndai DiegoAndai added the package: base-ui Specific to @mui/base label Jan 15, 2024
@danilo-leal danilo-leal added the component: switch This is the name of the generic UI component, not the React module! label Jan 15, 2024
@michaldudak michaldudak added this to the Base UI: Stable release milestone Jan 17, 2024
@DiegoAndai
Copy link
Member Author

DiegoAndai commented Jan 19, 2024

We need the design to specify first what behavior we're looking for. There's no clear answer:

Radix seems to follow the last example: https://www.radix-ui.com/primitives/docs/components/switch. React Aria components seem to follow none 😅

Going on a tangent from now forward...

@michaldudak @colmtuite @mj12albert: it's clearer to me by the day that:

  • Internally, we need accessibility/behavior documentation explaining how each component should behave. This is the design for an unstyled library. We need to apply the same principles we would apply to design workflows for styled libraries.
  • Said documentation would also be valuable to the community, as there seems to be no resources for it. Right now you can either benchmark with other component libraries by manually testing them, or you can use W3C subpar examples and documentation. I see this as an opportunity for a differentiator:
    • Create proper component authoring practices (we have to do it for internal purposes anyway) based on accessibility guidelines. In other words, creating a well-crafted version of W3C authoring practices.
    • Host it in the Base UI docs. This would hopefully add a new source of traffic for the library.

@michaldudak michaldudak moved this from Backlog to Selected in Base UI Alpha Jan 19, 2024
@michaldudak
Copy link
Member

According to the ARIA spec, the aria-checked attribute is required when role=switch. Ultimately, this should be our reference, as inventing our own standards or patterns may not be the best approach (it may work today, but not necessarily in the future).

@DiegoAndai
Copy link
Member Author

According to the ARIA spec, the aria-checked attribute is required when role=switch.

Then we're missing both, right? It doesn't seem right not to have the role switch for the switch component. As per the specs:

"A switch provides approximately the same functionality as a checkbox and toggle button, but makes it possible for assistive technologies to present the widget in a fashion consistent with its on-screen appearance."

@michaldudak
Copy link
Member

Yes, we should add both.

@KirankumarAmbati
Copy link
Contributor

@DiegoAndai Hey! Can I pick this up & make the change?

@KirankumarAmbati
Copy link
Contributor

#40907

@KirankumarAmbati
Copy link
Contributor

@DiegoAndai We can close this issue as the changes are now merged

@michaldudak michaldudak linked a pull request Feb 16, 2024 that will close this issue
1 task
@github-project-automation github-project-automation bot moved this from Selected to Done in Base UI Alpha Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y bug 🐛 Something doesn't work component: switch This is the name of the generic UI component, not the React module! package: base-ui Specific to @mui/base
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants