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(action menu): keyboard accessibility omnibus #5031

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
91b72f7
fix(menu): added rovingTabindexController and removed aria-activedesc…
nikkimk Jan 16, 2025
adc0d26
fix(menu): fixes firefox keyboard navigation and call stack issues
nikkimk Jan 16, 2025
97d88c3
test(menu): updates menu test based on changes
nikkimk Jan 16, 2025
d10ed4c
test(menu): adds click test for accessibility
nikkimk Jan 16, 2025
cb74cba
test(menu): menu itself should delegate focus to active item
nikkimk Jan 17, 2025
3876c36
fix(menu): fixed menu group navigation according WAI ARIA APG
nikkimk Jan 23, 2025
350e9ba
fix(reactive-controllers): complex element delegates focus to active …
nikkimk Jan 23, 2025
b7c750f
fix(menu): components should delegate focus
nikkimk Jan 27, 2025
68a298d
fix(menu): update keyboard nav to match WAI ARIA APG
nikkimk Jan 27, 2025
63e1dab
fix(menu): updated docs to reflect preferred accessible method
nikkimk Jan 28, 2025
b6ee246
fix(menu): selection and keyboard navigation according to APG
nikkimk Jan 29, 2025
ef99ebb
test(menu): updated to recommended a11y behavior from APG
nikkimk Jan 29, 2025
eb7e539
docs(menu): reverted docs but fixed typos
nikkimk Jan 29, 2025
36cc4a2
fix(menu): removed unused map
nikkimk Jan 29, 2025
9f5f16c
test(menu): updated test to match changed group id
nikkimk Jan 29, 2025
01be995
test(menu): keyboard selection is fixed
nikkimk Jan 29, 2025
c5a9514
test(menu): updated to reflect WAI ARIA APG
nikkimk Jan 29, 2025
a949c61
fix(picker): picker should delegate focus
nikkimk Jan 29, 2025
62147ea
fix(action-menu): fixed a11y error message
nikkimk Jan 29, 2025
8211077
fix(picker): according to APG ArrowDown should focus on first item
nikkimk Jan 30, 2025
66eb2d8
test(action-menu): updated tests for focus delegation and APG keyboar…
nikkimk Jan 30, 2025
7f6b363
fix(picker): ensure overlay is open before setting focus
nikkimk Jan 30, 2025
d4f7f93
fix(menu): removed extra focus
nikkimk Jan 30, 2025
291231f
Merge branch 'main' of github.com:adobe/spectrum-web-components into …
nikkimk Jan 30, 2025
3411413
chore: added changeset
nikkimk Jan 30, 2025
645342d
fix(picker): prevents the picker button from getting focus when menu …
nikkimk Jan 30, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/lemon-points-ring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
'@spectrum-web-components/reactive-controllers': minor
'@spectrum-web-components/action-menu': minor
'@spectrum-web-components/picker': minor
'@spectrum-web-components/menu': minor
---

Used WAI ARIA Authoring Practices Guide (APG) to make accessibility improvements for `<sp-action-menu>`, `<sp-menu>`, and `<sp-picker>`, including:
- Numpad keys now work with `<sp-picker>` and `<sp-action-menu>`
-`<sp-action-menu>`'s `<sp-menu-item>` elements can now be read by a screen reader ([#4556](https://github.com/adobe/spectrum-web-components/issues/4556))
- `<sp-menu-item>` href can now be clicked by a screen reader ([#4997](https://github.com/adobe/spectrum-web-components/issues/4997))
- Opening a `<sp-action-menu>`, `<sp-menu>`, and `<sp-picker>` with a keyboard now sets focus on an item within the menu. ([#4557](https://github.com/adobe/spectrum-web-components/issues/4557))

See the following APG examples for more information:
- [Navigation Menu Example](https://www.w3.org/WAI/ARIA/apg/patterns/menubar/examples/menubar-navigation/)
- [Editor Menubar Example](https://www.w3.org/WAI/ARIA/apg/patterns/menubar/examples/menubar-editor/)
28 changes: 28 additions & 0 deletions packages/action-menu/src/ActionMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,4 +135,32 @@ export class ActionMenu extends ObserveSlotPresence(
}
super.update(changedProperties);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Action Menu was throwing the no label warning for a test that used the label-only slot, so I decided to override Picker's accessible label check and warning with a warning specific to action menu.

protected override hasAccessibleLabel(): boolean {
return (
!!this.label ||
!!this.getAttribute('aria-label') ||
!!this.getAttribute('aria-labelledby') ||
!!this.appliedLabel ||
this.hasLabel ||
this.labelOnly
);
}

protected override warnNoLabel(): void {
window.__swc.warn(
this,
`<${this.localName}> needs one of the following to be accessible:`,
'https://opensource.adobe.com/spectrum-web-components/components/action-menu/#accessibility',
{
type: 'accessibility',
issues: [
`an <sp-field-label> element with a \`for\` attribute referencing the \`id\` of the \`<${this.localName}>\`, or`,
'value supplied to the "label" attribute, which will be displayed visually as placeholder text',
'text content supplied in a <span> with slot="label", or, text content supplied in a <span> with slot="label-only"',
'which will also be displayed visually as placeholder text.',
],
}
);
}
}
5 changes: 2 additions & 3 deletions packages/action-menu/test/action-menu-groups.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ describe('Action Menu - Groups', () => {
groupsWithSelects({ onChange: () => {} })
);

const firstGroup = el.querySelector('sp-menu-group') as HTMLElement;
const firstItem = el.querySelector('sp-menu-item') as MenuItem;

expect(firstItem.focused).to.be.false;
expect(document.activeElement === firstGroup).to.be.false;
expect(document.activeElement === firstItem).to.be.false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Menus and Groups should delegate focus to their active item, so the active element should be the item, not the group.


const opened = oneEvent(el, 'sp-opened');
el.focus();
Expand All @@ -39,7 +38,7 @@ describe('Action Menu - Groups', () => {

expect(firstItem.focused).to.be.true;
expect(
document.activeElement === firstGroup,
document.activeElement === firstItem,
document.activeElement?.localName
).to.be.true;
});
Expand Down
4 changes: 2 additions & 2 deletions packages/menu/menu-group.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ An `<sp-menu-group>` can be used to organize `<sp-menu-item>`s in an `<sp-memu>`
<p>
Your favorite park in New York is: <span id="group-1-value"></span>
<br><br>
Your favorite park in San Fransisco is: <span id="group-2-value"></span>
Your favorite park in San Francisco is: <span id="group-2-value"></span>
</p>
<sp-popover open style="position: relative">
<sp-menu
Expand Down Expand Up @@ -62,7 +62,7 @@ An `<sp-menu-group>` can be used to organize `<sp-menu-item>`s in an `<sp-memu>`
id="group-2"
selects="single"
>
<span slot="header">San Fransisco</span>
<span slot="header">San Francisco</span>
<sp-menu-item>
Golden Gate Park
</sp-menu-item>
Expand Down
33 changes: 28 additions & 5 deletions packages/menu/menu-item.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,35 @@ Content assigned to the `value` slot will be placed at the end of the `<sp-menu-
An `<sp-menu-item>` can also accept content addressed to its `submenu` slot. Using the `<sp-menu>` element with this slot name the options will be surfaced in flyout menu that can be activated by hovering over the root menu item with your pointer or focusing the menu item and pressing the appropriate `ArrowRight` or `ArrowLeft` key based on text direction to move into the submenu.

```html
<sp-menu style="width: 200px;">
<p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a more detailed example to docs to see menu item with submenus.

Your favorite park in New York is:
<span id="group-1-value"></span>
<br />
<br />
Your favorite park in San Francisco is:
<span id="group-2-value"></span>
</p>
<sp-menu
style="width: 200px;"
onchange="this.parentElement
.previousElementSibling
.querySelector(`#${arguments[0].target.id}-value`)
.textContent = arguments[0].target.value"
>
<sp-menu-item>
New York
<sp-menu slot="submenu" selects="single">
<sp-menu-item>Central Park</sp-menu-item>
<sp-menu-item>Flushing Meadows Corona Park</sp-menu-item>
<sp-menu-item>Prospect Park</sp-menu-item>
</sp-menu>
</sp-menu-item>
<sp-menu-item>
Item with submenu
<sp-menu slot="submenu">
<sp-menu-item>Additional options</sp-menu-item>
<sp-menu-item>Available on request</sp-menu-item>
San Francisco
<sp-menu slot="submenu" selects="single">
<sp-menu-item>Golden Gate Park</sp-menu-item>
<sp-menu-item>John McLaren Park</sp-menu-item>
<sp-menu-item>Lake Merced Park</sp-menu-item>
</sp-menu>
</sp-menu-item>
</sp-menu>
Expand Down
Loading
Loading