diff --git a/components/button/lib/button.ts b/components/button/lib/button.ts index d404447ec..71d479f1b 100644 --- a/components/button/lib/button.ts +++ b/components/button/lib/button.ts @@ -8,12 +8,12 @@ import '@material/mwc-icon/mwc-icon'; import '../../focus/focus-ring'; import '../../ripple/mwc-ripple'; -import {AriaHasPopup, ariaProperty} from '@material/mwc-base/aria-property'; import {html, LitElement, TemplateResult} from 'lit'; import {eventOptions, property, query, queryAssignedNodes, queryAsync, state} from 'lit/decorators'; import {ClassInfo, classMap} from 'lit/directives/class-map'; import {ifDefined} from 'lit/directives/if-defined'; +import {ariaProperty} from '../../decorators/aria-property'; import {pointerPress, shouldShowStrongFocus} from '../../focus/strong-focus'; import {Ripple} from '../../ripple/mwc-ripple'; import {RippleHandlers} from '../../ripple/ripple-handlers'; @@ -25,10 +25,12 @@ export abstract class Button extends LitElement implements ButtonState { static override shadowRootOptions: ShadowRootInit = {mode: 'open', delegatesFocus: true}; - /** @soyPrefixAttribute */ + // TODO(b/210730484): replace with @soyParam annotation + @property({type: String, attribute: 'data-aria-has-popup', noAccessor: true}) @ariaProperty - @property({type: String, attribute: 'aria-haspopup'}) - override ariaHasPopup!: AriaHasPopup; + // TODO(b/210675600): change to shared type + override ariaHasPopup!: 'false'|'true'|'menu'|'listbox'|'tree'|'grid'| + 'dialog'; @property({type: Boolean, reflect: true}) disabled = false; @@ -38,7 +40,10 @@ export abstract class Button extends LitElement implements ButtonState { @property({type: String}) label = ''; - @property({type: String}) override ariaLabel!: string; + // TODO(b/210730484): replace with @soyParam annotation + @property({type: String, attribute: 'data-aria-label', noAccessor: true}) + @ariaProperty + override ariaLabel!: string; @property({type: Boolean}) hasIcon = false; @@ -176,6 +181,7 @@ export abstract class Button extends LitElement implements ButtonState { this.hasIcon = !!this.iconElement && this.iconElement.length > 0; } + // TODO(b/210731759): remove once internal tooling delegates focus override focus() { const buttonElement = this.buttonElement; if (buttonElement) { @@ -183,6 +189,7 @@ export abstract class Button extends LitElement implements ButtonState { } } + // TODO(b/210731759): remove once internal tooling delegates focus override blur() { const buttonElement = this.buttonElement; if (buttonElement) { diff --git a/components/button/lib/state.ts b/components/button/lib/state.ts index ae9c1ce1f..5dfc56144 100644 --- a/components/button/lib/state.ts +++ b/components/button/lib/state.ts @@ -7,6 +7,5 @@ export interface ButtonState { disabled: boolean; label: string; - ariaLabel: string; trailingIcon: boolean; } diff --git a/components/decorators/aria-property.ts b/components/decorators/aria-property.ts new file mode 100644 index 000000000..ffc218337 --- /dev/null +++ b/components/decorators/aria-property.ts @@ -0,0 +1,106 @@ +/** + * @license + * Copyright 2021 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import {ReactiveElement} from 'lit'; + +/** + * A property decorator that helps proxy an aria attribute to an internal node. + * + * This decorator is only intended for use with ARIAMixin properties, + * such as `ariaLabel`, to help with screen readers. + * + * This decorator will remove the host `aria-*` attribute at runtime and add it + * to a `data-aria-*` attribute to avoid screenreader conflicts between the + * host and internal node. + * + * `@ariaProperty` decorated properties should sync with LitElement to the + * `data-aria-*` attribute, not the native `aria-*` attribute. + * + * @example + * ```ts + * class MyElement extends LitElement { + * \@ariaProperty + * // TODO(b/210730484): replace with @soyParam annotation + * \@property({ type: String, attribute: 'data-aria-label', noAccessor: true}) + * ariaLabel!: string; + * } + * ``` + * @category Decorator + * @ExportDecoratedItems + */ +export function ariaProperty( + prototype: ReactiveElement, property: keyof ARIAMixin) { + // Replace the ARIAMixin property with data-* attribute syncing instead of + // using the native aria-* attribute reflection. This preserves the attribute + // for SSR and avoids screenreader conflicts after delegating the attribute + // to a child node. + Object.defineProperty(prototype, property, { + configurable: true, + enumerable: true, + get(this: ReactiveElement) { + return this.dataset[property] ?? ''; + }, + set(this: ReactiveElement, value: unknown) { + // Coerce non-string values to a string + const strValue = String(value ?? ''); + const oldValue = this.dataset[property]; + if (strValue === oldValue) { + return; + } + + if (strValue) { + this.dataset[property] = strValue; + } else { + delete this.dataset[property]; + } + + this.requestUpdate(property, oldValue); + } + }); + + // Define an internal property that syncs from the `aria-*` attribute with lit + // and delegates to the real ARIAMixin property, which renders an update. + // This property will immediately remove the `aria-*` attribute, which doesn't + // work well with SSR (which is why there's a separate synced property). + const internalAriaProperty = Symbol(property); + // "ariaLabel" -> "aria-label" / "ariaLabelledBy" -> "aria-labelledby" + const ariaAttribute = property.replace('aria', 'aria-').toLowerCase(); + const constructor = (prototype.constructor as typeof ReactiveElement); + let removingAttribute = false; + Object.defineProperty(prototype, internalAriaProperty, { + get(this: ReactiveElement) { + // tslint is failing here, but the types are correct (ARIAMixin + // properties do not obfuscate with closure) + // tslint:disable-next-line:no-dict-access-on-struct-type + return this[property]; + }, + set(this: ReactiveElement, value: string) { + if (removingAttribute) { + // Ignore this update, which is triggered below + return; + } + + // Set the ARIAMixin property, which will sync the `data-*` attribute + // and trigger rendering if the value changed. + // tslint is failing here, but the types are correct (ARIAMixin + // properties do not obfuscate with closure) + // tslint:disable-next-line:no-dict-access-on-struct-type + this[property] = value; + // Remove the `aria-*` attribute, which will call this setter again with + // the incorrect value. Ignore these updates. + removingAttribute = true; + this.removeAttribute(ariaAttribute); + removingAttribute = false; + } + }); + + // Tell lit to observe the `aria-*` attribute and set the internal property, + // which acts as a "aria-* attribute changed" observer. + constructor.createProperty(internalAriaProperty, { + attribute: ariaAttribute, + noAccessor: true, + }); +} diff --git a/components/decorators/test/aria-property.test.ts b/components/decorators/test/aria-property.test.ts new file mode 100644 index 000000000..6a25d81b5 --- /dev/null +++ b/components/decorators/test/aria-property.test.ts @@ -0,0 +1,114 @@ +/** + * @license + * Copyright 2021 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import 'jasmine'; + +import {html, LitElement} from 'lit'; +import {customElement, property, query} from 'lit/decorators'; +import {ifDefined} from 'lit/directives/if-defined'; + +import {Environment} from '../../testing/environment'; +import {ariaProperty} from '../aria-property'; + +describe('@ariaProperty', () => { + const env = new Environment(); + + @customElement('my-element') + class MyElement extends LitElement { + // TODO(b/210730484): replace with @soyParam annotation + @property({type: String, attribute: 'data-aria-label', noAccessor: true}) + @ariaProperty + override ariaLabel!: string; + + @query('.root') labelledElement!: HTMLElement; + + override render() { + return html`
`; + } + } + + let element: MyElement; + + beforeEach(async () => { + const root = env.render(html``); + await env.waitForStability(); + element = root.querySelector('my-element') as MyElement; + }); + + it('should set `ariaX` from `data-*` attribute', () => { + const value = 'Aria label'; + element.setAttribute('data-aria-label', value); + expect(element.ariaLabel).toBe(value); + }); + + it('should set `data-*` attribute from `ariaX`', () => { + const value = 'Aria label'; + element.ariaLabel = value; + expect(element.getAttribute('data-aria-label')).toBe(value); + }); + + it('should remove `data-*` attribute when set to an empty string', + async () => { + element.ariaLabel = 'Aria label'; + element.ariaLabel = ''; + expect(element.hasAttribute('data-aria-label')) + .withContext('should not have data-aria-label attribute') + .toBeFalse(); + }); + + it('should set `ariaX` from `aria-*` attribute', () => { + const value = 'Aria label'; + element.setAttribute('aria-label', value); + expect(element.ariaLabel).toBe(value); + }); + + it('should remove `aria-*` attribute when set and keep `ariaX` value', () => { + const value = 'Aria label'; + element.setAttribute('aria-label', value); + expect(element.hasAttribute('aria-label')) + .withContext('should not have aria-label attribute') + .toBeFalse(); + expect(element.ariaLabel).toBe(value); + }); + + it('should delegate to rendered elements after updateComplete', async () => { + const value = 'Aria label'; + element.ariaLabel = value; + await element.updateComplete; + expect(element.labelledElement.getAttribute('aria-label')).toBe(value); + }); + + it('`ariaX` should coerce non-string values to strings', () => { + (element as any).ariaLabel = null; + expect(element.ariaLabel).withContext('null should coerce to ""').toBe(''); + + (element as any).ariaLabel = undefined; + expect(element.ariaLabel) + .withContext('undefined should coerce to ""') + .toBe(''); + + (element as any).ariaLabel = 42; + expect(element.ariaLabel) + .withContext('number should coerce to string') + .toBe('42'); + + (element as any).ariaLabel = true; + expect(element.ariaLabel) + .withContext('boolean should coerce to string') + .toBe('true'); + }); + + it('should not request an update if the value stays the same', async () => { + const value = 'Aria label'; + element.ariaLabel = value; + await element.updateComplete; + element.ariaLabel = value; + expect(element.isUpdatePending) + .withContext('there should not be an update pending') + .toBeFalse(); + }); +}); diff --git a/components/switch/lib/switch.ts b/components/switch/lib/switch.ts index 66a5205c1..579da8cf7 100644 --- a/components/switch/lib/switch.ts +++ b/components/switch/lib/switch.ts @@ -6,7 +6,7 @@ import '@material/mwc-ripple/mwc-ripple'; -import {ariaProperty} from '@material/mwc-base/aria-property'; +import {ariaProperty as legacyAriaProperty} from '@material/mwc-base/aria-property'; import {FormElement} from '@material/mwc-base/form-element'; import {Ripple} from '@material/mwc-ripple/mwc-ripple'; import {RippleHandlers} from '@material/mwc-ripple/ripple-handlers'; @@ -15,6 +15,8 @@ import {eventOptions, property, query, queryAsync, state} from 'lit/decorators'; import {ClassInfo, classMap} from 'lit/directives/class-map'; import {ifDefined} from 'lit/directives/if-defined'; +import {ariaProperty} from '../../decorators/aria-property'; + import {MDCSwitchFoundation} from './foundation'; import {MDCSwitchAdapter, MDCSwitchState} from './state'; @@ -26,13 +28,14 @@ export class Switch extends FormElement implements MDCSwitchState { @property({type: Boolean}) selected = false; // Aria - /** @soyPrefixAttribute */ @ariaProperty - @property({type: String, attribute: 'aria-label'}) - override ariaLabel = ''; + // TODO(b/210730484): replace with @soyParam annotation + @property({type: String, attribute: 'data-aria-label', noAccessor: true}) + override ariaLabel!: string; + // TODO: Add support in @ariaProperty for idref aria attributes /** @soyPrefixAttribute */ - @ariaProperty + @legacyAriaProperty @property({type: String, attribute: 'aria-labelledby'}) ariaLabelledBy = ''; @@ -79,7 +82,7 @@ export class Switch extends FormElement implements MDCSwitchState { class="mdc-switch ${classMap(this.getRenderClasses())}" role="switch" aria-checked="${this.selected}" - aria-label="${ifDefined(this.ariaLabel || undefined)}" + aria-label="${ifDefined(this.ariaLabel)}" aria-labelledby="${ifDefined(this.ariaLabelledBy || undefined)}" .disabled=${this.disabled} @click=${this.handleClick} diff --git a/components/testing/environment.ts b/components/testing/environment.ts index a4a97c44d..549e51800 100644 --- a/components/testing/environment.ts +++ b/components/testing/environment.ts @@ -72,9 +72,12 @@ export class Environment { * Render a Lit template in the environment's root container. * * @param template a Lit `TemplateResult` to render. + * @return The root container the template was rendered to. */ render(template: TemplateResult) { - litRender(template, this.createNewRoot()); + const root = this.createNewRoot(); + litRender(template, root); + return root; } /**