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

[SpeedDial] items should be accessible by tab key #12361

Closed
2 tasks done
brianchirls opened this issue Aug 1, 2018 · 9 comments
Closed
2 tasks done

[SpeedDial] items should be accessible by tab key #12361

brianchirls opened this issue Aug 1, 2018 · 9 comments
Labels
component: speed dial This is the name of the generic UI component, not the React module!

Comments

@brianchirls
Copy link

For accessibility by users with screen readers or who for whatever reason cannot use a mouse or touch, these items should be usable by keyboard combinations.

  • This is a v1.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

You should be able to tab to the FAB and then navigate by keyboard to the action icons, which can then in turn be activated with the Enter key.

Current Behavior

You can tab to the FAB, and the action buttons become visible. But if you try to move around with arrow keys or tab, the FAB is deselected. There is no way to reach the action buttons.

Examples

The behavior can be tested on the component demo page

Context

I'm trying to build an accessible web app. For comparison, here's another implementation that behaves a little better in this respect. https://codepen.io/jh3y/pen/NNegMz

For reference, see this quick guide to testing keyboard accessibility: https://a11yproject.com/posts/navigate-using-just-your-keyboard/
@oliviertassinari oliviertassinari added component: slider This is the name of the generic UI component, not the React module! package: lab Specific to @mui/lab labels Aug 1, 2018
@mbrookes
Copy link
Member

mbrookes commented Aug 1, 2018

@brianchirls The logic is there from the original implementation, but a subsequent PR must have broken it.

Do you want to try and fix it? You could always git bisect to narrow down when it stopped working if it isn't immediately obvious from the code.

@brianchirls
Copy link
Author

I did notice some tabIndex stuff in the code that indicated it should have been in there. So, yeah good call on bisect.

I'm on deadline and won't have time to dive into the code for probably at least three weeks. So if someone else would like to take a crack at it in the meantime, please go ahead.

@mbrookes
Copy link
Member

mbrookes commented Aug 1, 2018 •
edited
Loading

The problem was introduced in #10843. I'm amazed the problem hasn't been noticed sooner!

Cc: @shssoichiro any ideas?

@brianchirls
Copy link
Author

@mbrookes I noticed some other funkiness happening with Tooltips. I wonder if they're related issues. I'll try to isolate those and file separately when I can in the next few weeks.

@mbrookes mbrookes mentioned this issue Aug 5, 2018
Closed
@zanerock zanerock mentioned this issue Aug 9, 2018
Closed
2 tasks
@mbrookes
Copy link
Member

Sorry, slight disconnect on my part. The main FAB should be accessible by tab key, and the speed-dial actions by cursor keys. That was working, a regression was introduced with #10843.

@shssoichiro
Copy link
Contributor

shssoichiro commented Aug 11, 2018 •
edited
Loading

@mbrookes Sorry, this slipped through my notifications. Unfortunately I haven't even looked at the SpeedDial component. Nothing in that PR stands out to me as something that should break that arrow navigation, either. The only thing that really even looks suspect is the movement of the EventListener on the Tooltip from outside the Manager to inside, although I don't have a good understanding of the intricacies ofEventListener or Manager.

Ironically, my name is on that PR, but the majority of those changes are from @oliviertassinari.

@oliviertassinari
Copy link
Member

Ironically, my name is on that PR, but the majority of those changes are from @oliviertassinari.

@shssoichiro It's true. I have been continuing a lot of pull requests this year.

@eps1lon
Copy link
Member

eps1lon commented Oct 2, 2018

This will be addressed in #12725.

We will consider the dial as a menu. The actions will therefore be navigatable via arrow keys following https://www.w3.org/TR/wai-aria-practices/examples/menu-button/menu-button-links.html

@mbrookes
Copy link
Member

mbrookes commented Oct 2, 2018

We will consider the dial as a menu.

To be pedantic, we already do (or rather did) it just got broken by #10843. #12725 is more of a refactor, while also fixing the regression.

@eps1lon eps1lon added component: speed dial This is the name of the generic UI component, not the React module! and removed component: slider This is the name of the generic UI component, not the React module! labels Oct 3, 2018
@eps1lon eps1lon mentioned this issue Oct 3, 2018
Merged
2 tasks
eps1lon added a commit that referenced this issue Oct 3, 2018
@eps1lon
[SpeedDial] Fix navigation between SpeedDialActions (#12725)
770c375 
Closes #12361
Addresses #12159

* [SpeedDialAction] Fix react warnings

className and classes.root was not applied to anything or forwarded to
anything in the past. They should be implemented in the future however.

A missing tooltipTitle caused a warning further down the component tree.

* [SpeedDial] Fix react warnings

onKeyDown was applied to the button and the SpeedDialAction. However the
Action did not process onKeyDown and passing the same
event listener to different components is ambiguous.

* [SpeedDial] Fix keyboard navigation when focused

* [SpeedDial] Fix onKeyDown not being called

* [SpeedDial] Implement navigation behavior

Following the proposed spec with alternative B

* [SpeedDial] Fix buttonRef override

* [SpeedDial] Add test cases for navigation

* [docs] Remove unresolved merge residues

* [SpeedDial] Fix onKeyDown not called due to merge conflicts

* [SpeedDial] switch from wrapping to clamping in keyboard navigation

- FAB is included in navigation
- Fixed potential issues when mixing keyboard and mouse navigation or
  refocusing a SpeedDial

* [SpeedDial] Support createRef in ButtonProps
@oliviertassinari oliviertassinari changed the title SpeedDial items should be accessible by tab key [SpeedDial] items should be accessible by tab key Jan 9, 2021
@oliviertassinari oliviertassinari removed the package: lab Specific to @mui/lab label Jan 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: speed dial This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mbrookes @brianchirls @oliviertassinari @shssoichiro @eps1lon