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

Action and popovermenu accessibility #342

Merged
merged 3 commits into from
Apr 11, 2019
Merged

Action and popovermenu accessibility #342

merged 3 commits into from
Apr 11, 2019

Conversation

skjnldsv
Copy link
Contributor

@skjnldsv skjnldsv commented Apr 9, 2019 •
edited
Loading

See aria guidelines https://www.w3.org/TR/2016/WD-wai-aria-practices-1.1-20161214/examples/menu-button/menu-button-1/menu-button-1.html

Added:

  • visual feedback on the menu
  • can navigate through popovermenu with up/down pageUp/pageDown tab/shiftTab
  • can close popovermenu with escape
  • can open popovermenu with space and enter

Missing?

  • A-Z navigation
  • tabindex="-1" on all menu items

@nextcloud/accessibility @nextcloud/vue thoughts and reviews? ❤️

@skjnldsv skjnldsv added 3. to review Waiting for reviews feature: popover Related to the popovermenu component feature: actions Related to the actions components labels Apr 9, 2019
@skjnldsv skjnldsv added this to the next milestone Apr 9, 2019
@skjnldsv skjnldsv self-assigned this Apr 9, 2019
@skjnldsv skjnldsv force-pushed the fix/action/popover-mouse-design branch from e381d5c to 8f55a06 Compare April 9, 2019 17:07
skjnldsv
skjnldsv commented Apr 9, 2019
View reviewed changes
src/components/Action/Action.vue
@keydown.down.exact.prevent="focusNextAction"
@keydown.shift.tab.prevent="focusPreviousAction"
@keydown.tab.exact.prevent="focusNextAction"
@keydown.page-up.exact.prevent="focusFirstAction"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh man, this is so good!! 🌟

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 9, 2019
@skjnldsv skjnldsv force-pushed the fix/action/popover-mouse-design branch from 84fd1de to b2ff781 Compare April 9, 2019 17:38
@jancborchardt
Copy link
Contributor

Nice nice!

A-Z navigation

What do you mean by that?

tabindex="-1" on all menu items

Why that? "-1" means they would be taken out of the tab order, which should be avoided. (Ideally there is no tabindex needed at all, because the HTML elements and order should be accessible.)

@skjnldsv
Copy link
Contributor Author

skjnldsv commented Apr 9, 2019

@jancborchardt read the aria docs :p

A-Z, a-z | Moves focus to the next popup item that starts with that letter.If no popup menu item starts with the letter, focus does not move.

Role Attribute Element Usage
menuitem   li The role="menuitem" attribute identifies the li element as an ARIA menuitem widget.Accessible name for the menuitem widget comes from child text content of the li element.The menuitem widget has tabindex="-1".
  tabindex="-1" li Identifies each li[role=menuitem] as an interactive element on the page (e.g. can recived keyboard focus), but is not part of the tab order of the page.

@skjnldsv skjnldsv mentioned this pull request Apr 10, 2019
Merged
ChristophWurst
ChristophWurst approved these changes Apr 11, 2019
View reviewed changes
Copy link
Contributor

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Looks good!

src/components/Action/Action.vue Outdated Show resolved Hide resolved
@skjnldsv skjnldsv mentioned this pull request Apr 11, 2019
Merged
@skjnldsv
Action and popovermenu accessibility
fb65f45 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Make sure to prevent default on first action action
8a49c18 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Fix $nextTick function
8a714a0 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the fix/action/popover-mouse-design branch from b2ff781 to 8a714a0 Compare April 11, 2019 12:01
@ChristophWurst ChristophWurst merged commit 7eb4f1e into master Apr 11, 2019
@ChristophWurst ChristophWurst deleted the fix/action/popover-mouse-design branch April 11, 2019 12:34
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Apr 11, 2019
@skjnldsv skjnldsv modified the milestones: next, 0.10.0 May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@jancborchardt jancborchardt Awaiting requested review from jancborchardt

@juliushaertl juliushaertl Awaiting requested review from juliushaertl

@korelstar korelstar Awaiting requested review from korelstar

Assignees

@skjnldsv skjnldsv

Labels
4. to release Ready to be released and/or waiting for tests to finish feature: actions Related to the actions components feature: popover Related to the popovermenu component
Projects
None yet
Milestone
0.10.0
Development

Successfully merging this pull request may close these issues.
3 participants
@skjnldsv @jancborchardt @ChristophWurst