Skip to content

Commit

Permalink
fix(menu,select): fix final aria issues
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 568326002
  • Loading branch information
Elliott Marquez authored and copybara-github committed Sep 25, 2023
1 parent 54c4ddb commit aeb5103
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 23 deletions.
4 changes: 2 additions & 2 deletions docs/components/menu.md
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ The role of the `md-list` inside the menu can be set with the `type` attribute.
The role of each individual `md-menu-item` can also be set with the `type`
attribute. Anything else slotted into the menu that is not a list item in most
cases should be set to `role="none"`, and `md-divider` should have
`role="separator"`.
`role="separator"` and `tabindex="-1"`.

```html
<!--
Expand All @@ -301,7 +301,7 @@ cases should be set to `role="none"`, and `md-divider` should have
<md-menu-item type="option" id="0">
<div slot="headline">Alabama</div>
</md-menu-item>
<md-divider role="separator"></md-divider>
<md-divider role="separator" tabindex="-1"></md-divider>
<md-menu-item type="option" id="1" selected aria-selected="true">
<div slot="headline">Alabama</div>
</md-menu-item>
Expand Down
12 changes: 6 additions & 6 deletions menu/demo/stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,6 @@ const linkable: MaterialStoryInit<StoryKnobs> = {
styles: sharedStyle,
render(knobs) {
const items = fruitNames.map((name, index) => {
// we want to render dividers between items, so we need to know whether
// it's the last item or not.
const isLastItem = index === fruitNames.length - 1;
return html`
<md-menu-item
id=${index}
Expand All @@ -165,7 +162,10 @@ const linkable: MaterialStoryInit<StoryKnobs> = {
${knobs['link icon']}
</md-icon>
</md-menu-item>
${!isLastItem ? html`<md-divider role="separator"></md-divider>` : ''}`;
${
index === 2 ?
html`<md-divider role="separator" tabindex="-1"></md-divider>` :
''}`;
});

return html`
Expand Down Expand Up @@ -471,8 +471,8 @@ function displayCloseEvent(event: CloseMenuEvent) {

function setButtonAriaExpandedFalse(e: Event) {
const root = (e.target as HTMLElement).getRootNode() as ShadowRoot;
// get the button element and set aria-expaned="false" if exists
root.querySelector('#button')?.setAttribute('aria-expanded', 'false');
// get the button element and remove aria-expaned if exists
root.querySelector('#button')?.removeAttribute('aria-expanded');
}

/** Menu stories. */
Expand Down
24 changes: 14 additions & 10 deletions menu/internal/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import '../../focus/md-focus-ring.js';
import '../../elevation/elevation.js';

import {html, isServer, LitElement} from 'lit';
import {html, isServer, LitElement, PropertyValues} from 'lit';
import {property, query, queryAssignedElements, state} from 'lit/decorators.js';
import {classMap} from 'lit/directives/class-map.js';
import {styleMap} from 'lit/directives/style-map.js';
Expand Down Expand Up @@ -345,6 +345,19 @@ export abstract class Menu extends LitElement {
return this.listController.items;
}

protected override willUpdate(changed: PropertyValues<Menu>) {
if (!changed.has('open')) {
return;
}

if (this.open) {
this.removeAttribute('aria-hidden');
return;
}

this.setAttribute('aria-hidden', 'true');
}

protected override render() {
return this.renderSurface();
}
Expand All @@ -363,7 +376,6 @@ export abstract class Menu extends LitElement {
${this.renderMenuItems()}
</div>
</div>
${this.renderFocusRing()}
</div>
`;
}
Expand All @@ -390,14 +402,6 @@ export abstract class Menu extends LitElement {
return html`<md-elevation part="elevation"></md-elevation>`;
}

/**
* Renders the focus ring component.
*/
private renderFocusRing() {
return html`<md-focus-ring part="focus-ring" .control=${this}>
</md-focus-ring>`;
}

private getSurfaceClasses() {
return {
open: this.open,
Expand Down
2 changes: 1 addition & 1 deletion select/demo/demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {stories, StoryKnobs} from './stories.js';
const collection =
new MaterialCollection<KnobTypesToKnobs<StoryKnobs>>('Select', [
new Knob('md-select', {ui: title()}),
new Knob('label', {ui: textInput(), defaultValue: ''}),
new Knob('label', {ui: textInput(), defaultValue: 'Fruit'}),
new Knob('typeaheadDelay', {ui: numberInput(), defaultValue: 200}),
new Knob('quick', {ui: boolInput(), defaultValue: false}),
new Knob('required', {ui: boolInput(), defaultValue: false}),
Expand Down
2 changes: 2 additions & 0 deletions select/demo/stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const outlined: MaterialStoryInit<StoryKnobs> = {
render(knobs) {
return html`
<md-outlined-select
aria-label="select a fruit"
.label=${knobs.label}
.quick=${knobs.quick}
.required=${knobs.required}
Expand All @@ -57,6 +58,7 @@ const filled: MaterialStoryInit<StoryKnobs> = {
render(knobs) {
return html`
<md-filled-select
aria-label="select a fruit"
.label=${knobs.label}
.quick=${knobs.quick}
.required=${knobs.required}
Expand Down
3 changes: 3 additions & 0 deletions select/internal/_shared.scss
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
:host {
color: unset;
min-width: 210px;
display: flex;
}

.field {
Expand All @@ -26,6 +27,7 @@

.select {
position: relative;
flex-direction: column;
}

.icon.trailing svg,
Expand Down Expand Up @@ -64,6 +66,7 @@
min-width: inherit;
width: inherit;
max-width: inherit;
display: flex;
}

.field,
Expand Down
17 changes: 13 additions & 4 deletions select/internal/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {classMap} from 'lit/directives/class-map.js';
import {html as staticHtml, StaticValue} from 'lit/static-html.js';

import {Field} from '../../field/internal/field.js';
import {ARIAMixinStrict} from '../../internal/aria/aria.js';
import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js';
import {redispatchEvent} from '../../internal/controller/events.js';
import {getActiveItem} from '../../list/internal/list-navigation-helpers.js';
import {CloseMenuEvent, isElementInSubtree, isSelectableKey} from '../../menu/internal/controllers/shared.js';
Expand All @@ -36,6 +38,10 @@ const VALUE = Symbol('value');
* closed.
*/
export abstract class Select extends LitElement {
static {
requestUpdateOnAriaChange(Select);
}

/** @nocollapse */
static readonly formAssociated = true;

Expand Down Expand Up @@ -423,7 +429,9 @@ export abstract class Select extends LitElement {
part="field"
id="field"
tabindex=${this.disabled ? '-1' : '0'}
aria-expanded=${this.open ? 'true' : 'false'}
aria-label=${(this as ARIAMixinStrict).ariaLabel || nothing}
aria-describedby="description"
aria-expanded=${this.open ? 'true' : nothing}
aria-controls="listbox"
class="field"
label=${this.label}
Expand All @@ -440,7 +448,8 @@ export abstract class Select extends LitElement {
@click=${this.handleClick}
@focus=${this.handleFocus}
@blur=${this.handleBlur}>
${this.renderFieldContent()}
${this.renderFieldContent()}
<div id="description" slot="aria-describedby"></div>
</${this.fieldTag}>`;
}

Expand Down Expand Up @@ -484,9 +493,9 @@ export abstract class Select extends LitElement {
<md-menu
id="listbox"
default-focus="none"
tabindex="-1"
role="listbox"
aria-hidden=${this.open ? nothing : 'true'}
tabindex="-1"
aria-label=${(this as ARIAMixinStrict).ariaLabel || nothing}
stay-open-on-focusout
part="menu"
exportparts="focus-ring: menu-focus-ring"
Expand Down

0 comments on commit aeb5103

Please sign in to comment.