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

fix(NcActions): adjust keyboard navigation #4841

Merged
merged 4 commits into from Nov 17, 2023
Merged

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Nov 16, 2023

☑️ Resolves

See also: https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/

🚧 Tasks

  • Make Arrows ⬆️⬇️ loop over elements
  • TAB and SHIFT + TAB:
    • In menu and navigation should move to next/prev component
    • In popover like actions with form elements should loop over elements
      • ESC closes actions

🖼️ Screenshots

🏚️ Before 🏡 After
Arrow
Don't loop Loop
arrows-before arrows-after
TAB in Menu / Navigation
Between elements Between components
Tab-Menu-Before Tab-Menu
TAB in Popover-like
Submit is not focusable Classic popover with focus trap
Tab-Before Tab-Popover

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
… actions

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme added bug Something isn't working 3. to review Waiting for reviews feature: actions Related to the actions components accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Nov 16, 2023
@ShGKme ShGKme added this to the 8.1.1 milestone Nov 16, 2023
@ShGKme ShGKme self-assigned this Nov 16, 2023
@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 16, 2023

⚠️ Before merge should be tested in apps

@Pytal

This comment was marked as resolved.

@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 16, 2023

I recall that there was a reason the actions menu specifically didn't need to be focus trapped but this seems better :)

Currently it has a focus trap when it is used more like as a popover, but with action items. Like forms.

When it is used as a menu, list of buttons/links - it doesn't have focus-trap

Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥳 works well

@ShGKme
Copy link
Contributor Author

ShGKme commented Nov 17, 2023

Tested in:

  • Server
    • Dashboard - Weather Status
    • Files - File list
    • Files - sidebar: sharing, versions
    • Users - new group + user edit
  • Talk - issues in navigation, but seems unrelated
  • Text - works weirdly, but I'd call it unrelated

@ShGKme ShGKme merged commit d31be11 into master Nov 17, 2023
15 checks passed
@ShGKme ShGKme deleted the fix/nc-actions--keyboard branch November 17, 2023 09:05
@susnux susnux mentioned this pull request Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working feature: actions Related to the actions components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BITV]: Adapt keyboard support for action menu
4 participants