diff --git a/core/src/components/button/button.tsx b/core/src/components/button/button.tsx index a1e7f72bf01..eff2143f9aa 100644 --- a/core/src/components/button/button.tsx +++ b/core/src/components/button/button.tsx @@ -1,8 +1,9 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core'; import { Component, Element, Event, Host, Prop, Watch, State, forceUpdate, h } from '@stencil/core'; +import type { AttributeController } from '@utils/attribute-controller'; +import { createAriaAttributeController } from '@utils/attribute-controller'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes } from '@utils/helpers'; -import { inheritAriaAttributes, hasShadowDom } from '@utils/helpers'; +import { hasShadowDom } from '@utils/helpers'; import { printIonWarning } from '@utils/logging'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; @@ -34,7 +35,7 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf private inToolbar = false; private formButtonEl: HTMLButtonElement | null = null; private formEl: HTMLFormElement | null = null; - private inheritedAttributes: Attributes = {}; + private ariaController?: AttributeController; @Element() el!: HTMLElement; @@ -158,27 +159,6 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf */ @Event() ionBlur!: EventEmitter; - /** - * This component is used within the `ion-input-password-toggle` component - * to toggle the visibility of the password input. - * These attributes need to update based on the state of the password input. - * Otherwise, the values will be stale. - * - * @param newValue - * @param _oldValue - * @param propName - */ - @Watch('aria-checked') - @Watch('aria-label') - @Watch('aria-pressed') - onAriaChanged(newValue: string, _oldValue: string, propName: string) { - this.inheritedAttributes = { - ...this.inheritedAttributes, - [propName]: newValue, - }; - forceUpdate(this); - } - /** * This is responsible for rendering a hidden native * button element inside the associated form. This allows @@ -220,7 +200,24 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf this.inToolbar = !!this.el.closest('ion-buttons'); this.inListHeader = !!this.el.closest('ion-list-header'); this.inItem = !!this.el.closest('ion-item') || !!this.el.closest('ion-item-divider'); - this.inheritedAttributes = inheritAriaAttributes(this.el); + + /** + * The ARIA state has to stay live, since `ion-input-password-toggle` rewrites + * `aria-label` and `aria-pressed` on its `ion-button` on every toggle. We keep + * `aria-disabled` out of the watch because the `` below renders it from the + * `disabled` prop and those writes would clobber a developer's value, and `role` out + * because a post-load write stays on the host too, which would put the same role on + * two elements in the accessibility tree. + */ + this.ariaController = createAriaAttributeController(this.el, () => forceUpdate(this), ['aria-disabled', 'role']); + } + + connectedCallback() { + this.ariaController?.init(); + } + + disconnectedCallback() { + this.ariaController?.destroy(); } private get hasIconOnly() { @@ -339,21 +336,8 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf render() { const mode = getIonMode(this); - const { - buttonType, - type, - disabled, - rel, - target, - size, - href, - color, - expand, - hasIconOnly, - shape, - strong, - inheritedAttributes, - } = this; + const { buttonType, type, disabled, rel, target, size, href, color, expand, hasIconOnly, shape, strong } = this; + const inheritedAttributes = this.ariaController?.attributes ?? {}; const finalSize = size === undefined && this.inItem ? 'small' : size; const TagType = href === undefined ? 'button' : ('a' as any); const attrs = diff --git a/core/src/components/button/test/a11y/button.e2e.ts b/core/src/components/button/test/a11y/button.e2e.ts index 585c0b5853d..03cf866534c 100644 --- a/core/src/components/button/test/a11y/button.e2e.ts +++ b/core/src/components/button/test/a11y/button.e2e.ts @@ -148,3 +148,176 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); }); }); + +/** + * Attribute syncing does not vary across modes or directions + */ +configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('button: aria attribute sync'), () => { + /** + * A sample rather than the full ARIA list, since they all go through the same + * membership check and looping every one of them only multiplies the run time. + */ + const ariaAttributes = ['aria-checked', 'aria-label', 'aria-pressed', 'aria-description']; + + for (const attr of ariaAttributes) { + test(`should sync ${attr} to the native button when it changes on the host`, async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + }); + + await page.setContent(`Button`, config); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + await expect(nativeButton).toHaveAttribute(attr, 'initial'); + + await host.evaluate((el, attr) => el.setAttribute(attr, 'updated'), attr); + + await expect(nativeButton).toHaveAttribute(attr, 'updated'); + }); + } + + test('should not sync aria-disabled from the host', async ({ page }) => { + await page.setContent(`Button`, config); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + // The developer-provided value is still copied to the native button at load. + await expect(nativeButton).toHaveAttribute('aria-disabled', 'true'); + + // The host's `aria-disabled` belongs to the `disabled` prop from here on, so later + // writes to it must not reach the native button. We write `aria-label` in the same + // batch as a barrier, since once that lands the sync has run. + await host.evaluate((el) => { + el.setAttribute('aria-disabled', 'false'); + el.setAttribute('aria-label', 'barrier'); + }); + await expect(nativeButton).toHaveAttribute('aria-label', 'barrier'); + await expect(nativeButton).toHaveAttribute('aria-disabled', 'true'); + + // Toggling disabled makes the component write and then clear aria-disabled on the + // host. Neither write should reach the native button. + await host.evaluate((el: HTMLIonButtonElement) => { + el.disabled = true; + el.setAttribute('aria-label', 'disabled'); + }); + await expect(nativeButton).toHaveAttribute('aria-label', 'disabled'); + await expect(nativeButton).toHaveAttribute('aria-disabled', 'true'); + + await host.evaluate((el: HTMLIonButtonElement) => { + el.disabled = false; + el.setAttribute('aria-label', 'enabled'); + }); + await expect(nativeButton).toHaveAttribute('aria-label', 'enabled'); + await expect(nativeButton).toHaveAttribute('aria-disabled', 'true'); + }); + + test('should not sync role from the host', async ({ page }) => { + await page.setContent(`Button`, config); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + // The initial copy moves role onto the native button, as it always has. + await expect(nativeButton).toHaveAttribute('role', 'switch'); + + // A later write is only read, so it stays on the host. Copying it as well would put + // the same role on both elements, and two of that role in the accessibility tree. + await host.evaluate((el) => { + el.setAttribute('role', 'checkbox'); + el.setAttribute('aria-label', 'barrier'); + }); + await expect(nativeButton).toHaveAttribute('aria-label', 'barrier'); + await expect(nativeButton).toHaveAttribute('role', 'switch'); + }); + + test('should keep syncing after the button is detached and reattached', async ({ page }) => { + await page.setContent( + ` +
+ Button +
+ `, + config + ); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + await expect(nativeButton).toHaveAttribute('aria-description', 'described'); + + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + parent.appendChild(el); + }); + await page.waitForChanges(); + + // The value captured at load survives the move. + await expect(nativeButton).toHaveAttribute('aria-description', 'described'); + + // Updates made after the move must still reach the native button. + await host.evaluate((el) => el.setAttribute('aria-description', 'updated')); + await expect(nativeButton).toHaveAttribute('aria-description', 'updated'); + + // So must one made while it was detached, when nothing is watching. + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + el.setAttribute('aria-description', 'while detached'); + parent.appendChild(el); + }); + await expect(nativeButton).toHaveAttribute('aria-description', 'while detached'); + }); + + test('should sync updates, empty values and removals after the initial copy', async ({ page }) => { + await page.setContent(`Button`, config); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + // The initial copy moves the value from the host to the native button. + await expect(host).not.toHaveAttribute('aria-description'); + await expect(nativeButton).toHaveAttribute('aria-description', 'initial'); + + // Post-load writes stay on the host and are copied to the native button. + await host.evaluate((el) => el.setAttribute('aria-description', 'second')); + await expect(host).toHaveAttribute('aria-description', 'second'); + await expect(nativeButton).toHaveAttribute('aria-description', 'second'); + + // An empty string is a valid ARIA attribute value. + await host.evaluate((el) => el.setAttribute('aria-description', '')); + await expect(nativeButton).toHaveAttribute('aria-description', ''); + + // A removal of a post-load write does reach the native button. + await host.evaluate((el) => el.removeAttribute('aria-description')); + await expect(host).not.toHaveAttribute('aria-description'); + await expect(nativeButton).not.toHaveAttribute('aria-description'); + }); + + test('should keep a value from the initial markup when the host attribute is removed', async ({ page }) => { + await page.setContent(`Button`, config); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); + + // The initial copy already took the attribute off the host, so removing it there + // changes nothing and the native button keeps the copied value. Setting an empty + // value is how you clear one of these. + await host.evaluate((el) => el.removeAttribute('aria-label')); + + // Force a render and wait for it, otherwise the assertion passes on a button that + // never re-rendered at all. + await host.evaluate((el: HTMLIonButtonElement) => (el.color = 'primary')); + await expect(host).toHaveClass(/ion-color-primary/); + + await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); + }); + }); +}); diff --git a/core/src/components/card/card.tsx b/core/src/components/card/card.tsx index 68e21d0ba5a..5572a82cdf4 100644 --- a/core/src/components/card/card.tsx +++ b/core/src/components/card/card.tsx @@ -1,8 +1,8 @@ import type { ComponentInterface } from '@stencil/core'; -import { Element, Component, Host, Prop, h } from '@stencil/core'; +import { Element, Component, Host, Prop, h, forceUpdate } from '@stencil/core'; +import type { AttributeController } from '@utils/attribute-controller'; +import { createAttributeController } from '@utils/attribute-controller'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes } from '@utils/helpers'; -import { inheritAttributes } from '@utils/helpers'; import { createColorClasses, openURL } from '@utils/theme'; import { getIonMode } from '../../global/ionic-global'; @@ -23,7 +23,7 @@ import type { RouterDirection } from '../router/utils/interface'; shadow: true, }) export class Card implements ComponentInterface, AnchorInterface, ButtonInterface { - private inheritedAriaAttributes: Attributes = {}; + private ariaController?: AttributeController; @Element() el!: HTMLElement; /** @@ -88,7 +88,20 @@ export class Card implements ComponentInterface, AnchorInterface, ButtonInterfac @Prop() target: string | undefined; componentWillLoad() { - this.inheritedAriaAttributes = inheritAttributes(this.el, ['aria-label']); + /** + * Only the initial copy takes the attribute off the host, so an `aria-label` written + * after load stays on the host too. That's harmless here, because unlike `ion-item` + * the card host renders no role of its own, so nothing reads the leftover copy. + */ + this.ariaController = createAttributeController(this.el, ['aria-label'], () => forceUpdate(this)); + } + + connectedCallback() { + this.ariaController?.init(); + } + + disconnectedCallback() { + this.ariaController?.destroy(); } private isClickable(): boolean { @@ -101,7 +114,8 @@ export class Card implements ComponentInterface, AnchorInterface, ButtonInterfac if (!clickable) { return []; } - const { href, routerAnimation, routerDirection, inheritedAriaAttributes } = this; + const { href, routerAnimation, routerDirection } = this; + const inheritedAriaAttributes = this.ariaController?.attributes ?? {}; const TagType = clickable ? (href === undefined ? 'button' : 'a') : ('div' as any); const attrs = TagType === 'button' diff --git a/core/src/components/card/test/a11y/card.e2e.ts b/core/src/components/card/test/a11y/card.e2e.ts index 6902037f998..7adb42ed5e7 100644 --- a/core/src/components/card/test/a11y/card.e2e.ts +++ b/core/src/components/card/test/a11y/card.e2e.ts @@ -32,3 +32,130 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); }); }); + +/** + * Attribute syncing does not vary across modes or directions + */ +configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('card: aria attribute sync'), () => { + test('should sync aria-label to the native element when it changes on the host', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + }); + + await page.setContent(`Card`, config); + + const host = page.locator('ion-card'); + const nativeCard = host.locator('[part="native"]'); + + await expect(nativeCard).toHaveAttribute('aria-label', 'label'); + + await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); + + await expect(nativeCard).toHaveAttribute('aria-label', 'updated'); + }); + + test('should keep syncing after the card is detached and reattached', async ({ page }) => { + await page.setContent( + ` +
+ Card +
+ `, + config + ); + + const host = page.locator('ion-card'); + const nativeCard = host.locator('[part="native"]'); + + await expect(nativeCard).toHaveAttribute('aria-label', 'label'); + + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + parent.appendChild(el); + }); + await page.waitForChanges(); + + // The value captured at load survives the move. + await expect(nativeCard).toHaveAttribute('aria-label', 'label'); + + // Updates made after the move must still reach the native element. + await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); + await expect(nativeCard).toHaveAttribute('aria-label', 'updated'); + + // So must one made while it was detached, when nothing is watching. + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + el.setAttribute('aria-label', 'while detached'); + parent.appendChild(el); + }); + await expect(nativeCard).toHaveAttribute('aria-label', 'while detached'); + }); + + test('should sync updates, empty values and removals after the initial copy', async ({ page }) => { + await page.setContent(`Card`, config); + + const host = page.locator('ion-card'); + const nativeCard = host.locator('[part="native"]'); + + // The initial copy moves the value from the host to the native element. + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeCard).toHaveAttribute('aria-label', 'initial'); + + // Post-load writes stay on the host and are copied to the native element. + await host.evaluate((el) => el.setAttribute('aria-label', 'second')); + await expect(host).toHaveAttribute('aria-label', 'second'); + await expect(nativeCard).toHaveAttribute('aria-label', 'second'); + + // An empty string is a valid ARIA attribute value. + await host.evaluate((el) => el.setAttribute('aria-label', '')); + await expect(nativeCard).toHaveAttribute('aria-label', ''); + + // A removal of a post-load write does reach the native element. + await host.evaluate((el) => el.removeAttribute('aria-label')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeCard).not.toHaveAttribute('aria-label'); + }); + + test('should apply aria-label to the native element when the card becomes clickable', async ({ page }) => { + await page.setContent(`Card`, config); + + const host = page.locator('ion-card'); + await page.waitForChanges(); + + // A card that is neither a button nor a link renders no native element. + await expect(host.locator('[part="native"]')).toHaveCount(0); + + // Both `button` and `href` can be set after load, and the native element that + // appears then still needs the label copied at load. + await host.evaluate((el: HTMLIonCardElement) => (el.button = true)); + + await expect(host.locator('[part="native"]')).toHaveAttribute('aria-label', 'label'); + }); + + test('should not sync ARIA attributes other than aria-label', async ({ page }) => { + await page.setContent(`Card`, config); + + const host = page.locator('ion-card'); + const nativeCard = host.locator('[part="native"]'); + + /** + * Only `aria-label` should reach the native element. A wider watch set would put + * attributes there after load that the element never gets at load. + */ + await host.evaluate((el) => { + el.setAttribute('role', 'presentation'); + el.setAttribute('aria-describedby', 'hint'); + // Written in the same batch as a barrier, since once it lands the sync has run. + el.setAttribute('aria-label', 'updated'); + }); + + await expect(nativeCard).toHaveAttribute('aria-label', 'updated'); + await expect(nativeCard).not.toHaveAttribute('aria-describedby'); + await expect(nativeCard).not.toHaveAttribute('role'); + }); + }); +}); diff --git a/core/src/components/item/item.tsx b/core/src/components/item/item.tsx index 45eea6867d3..fa7de421fc2 100644 --- a/core/src/components/item/item.tsx +++ b/core/src/components/item/item.tsx @@ -1,8 +1,9 @@ import type { ComponentInterface } from '@stencil/core'; import { Build, Component, Element, Host, Listen, Prop, State, Watch, forceUpdate, h } from '@stencil/core'; +import type { AttributeController } from '@utils/attribute-controller'; +import { createAttributeController } from '@utils/attribute-controller'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes } from '@utils/helpers'; -import { inheritAttributes, raf } from '@utils/helpers'; +import { raf } from '@utils/helpers'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; import { chevronForward } from 'ionicons/icons'; @@ -35,9 +36,9 @@ const INDICATOR_CONTROL_SELECTOR = 'ion-checkbox, ion-radio, ion-toggle'; export class Item implements ComponentInterface, AnchorInterface, ButtonInterface { private labelColorStyles = {}; private itemStyles = new Map(); - private inheritedAriaAttributes: Attributes = {}; private indicatorControlObserver?: MutationObserver; private didLoad = false; + private ariaController?: AttributeController; @Element() el!: HTMLIonItemElement; @@ -180,10 +181,18 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac this.watchForIndicatorControls(); this.updateInteractivityOnSlotChange(); } + + this.ariaController?.init(); } componentWillLoad() { - this.inheritedAriaAttributes = inheritAttributes(this.el, ['aria-label']); + /** + * Only the initial copy takes the attribute off the host, so an `aria-label` written + * after load names both the native element and the Host, which is a `listitem` when + * the item is in an `ion-list`. The two names always agree, so a screen reader just + * reads it twice. + */ + this.ariaController = createAttributeController(this.el, ['aria-label'], () => forceUpdate(this)); } componentDidLoad() { @@ -203,6 +212,8 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac this.indicatorControlObserver.disconnect(); this.indicatorControlObserver = undefined; } + + this.ariaController?.destroy(); } private totalNestedInputs() { @@ -355,9 +366,9 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac target, routerAnimation, routerDirection, - inheritedAriaAttributes, multipleInputs, } = this; + const inheritedAriaAttributes = this.ariaController?.attributes ?? {}; const childStyles = {} as StyleEventDetail; const mode = getIonMode(this); const clickable = this.isClickable(); diff --git a/core/src/components/item/test/a11y/item.e2e.ts b/core/src/components/item/test/a11y/item.e2e.ts index 20536beb71a..084e4d3b65e 100644 --- a/core/src/components/item/test/a11y/item.e2e.ts +++ b/core/src/components/item/test/a11y/item.e2e.ts @@ -153,3 +153,115 @@ configs({ directions: ['ltr'] }).forEach(({ config, screenshot, title }) => { }); }); }); + +/** + * Attribute syncing does not vary across modes or directions + */ +configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('item: aria attribute sync'), () => { + test('should sync aria-label to the native element when it changes on the host', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + }); + + await page.setContent(`Item`, config); + + const host = page.locator('ion-item'); + const nativeItem = host.locator('[part="native"]'); + + await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + + await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); + + await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); + }); + + test('should keep syncing after the item is detached and reattached', async ({ page }) => { + await page.setContent( + ` +
+ Item +
+ `, + config + ); + + const host = page.locator('ion-item'); + const nativeItem = host.locator('[part="native"]'); + + await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + parent.appendChild(el); + }); + await page.waitForChanges(); + + // The value captured at load survives the move. + await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + + // Updates made after the move must still reach the native element. + await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); + await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); + + // So must one made while it was detached, when nothing is watching. + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + el.setAttribute('aria-label', 'while detached'); + parent.appendChild(el); + }); + await expect(nativeItem).toHaveAttribute('aria-label', 'while detached'); + }); + + test('should sync updates, empty values and removals after the initial copy', async ({ page }) => { + await page.setContent(`Item`, config); + + const host = page.locator('ion-item'); + const nativeItem = host.locator('[part="native"]'); + + // The initial copy moves the value from the host to the native element. + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeItem).toHaveAttribute('aria-label', 'initial'); + + // Post-load writes stay on the host and are copied to the native element. + await host.evaluate((el) => el.setAttribute('aria-label', 'second')); + await expect(host).toHaveAttribute('aria-label', 'second'); + await expect(nativeItem).toHaveAttribute('aria-label', 'second'); + + // An empty string is a valid ARIA attribute value. + await host.evaluate((el) => el.setAttribute('aria-label', '')); + await expect(nativeItem).toHaveAttribute('aria-label', ''); + + // A removal of a post-load write does reach the native element. + await host.evaluate((el) => el.removeAttribute('aria-label')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeItem).not.toHaveAttribute('aria-label'); + }); + + test('should not sync ARIA attributes other than aria-label', async ({ page }) => { + await page.setContent(`Item`, config); + + const host = page.locator('ion-item'); + const nativeItem = host.locator('[part="native"]'); + + /** + * Only `aria-label` should reach the native element. An `ion-item` in a list renders + * `role="listitem"` on its own Host, so watching `role` would copy that onto the + * native button. + */ + await host.evaluate((el) => { + el.setAttribute('role', 'presentation'); + el.setAttribute('aria-describedby', 'hint'); + // Written in the same batch as a barrier, since once it lands the sync has run. + el.setAttribute('aria-label', 'updated'); + }); + + await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); + await expect(nativeItem).not.toHaveAttribute('aria-describedby'); + await expect(nativeItem).not.toHaveAttribute('role'); + }); + }); +}); diff --git a/core/src/utils/attribute-controller.ts b/core/src/utils/attribute-controller.ts new file mode 100644 index 00000000000..e27db66d1f7 --- /dev/null +++ b/core/src/utils/attribute-controller.ts @@ -0,0 +1,171 @@ +import { win } from '@utils/browser'; +import type { Attributes } from '@utils/helpers'; +import { ariaAttributes, inheritAttributes } from '@utils/helpers'; + +/** + * Copies a set of attributes off the host element so they can be applied to an element + * inside the shadow root, then keeps that copy in sync as the host attributes change. + * Using `inheritAttributes` alone copies once at load, so anything written to the host + * afterwards never reaches the element the assistive technology reads. + * + * Create the controller in `componentWillLoad`. It takes the initial copy and starts + * watching right away, so `init()` is only needed to resume after a move. + * + * Two things to know before adopting it. Only the initial copy removes the attributes + * from the host, so a value written after load sits on the host as well as on the + * element it is applied to, which matters if the host has a role of its own. And an + * attribute from the initial markup can never be removed, only overwritten, since the + * copy already took it off the host and a `removeAttribute` there fires no mutation. + * An empty string works for `aria-label` and friends, but a token-valued attribute + * has to be set to its default instead, so `aria-pressed="false"` rather than empty. + * + * @internal + * @param el The host element to copy from and watch. + * @param attributes The attributes to copy and watch. + * @param onChange Called after `attributes` changes, so the component can re-render. + * @param hostOwnedAttributes Attributes copied at load but not watched. Use it for + * attributes the component renders on its own ``, since a later change to one of + * those is the component's own write, and for attributes that would put a second node in + * the accessibility tree if the host kept a copy alongside the one we apply. + */ +export const createAttributeController = ( + el: HTMLElement, + attributes: string[], + onChange: () => void, + hostOwnedAttributes: string[] = [] +): AttributeController => { + let inherited: Attributes = inheritAttributes(el, attributes); + let observer: MutationObserver | undefined; + + const watchedAttributes = attributes.filter((attr) => !hostOwnedAttributes.includes(attr)); + + /** + * The names still on the host, which is everything written after load. The initial + * copy removes what it captures, so a missing attribute only counts as a removal + * when its name is in here. + */ + const hostWritten = new Set(); + + const setPresence = (name: string, value: string | null) => { + if (value === null) { + hostWritten.delete(name); + } else { + hostWritten.add(name); + } + }; + + /** + * Nothing is watching while the host is out of the tree, since `disconnectedCallback` + * calls `destroy()`, and `forceUpdate` is a no-op on a disconnected host anyway. So + * the host has to be re-read on the way back in. + */ + const readMissedChanges = () => { + const changed: Attributes = {}; + + for (const name of watchedAttributes) { + const value = el.getAttribute(name); + const wasOnHost = hostWritten.has(name); + + setPresence(name, value); + + /** + * An attribute that was never written to the host is missing because the initial + * copy took it, not because the developer cleared it. + */ + if ((value === null && !wasOnHost) || value === inherited[name]) { + continue; + } + + changed[name] = value; + } + + if (Object.keys(changed).length > 0) { + inherited = { ...inherited, ...changed }; + onChange(); + } + }; + + const init = () => { + // There is no MutationObserver in SSR or the hydrate build. + if (observer !== undefined || watchedAttributes.length === 0 || win === undefined || !('MutationObserver' in win)) { + return; + } + + readMissedChanges(); + + observer = new MutationObserver((mutations) => { + const changed: Attributes = {}; + + for (const mutation of mutations) { + const name = mutation.attributeName!; + // A removed attribute reads back as null, which clears it from the element the + // values are spread onto. + const value = el.getAttribute(name); + + setPresence(name, value); + changed[name] = value; + } + + inherited = { ...inherited, ...changed }; + onChange(); + }); + + observer.observe(el, { attributeFilter: watchedAttributes }); + }; + + const destroy = () => { + observer?.disconnect(); + observer = undefined; + }; + + /** + * Has to run after the initial copy, because that copy removes the attributes from the + * host and an observer armed any earlier would read the removal as a developer clearing + * them. + */ + init(); + + return { + get attributes() { + return inherited; + }, + init, + destroy, + }; +}; + +/** + * The `createAttributeController` equivalent of `inheritAriaAttributes`, which copies and + * watches every ARIA attribute plus `role`. + * + * @internal + * @param el The host element to copy from and watch. + * @param onChange Called after the attributes change, so the component can re-render. + * @param hostOwnedAttributes Attributes copied at load but not watched. See + * `createAttributeController`. + */ +export const createAriaAttributeController = ( + el: HTMLElement, + onChange: () => void, + hostOwnedAttributes: string[] = [] +): AttributeController => { + return createAttributeController(el, ariaAttributes, onChange, hostOwnedAttributes); +}; + +export type AttributeController = { + /** + * The attributes copied off the host element. Spread these onto the native element + * in `render()`. + */ + readonly attributes: Attributes; + /** + * Resumes watching the host element. Only needed from `connectedCallback`, because the + * controller can't just be recreated in `componentWillLoad` after a move once the + * initial copy has taken the attributes off the host. + */ + init: () => void; + /** + * Stops watching the host element. Call this from `disconnectedCallback`. + */ + destroy: () => void; +}; diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index 9c6052b466f..bc7447d9620 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -98,8 +98,8 @@ export type Attributes = { [key: string]: any }; * helper function should be called in componentWillLoad and assigned to a variable * that is later used in the render function. * - * This does not need to be reactive as changing attributes on the host element - * does not trigger a re-render. + * This copies once. Use `createAttributeController` instead when the attributes can + * change after load, since a host attribute change does not trigger a re-render. */ export const inheritAttributes = (el: HTMLElement, attributes: string[] = []) => { const attributeObject: Attributes = {}; @@ -121,8 +121,10 @@ export const inheritAttributes = (el: HTMLElement, attributes: string[] = []) => * List of available ARIA attributes + `role`. * Removed deprecated attributes. * https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes + * + * @internal Exported for `attribute-controller.ts`, which needs the same set. */ -const ariaAttributes = [ +export const ariaAttributes = [ 'role', 'aria-activedescendant', 'aria-atomic', diff --git a/core/src/utils/test/attribute-controller.spec.ts b/core/src/utils/test/attribute-controller.spec.ts new file mode 100644 index 00000000000..ee2b6afe713 --- /dev/null +++ b/core/src/utils/test/attribute-controller.spec.ts @@ -0,0 +1,286 @@ +import { win } from '@utils/browser'; + +import { createAttributeController, createAriaAttributeController } from '../attribute-controller'; + +/** + * There is no `MutationObserver` in mock-doc, so the observer half of the controller is + * only reachable from a spec through a stub. This one records what each instance was + * asked to observe and lets the test drive the callback by hand. + */ +class MockMutationObserver { + static instances: MockMutationObserver[] = []; + + observedFilter: string[] | undefined; + connected = false; + + constructor(private callback: (mutations: { attributeName: string }[]) => void) { + MockMutationObserver.instances.push(this); + } + + observe(_el: Node, options: { attributeFilter?: string[] }) { + this.observedFilter = options.attributeFilter; + this.connected = true; + } + + disconnect() { + this.connected = false; + } + + /** Stands in for the browser delivering an attribute mutation. */ + emit(...attributeNames: string[]) { + this.callback(attributeNames.map((attributeName) => ({ attributeName }))); + } + + static get live() { + return MockMutationObserver.instances.filter((instance) => instance.connected); + } + + static get latest() { + return MockMutationObserver.instances[MockMutationObserver.instances.length - 1]; + } +} + +/** + * The controller gates on `'MutationObserver' in win` but constructs the global, so the + * stub has to go in both places. mock-doc defines neither, so teardown deletes the keys + * instead of restoring a value, because that check has to read false again for the + * unavailable-environment test. + */ +const installMockMutationObserver = () => { + MockMutationObserver.instances = []; + (globalThis as any).MutationObserver = MockMutationObserver; + (win as any).MutationObserver = MockMutationObserver; +}; + +const restoreOverrides = () => { + delete (globalThis as any).MutationObserver; + delete (win as any).MutationObserver; +}; + +const createHost = (attributes: Record = {}) => { + const el = document.createElement('div'); + Object.entries(attributes).forEach(([name, value]) => el.setAttribute(name, value)); + return el; +}; + +describe('createAttributeController()', () => { + beforeEach(installMockMutationObserver); + + afterEach(restoreOverrides); + + it('should copy the attributes off the host on creation', () => { + const el = createHost({ 'aria-label': 'Save', 'aria-describedby': 'hint' }); + + const controller = createAttributeController(el, ['aria-label'], jest.fn()); + + expect(controller.attributes).toEqual({ 'aria-label': 'Save' }); + expect(el.hasAttribute('aria-label')).toBe(false); + // Attributes outside the set are left alone. + expect(el.getAttribute('aria-describedby')).toBe('hint'); + }); + + it('should start watching on creation', () => { + createAttributeController(createHost(), ['aria-label'], jest.fn()); + + expect(MockMutationObserver.live).toHaveLength(1); + expect(MockMutationObserver.latest.observedFilter).toEqual(['aria-label']); + }); + + it('should apply a later attribute change', () => { + const el = createHost({ 'aria-label': 'Save' }); + const onChange = jest.fn(); + const controller = createAttributeController(el, ['aria-label'], onChange); + + el.setAttribute('aria-label', 'Submit'); + MockMutationObserver.latest.emit('aria-label'); + + expect(controller.attributes).toEqual({ 'aria-label': 'Submit' }); + expect(onChange).toHaveBeenCalledTimes(1); + }); + + it('should clear an attribute that is removed after load', () => { + const el = createHost(); + const controller = createAttributeController(el, ['aria-label'], jest.fn()); + + el.setAttribute('aria-label', 'Save'); + MockMutationObserver.latest.emit('aria-label'); + el.removeAttribute('aria-label'); + MockMutationObserver.latest.emit('aria-label'); + + expect(controller.attributes['aria-label']).toBeNull(); + }); + + it('should not watch host owned attributes', () => { + const el = createHost({ 'aria-label': 'Save', 'aria-disabled': 'true' }); + + const controller = createAttributeController(el, ['aria-label', 'aria-disabled'], jest.fn(), ['aria-disabled']); + + // Still copied at load, just never watched afterwards. + expect(controller.attributes).toEqual({ 'aria-label': 'Save', 'aria-disabled': 'true' }); + expect(MockMutationObserver.latest.observedFilter).toEqual(['aria-label']); + }); + + it('should not observe when every attribute is host owned', () => { + createAttributeController(createHost(), ['aria-disabled'], jest.fn(), ['aria-disabled']); + + expect(MockMutationObserver.instances).toHaveLength(0); + }); + + it('should stop watching on destroy', () => { + const controller = createAttributeController(createHost(), ['aria-label'], jest.fn()); + + controller.destroy(); + + expect(MockMutationObserver.live).toHaveLength(0); + }); + + it('should not stack observers when init is called while already watching', () => { + const controller = createAttributeController(createHost(), ['aria-label'], jest.fn()); + + controller.init(); + + expect(MockMutationObserver.live).toHaveLength(1); + }); + + it('should resume watching after destroy', () => { + const el = createHost(); + const onChange = jest.fn(); + const controller = createAttributeController(el, ['aria-label'], onChange); + + controller.destroy(); + controller.init(); + + el.setAttribute('aria-label', 'Submit'); + MockMutationObserver.latest.emit('aria-label'); + + expect(controller.attributes).toEqual({ 'aria-label': 'Submit' }); + }); + + it('should pick up a change made while not watching', () => { + const el = createHost(); + const onChange = jest.fn(); + const controller = createAttributeController(el, ['aria-label'], onChange); + + controller.destroy(); + el.setAttribute('aria-label', 'Submit'); + controller.init(); + + expect(controller.attributes).toEqual({ 'aria-label': 'Submit' }); + expect(onChange).toHaveBeenCalledTimes(1); + }); + + it('should clear an attribute removed while not watching', () => { + const el = createHost(); + const controller = createAttributeController(el, ['aria-label'], jest.fn()); + + el.setAttribute('aria-label', 'Submit'); + MockMutationObserver.latest.emit('aria-label'); + + controller.destroy(); + el.removeAttribute('aria-label'); + controller.init(); + + expect(controller.attributes['aria-label']).toBeNull(); + }); + + it('should track a detached write of an unchanged value, so a later removal is seen', () => { + const el = createHost({ 'aria-label': 'Save' }); + const controller = createAttributeController(el, ['aria-label'], jest.fn()); + + // Re-setting the value the copy already captured puts the attribute back on the + // host, even though nothing about the rendered value changed. + controller.destroy(); + el.setAttribute('aria-label', 'Save'); + controller.init(); + + controller.destroy(); + el.removeAttribute('aria-label'); + controller.init(); + + expect(controller.attributes['aria-label']).toBeNull(); + }); + + it('should not re-render when a detached write matches the current value', () => { + const el = createHost(); + const onChange = jest.fn(); + const controller = createAttributeController(el, ['aria-label'], onChange); + + el.setAttribute('aria-label', 'Save'); + MockMutationObserver.latest.emit('aria-label'); + onChange.mockClear(); + + controller.destroy(); + controller.init(); + + expect(onChange).not.toHaveBeenCalled(); + }); + + it('should apply every attribute in one mutation batch with a single re-render', () => { + const el = createHost(); + const onChange = jest.fn(); + const controller = createAttributeController(el, ['aria-label', 'aria-description'], onChange); + + el.setAttribute('aria-label', 'Save'); + el.setAttribute('aria-description', 'Saves the draft'); + MockMutationObserver.latest.emit('aria-label', 'aria-description'); + + expect(controller.attributes).toEqual({ 'aria-label': 'Save', 'aria-description': 'Saves the draft' }); + expect(onChange).toHaveBeenCalledTimes(1); + }); + + it('should still copy when MutationObserver is unavailable', () => { + // Neither the hydrate build nor mock-doc has a `MutationObserver`. + restoreOverrides(); + const el = createHost({ 'aria-label': 'Save' }); + + const controller = createAttributeController(el, ['aria-label'], jest.fn()); + + expect(controller.attributes).toEqual({ 'aria-label': 'Save' }); + expect(() => { + controller.init(); + controller.destroy(); + }).not.toThrow(); + }); + + it('should keep the initial copy when resuming, since the copy removed it from the host', () => { + const el = createHost({ 'aria-label': 'Save' }); + const onChange = jest.fn(); + const controller = createAttributeController(el, ['aria-label'], onChange); + + controller.destroy(); + controller.init(); + + expect(controller.attributes).toEqual({ 'aria-label': 'Save' }); + expect(onChange).not.toHaveBeenCalled(); + }); +}); + +describe('createAriaAttributeController()', () => { + beforeEach(installMockMutationObserver); + + afterEach(restoreOverrides); + + it('should copy and watch every aria attribute and role', () => { + const el = document.createElement('div'); + el.setAttribute('aria-label', 'Save'); + el.setAttribute('role', 'button'); + el.setAttribute('title', 'not aria'); + + const controller = createAriaAttributeController(el, jest.fn()); + + expect(controller.attributes).toEqual({ 'aria-label': 'Save', role: 'button' }); + expect(el.getAttribute('title')).toBe('not aria'); + expect(MockMutationObserver.latest.observedFilter).toContain('aria-description'); + expect(MockMutationObserver.latest.observedFilter).toContain('role'); + }); + + it('should exclude host owned attributes from the watch only', () => { + const el = document.createElement('div'); + el.setAttribute('aria-disabled', 'true'); + + const controller = createAriaAttributeController(el, jest.fn(), ['aria-disabled']); + + expect(controller.attributes).toEqual({ 'aria-disabled': 'true' }); + expect(MockMutationObserver.latest.observedFilter).not.toContain('aria-disabled'); + }); +}); diff --git a/docs/component-guide.md b/docs/component-guide.md index 26cdf874e8c..f685fd6163d 100644 --- a/docs/component-guide.md +++ b/docs/component-guide.md @@ -465,24 +465,41 @@ render() { Labels should be passed directly to the component in the form of either visible text or an `aria-label`. The visible text can be set inside of a `label` element, and the `aria-label` can be set directly on the interactive element. -In the following example the `aria-label` can be inherited from the Host using the `inheritAttributes` or `inheritAriaAttributes` utilities. This allows developers to set `aria-label` on the host element since they do not have access to inside the shadow root. +In the following example the `aria-label` is copied from the Host using `createAttributeController`. This allows developers to set `aria-label` on the host element since they do not have access to inside the shadow root. > [!NOTE] -> Use `inheritAttributes` to specify which attributes should be inherited or `inheritAriaAttributes` to inherit all of the possible `aria` attributes. +> Use `createAttributeController` to specify which attributes should be copied or `createAriaAttributeController` to copy all of the possible `aria` attributes. + +The controller keeps the copy in sync when the host attribute changes after load. Both `inheritAttributes` and `inheritAriaAttributes` do the same copy but only once, so a change made after load never reaches the native element. Those are still the right choice for an attribute that is only read at load. + +> [!IMPORTANT] +> Pass `hostOwnedAttributes`, the last argument of either function, for any attribute that should be copied at load but not watched afterwards. There are two cases. One is an attribute the component renders on its own ``, like the `aria-disabled` that `ion-button` renders from its `disabled` prop, where watching it would let the component's own renders overwrite a developer's value. The other is an attribute that would put a second node in the accessibility tree if the host kept a copy, like `role`, since only the initial copy removes attributes from the host and anything written after load stays there too. + +> [!NOTE] +> Attributes that reference an element by ID (`aria-labelledby`, `aria-describedby`, `aria-controls`, `aria-owns`, `aria-activedescendant`) cannot resolve a light DOM ID from inside the shadow root, so only copy those when the target is in the same tree. ```tsx -import { Prop } from '@stencil/core'; -import { inheritAttributes } from '@utils/helpers'; -import type { Attributes } from '@utils/helpers'; +import { Prop, forceUpdate } from '@stencil/core'; +import { createAttributeController } from '@utils/attribute-controller'; +import type { AttributeController } from '@utils/attribute-controller'; ... -private inheritedAttributes: Attributes = {}; +private ariaController?: AttributeController; @Prop() labelText?: string; componentWillLoad() { - this.inheritedAttributes = inheritAttributes(this.el, ['aria-label']); + this.ariaController = createAttributeController(this.el, ['aria-label'], () => forceUpdate(this)); +} + +connectedCallback() { + // componentWillLoad does not run again when the host is moved. + this.ariaController?.init(); +} + +disconnectedCallback() { + this.ariaController?.destroy(); } render() { @@ -490,7 +507,7 @@ render() { ) @@ -578,24 +595,41 @@ render() { Labels should be passed directly to the component in the form of either visible text or an `aria-label`. The visible text can be set inside of a `label` element, and the `aria-label` can be set directly on the interactive element. -In the following example the `aria-label` can be inherited from the Host using the `inheritAttributes` or `inheritAriaAttributes` utilities. This allows developers to set `aria-label` on the host element since they do not have access to inside the shadow root. +In the following example the `aria-label` is copied from the Host using `createAttributeController`. This allows developers to set `aria-label` on the host element since they do not have access to inside the shadow root. > [!NOTE] -> Use `inheritAttributes` to specify which attributes should be inherited or `inheritAriaAttributes` to inherit all of the possible `aria` attributes. +> Use `createAttributeController` to specify which attributes should be copied or `createAriaAttributeController` to copy all of the possible `aria` attributes. + +The controller keeps the copy in sync when the host attribute changes after load. Both `inheritAttributes` and `inheritAriaAttributes` do the same copy but only once, so a change made after load never reaches the native element. Those are still the right choice for an attribute that is only read at load. + +> [!IMPORTANT] +> Pass `hostOwnedAttributes`, the last argument of either function, for any attribute that should be copied at load but not watched afterwards. There are two cases. One is an attribute the component renders on its own ``, like the `aria-disabled` that `ion-button` renders from its `disabled` prop, where watching it would let the component's own renders overwrite a developer's value. The other is an attribute that would put a second node in the accessibility tree if the host kept a copy, like `role`, since only the initial copy removes attributes from the host and anything written after load stays there too. + +> [!NOTE] +> Attributes that reference an element by ID (`aria-labelledby`, `aria-describedby`, `aria-controls`, `aria-owns`, `aria-activedescendant`) cannot resolve a light DOM ID from inside the shadow root, so only copy those when the target is in the same tree. ```tsx -import { Prop } from '@stencil/core'; -import { inheritAttributes } from '@utils/helpers'; -import type { Attributes } from '@utils/helpers'; +import { Prop, forceUpdate } from '@stencil/core'; +import { createAttributeController } from '@utils/attribute-controller'; +import type { AttributeController } from '@utils/attribute-controller'; ... -private inheritedAttributes: Attributes = {}; +private ariaController?: AttributeController; @Prop() labelText?: string; componentWillLoad() { - this.inheritedAttributes = inheritAttributes(this.el, ['aria-label']); + this.ariaController = createAttributeController(this.el, ['aria-label'], () => forceUpdate(this)); +} + +connectedCallback() { + // componentWillLoad does not run again when the host is moved. + this.ariaController?.init(); +} + +disconnectedCallback() { + this.ariaController?.destroy(); } render() { @@ -603,7 +637,7 @@ render() { )