From 7b52c4515fa048a772f7d598e948b1d62de11c03 Mon Sep 17 00:00:00 2001 From: Elizabeth Mitchell Date: Mon, 24 Apr 2023 14:10:06 -0700 Subject: [PATCH] fix: remove @ariaProperty decorator PiperOrigin-RevId: 526750432 --- checkbox/lib/checkbox.ts | 15 ++- circularprogress/lib/circular-progress.ts | 24 ++-- decorators/aria-property.ts | 105 ------------------ decorators/test/aria-property_test.ts | 103 ----------------- fab/lib/shared.ts | 23 ++-- iconbutton/lib/icon-button.ts | 39 +++---- linearprogress/lib/linear-progress.ts | 18 +-- list/lib/list.ts | 26 ++--- list/lib/listitem/list-item.ts | 20 ++-- list/lib/listitemlink/list-item-link.ts | 5 +- menu/lib/menu.ts | 20 ++-- navigationbar/lib/navigation-bar.ts | 19 ++-- .../lib/navigation-drawer-modal.ts | 45 +++----- navigationdrawer/lib/navigation-drawer.ts | 41 ++----- navigationtab/lib/navigation-tab.ts | 20 ++-- radio/lib/radio.ts | 15 ++- segmentedbutton/lib/segmented-button.ts | 18 +-- .../lib/segmented-button-set.ts | 18 +-- slider/lib/slider.ts | 19 ++-- switch/lib/switch.ts | 25 ++--- switch/lib/switch_test.ts | 26 +---- textfield/lib/text-field.ts | 42 ++----- 22 files changed, 194 insertions(+), 492 deletions(-) delete mode 100644 decorators/aria-property.ts delete mode 100644 decorators/test/aria-property_test.ts diff --git a/checkbox/lib/checkbox.ts b/checkbox/lib/checkbox.ts index 96053fd646..7a1dd0c223 100644 --- a/checkbox/lib/checkbox.ts +++ b/checkbox/lib/checkbox.ts @@ -12,18 +12,23 @@ import {property, query, queryAsync, state} from 'lit/decorators.js'; import {classMap} from 'lit/directives/class-map.js'; import {when} from 'lit/directives/when.js'; +import {requestUpdateOnAriaChange} from '../../aria/delegate.js'; import {dispatchActivationClick, isActivationClick, redispatchEvent} from '../../controller/events.js'; import {FormController, getFormValue} from '../../controller/form-controller.js'; import {stringConverter} from '../../controller/string-converter.js'; -import {ariaProperty} from '../../decorators/aria-property.js'; import {pointerPress, shouldShowStrongFocus} from '../../focus/strong-focus.js'; import {ripple} from '../../ripple/directive.js'; import {MdRipple} from '../../ripple/ripple.js'; +import {ARIAMixinStrict} from '../../types/aria.js'; /** * A checkbox component. */ export class Checkbox extends LitElement { + static { + requestUpdateOnAriaChange(this); + } + /** * @nocollapse */ @@ -70,10 +75,6 @@ export class Checkbox extends LitElement { return this.closest('form'); } - @ariaProperty // tslint:disable-line:no-new-decorators - @property({attribute: 'data-aria-label', noAccessor: true}) - override ariaLabel!: string; - @state() private prevChecked = false; @state() private prevDisabled = false; @state() private prevIndeterminate = false; @@ -135,6 +136,8 @@ export class Checkbox extends LitElement { 'prev-disabled': this.prevDisabled, }); + // Needed for closure conformance + const {ariaLabel} = this as ARIAMixinStrict; return html`
@@ -148,7 +151,7 @@ export class Checkbox extends LitElement {
@@ -58,7 +60,7 @@ export class CircularProgress extends LitElement { `; } - // Determinate mode is rendered with an svg so the progress arc can be + // Determinate mode is rendered with an svg so the progress arc can be // easily animated via stroke-dashoffset. protected renderDeterminateContainer() { const dashOffset = (1 - this.progress) * 100; @@ -72,10 +74,10 @@ export class CircularProgress extends LitElement { `; } - // Indeterminate mode rendered with 2 bordered-divs. The borders are + // Indeterminate mode rendered with 2 bordered-divs. The borders are // clipped into half circles by their containers. The divs are then carefully // animated to produce changes to the spinner arc size. - // This approach has 4.5x the FPS of rendering via svg on Chrome 111. + // This approach has 4.5x the FPS of rendering via svg on Chrome 111. // See https://lit.dev/playground/#gist=febb773565272f75408ab06a0eb49746. protected renderIndeterminateContainer() { return html` @@ -88,4 +90,4 @@ export class CircularProgress extends LitElement { `; } -} \ No newline at end of file +} diff --git a/decorators/aria-property.ts b/decorators/aria-property.ts deleted file mode 100644 index 73f1951a2d..0000000000 --- a/decorators/aria-property.ts +++ /dev/null @@ -1,105 +0,0 @@ -/** - * @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 - * \@property({attribute: 'data-aria-label', noAccessor: true}) - * ariaLabel!: string; - * } - * ``` - * @category Decorator - * @ExportDecoratedItems - */ -export function ariaProperty( - prototype: E, property: K) { - // 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 ?? ''); - if (strValue) { - this.dataset[property] = strValue; - } else { - delete this.dataset[property]; - } - - // lit will call this setter whenever the data-* attribute changes. - // However, this.dataset[property] will automatically be updated to the - // current value. To avoid bugs, always request an update regardless of - // the old value. - this.requestUpdate(); - } - }); - - // 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 as E)[property]; - }, - set(this: ReactiveElement, value: E[K]) { - 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 as E)[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/decorators/test/aria-property_test.ts b/decorators/test/aria-property_test.ts deleted file mode 100644 index b9be655b92..0000000000 --- a/decorators/test/aria-property_test.ts +++ /dev/null @@ -1,103 +0,0 @@ -/** - * @license - * Copyright 2021 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -// import 'jasmine'; (google3-only) - -import {html, LitElement} from 'lit'; -import {customElement, property, query} from 'lit/decorators.js'; -import {ifDefined} from 'lit/directives/if-defined.js'; - -import {Environment} from '../../testing/environment.js'; -import {ariaProperty} from '../aria-property.js'; - -describe('@ariaProperty', () => { - const env = new Environment(); - - @customElement('my-element') - class MyElement extends LitElement { - @property({attribute: 'data-aria-label', noAccessor: true}) - @ariaProperty // tslint:disable-line:no-new-decorators - 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'); - }); -}); diff --git a/fab/lib/shared.ts b/fab/lib/shared.ts index ae66d7f838..7dac077722 100644 --- a/fab/lib/shared.ts +++ b/fab/lib/shared.ts @@ -4,9 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -// This is required for @ariaProperty -// tslint:disable:no-new-decorators - import '../../elevation/elevation.js'; import '../../focus/focus-ring.js'; import '../../ripple/ripple.js'; @@ -16,10 +13,11 @@ import {property, queryAsync, state} from 'lit/decorators.js'; import {ClassInfo, classMap} from 'lit/directives/class-map.js'; import {when} from 'lit/directives/when.js'; -import {ariaProperty} from '../../decorators/aria-property.js'; +import {requestUpdateOnAriaChange} from '../../aria/delegate.js'; import {pointerPress, shouldShowStrongFocus} from '../../focus/strong-focus.js'; import {ripple} from '../../ripple/directive.js'; import {MdRipple} from '../../ripple/ripple.js'; +import {ARIAMixinStrict} from '../../types/aria.js'; /** * Sizes variants available to non-extended FABs. @@ -28,14 +26,15 @@ export type FabSize = 'medium'|'small'|'large'; // tslint:disable-next-line:enforce-comments-on-exported-symbols export abstract class SharedFab extends LitElement { + static { + requestUpdateOnAriaChange(this); + } + static override shadowRootOptions: ShadowRootInit = { mode: 'open' as const, delegatesFocus: true, }; - @property({attribute: 'data-aria-label', noAccessor: true}) - @ariaProperty - override ariaLabel!: string; /** * The size of the FAB. * @@ -71,13 +70,15 @@ export abstract class SharedFab extends LitElement { }; protected override render(): TemplateResult { + // Needed for closure conformance + const {ariaLabel} = this as ARIAMixinStrict; return html`