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(select-rich): update WAI ARIA pattern to combobox-select-only #1942

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/honest-foxes-pretend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': patch
---

[select-rich] update design pattern to combobox-select-only
2 changes: 1 addition & 1 deletion docs/components/select-rich/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ It allows providing fully customized options and a fully customized invoker butt
styling,theming or user interaction opportunities.

Its implementation is based on the following Design pattern:
<https://www.w3.org/TR/wai-aria-practices/examples/listbox/listbox-collapsible.html>
<https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-select-only/>

```js script
import { LitElement, html } from '@mdjs/mdjs-preview';
Expand Down
9 changes: 3 additions & 6 deletions docs/fundamentals/systems/overlays/rationale.md
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,8 @@ global(modal) dialog.

## Considerations

### Future for mode local (Popper)
We use the native dialog purely because it renders to the "top layer". Not because of its accessible role, which varies per component. In the future we can use the popover attribute(<https://open-ui.org/components/popover.research.explainer/>) for this (which neatly differentiates between presentation and semantics).

- Default overflow and/or max-width behavior when content is too wide or high for the viewport.
So in our use of the dialog we want also a dialog that only does presentation (rendering on top layer in this case), but we want to check the semantics ourselves. Hence: `role="none"`. Even though the outcome is completely accessible, AXE-core uses this rule as a kind of "precautionary measure".

### Aria roles

- No `aria-controls` as support for it is not quite there yet
- No `aria-haspopup`. People knowing the haspop up and hear about it don’t expect a dialog to open (at this moment in time) but expect a sub-menu. Until support for the dialog value has better implementation, it’s probably best to not use aria-haspopup on the element that opens the modal dialog.
After a cost/benefit analysis, we choose to be temporarily incompatible with AXE-core. Therefore, we recommend running tests with `{ ignoredRules: ['aria-allowed-role'] }`. When the popover is stable and used underwater in lion overlays, AXE can again be run without this ignoredRule.
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ export function runListboxMixinSuite(customConfig = {}) {
`);
await el.updateComplete;
await el.updateComplete; // need 2 awaits as overlay.show is an async function

// for more info about why we need the ignoreRules, see: https://lion-web.netlify.app/fundamentals/systems/overlays/rationale/#considerations
await expect(el).to.be.accessible({ ignoredRules: ['aria-allowed-role'] });
});

Expand All @@ -405,6 +405,7 @@ export function runListboxMixinSuite(customConfig = {}) {
<${optionTag} .choiceValue=${20}>Item 2</${optionTag}>
</${tag}>
`);
// for more info about why we need the ignoreRules, see: https://lion-web.netlify.app/fundamentals/systems/overlays/rationale/#considerations
await expect(el).to.be.accessible({ ignoredRules: ['aria-allowed-role'] });
});

Expand Down
4 changes: 4 additions & 0 deletions packages/ui/components/overlays/src/OverlayController.js
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,10 @@ export class OverlayController extends EventTarget {
// We use a dialog for its visual capabilities: it renders to the top layer.
// A11y will depend on the type of overlay and is arranged on contentNode level.
// Also see: https://www.scottohara.me/blog/2019/03/05/open-dialog.html
//
// The role="dialog" is set on the contentNode (or another role), so role="none"
// is valid here, although AXE complains about this setup.
// For now we need to add `ignoredRules: ['aria-allowed-role']` in your AXE tests.
wrappingDialogElement.setAttribute('role', 'none');
wrappingDialogElement.setAttribute('data-overlay-outer-wrapper', '');
// N.B. position: fixed is needed to escape out of 'overflow: hidden'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export class LionSelectInvoker extends SlotMixin(LionButton) {

connectedCallback() {
super.connectedCallback();
this.setAttribute('role', 'combobox');
this.addEventListener('keydown', this.__handleKeydown);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/ui/components/select-rich/src/LionSelectRich.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ export class LionSelectRich extends SlotMixin(ScopedElementsMixin(OverlayMixin(L
__setupInvokerNode() {
this._invokerNode.id = `invoker-${this._inputId}`;
this._invokerNode.setAttribute('aria-haspopup', 'listbox');

this._invokerNode.setAttribute('aria-controls', this._inputId);
this.__setupInvokerNodeEventListener();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { LionSelectInvoker } from '@lion/ui/select-rich.js';
import '@lion/ui/define/lion-select-invoker.js';

/**
* @typedef {import('../../listbox/src/LionOption.js').LionOption} LionOption
* @typedef {import('@lion/ui/listbox.js').LionOption} LionOption
*/

describe('lion-select-invoker', () => {
Expand Down Expand Up @@ -100,4 +100,13 @@ describe('lion-select-invoker', () => {
expect(el._contentWrapperNode).lightDom.to.equal('no valid selection');
});
});

describe('Accessibility', () => {
it('has set the correct roles', async () => {
const el = /** @type {LionSelectInvoker} */ (
await fixture(html`<lion-select-invoker></lion-select-invoker>`)
);
await expect(el.role).to.equal('combobox');
});
});
});