[EuiContextMenuItem] Added an example aria-current attribute and documentation for a11y
#7153
Conversation
* WIP. Tried out using an isCurrent boolean prop. * Refactored icon check to set aria-current also. * Removed isCurrent prop because aria-current is state not handed down from parent.
| const getCurrentItem = (size, attr) => { | ||
| if (attr === 'icon') { | ||
| return size === rowSize ? 'check' : 'empty'; | ||
| } | ||
|
|
||
| if (attr === 'aria-current') { | ||
| return size === rowSize ? 'true' : undefined; | ||
| } | ||
|
|
||
| return undefined; |
There was a problem hiding this comment.
I went this direction after looking at the EuiContextMenuPanel and EuiContextMenuItem for an hour or so. I didn't see a relationship between the two for state management on single panel menus. The panel component checked for an array of items and then created the child components and rendered them.
Looking at the docs, I saw an opportunity to use the local state management function and extend it. This approach does leave it on the consumer to add an aria-current attribute, but doesn't degrade the screen reader UX by setting a default selected item and not having it get updated.
There was a problem hiding this comment.
I think there's a cleaner way this could be written. Try this:
const isSelectedProps = (size: number) => {
return size === rowSize
? { icon: 'checked', 'aria-current': true }
: { icon: 'empty', 'aria-current': undefined };
};
<EuiContextMenuItem
{...isSelectedProps(10)}
key="10 rows"
// ...
>There was a problem hiding this comment.
Sorry, just realized I didn't address your first paragraph - you're right on this being a docs/example only change and not making this change opinionatedly in EuiContextMenuItem's source code. EuiTablePagination on the other hand should have an actual source code change.
| title="The selected context menu item should have an aria-current attribute" | ||
| > | ||
| <p> | ||
| <strong>aria-current</strong> tells screen readers which item is |
There was a problem hiding this comment.
Just curious - what meaningful or semantic difference is there between aria-current and aria-pressed for this use case?
There was a problem hiding this comment.
It's a subtle distinction, but I went with aria-current="true" because it announced the page as Page 1, current button in screen readers. This felt like a natural, concise UX.
I also looked at MDN for their definitions of aria-pressed and aria-current:
aria-current="true"
Represents the current item within a set.
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-current
aria-pressed="true"
The button is pressed.
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-pressed
There was a problem hiding this comment.
Awesome, thanks for the explanation! 🎉
1Copenut
added
skip-changelog
documentation
Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_7153/ |
💚 Build Succeeded
History
cc @1Copenut |
Summary
The
EuiContextMenuhas an option to create a single panel menu where one item can be selected. I added anaria-current="true"to the example views and updated the local state handler function and documentation.This PR closes #6648
QA
Remove or strikethrough items that do not apply to your PR.
General checklist