From 456c161cb7cc5de9ac3836add32f87fe8d916f9a Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Fri, 10 Jul 2026 16:53:53 -0500 Subject: [PATCH 01/14] fix(button): sync aria description between host and native button Adds @Watch('aria-description') to button.tsx before onAriaChanged --- core/src/components/button/button.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/components/button/button.tsx b/core/src/components/button/button.tsx index a1e7f72bf01..59e46bd2a3a 100644 --- a/core/src/components/button/button.tsx +++ b/core/src/components/button/button.tsx @@ -171,6 +171,7 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf @Watch('aria-checked') @Watch('aria-label') @Watch('aria-pressed') + @Watch('aria-description') onAriaChanged(newValue: string, _oldValue: string, propName: string) { this.inheritedAttributes = { ...this.inheritedAttributes, From d2b73eedb2406748585b27fce53b6f9b8602f8e1 Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Fri, 10 Jul 2026 16:57:48 -0500 Subject: [PATCH 02/14] test(button): add e2e test for aria-description sync Set aria description and both buttons should match. Update aria description on host button, and both buttons should still match. --- .../components/button/test/a11y/button.e2e.ts | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/core/src/components/button/test/a11y/button.e2e.ts b/core/src/components/button/test/a11y/button.e2e.ts index 585c0b5853d..fb712c99abe 100644 --- a/core/src/components/button/test/a11y/button.e2e.ts +++ b/core/src/components/button/test/a11y/button.e2e.ts @@ -148,3 +148,32 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); }); }); + +configs({ directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('button: aria description updates'), () => { + test('native button updates aria-description when host attribute changes', 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('aria-description', '0'); + + await host.evaluate((el) => { + el.setAttribute('aria-description', '1'); + }); + + await expect(nativeButton).toHaveAttribute('aria-description', '1'); + }); + }); +}); From 1d0e0161c50c8318742b2f18efd0160032ffd284 Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Sat, 25 Jul 2026 16:12:11 -0500 Subject: [PATCH 03/14] fix(button): sync aria attributes to native button reactively Previously, ARIA attributes inherited from the host were only captured once at componentWillLoad. Attributes set or changed after initial load (e.g. by ion-input-password-toggle updating aria-label/aria-pressed as visibility toggles) were not reflected onto the native button, causing screen readers to announce stale values unless a watch decorator was used for each attribute. Adds watchAttributes/watchForAriaAttributeChanges to helpers.ts, which use a MutationObserver to keep inherited ARIA attributes in sync for the lifetime of the component. Replaces the previous per-attribute @Watch decorators with this more general mechanism. Update Button.tsx to reflect this and use these new helpers. Add tests to test syncing all attributes. Fixes #30626 --- core/src/components/button/button.tsx | 51 ++++++------- .../components/button/test/a11y/button.e2e.ts | 41 ++++++----- core/src/utils/helpers.ts | 72 ++++++++++++++++++- 3 files changed, 121 insertions(+), 43 deletions(-) diff --git a/core/src/components/button/button.tsx b/core/src/components/button/button.tsx index 59e46bd2a3a..af3886277e4 100644 --- a/core/src/components/button/button.tsx +++ b/core/src/components/button/button.tsx @@ -2,7 +2,7 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core'; import { Component, Element, Event, Host, Prop, Watch, State, forceUpdate, h } from '@stencil/core'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; import type { Attributes } from '@utils/helpers'; -import { inheritAriaAttributes, hasShadowDom } from '@utils/helpers'; +import { inheritAriaAttributes, hasShadowDom, watchForAriaAttributeChanges, type AttributeWatcher } from '@utils/helpers'; import { printIonWarning } from '@utils/logging'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; @@ -35,6 +35,7 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf private formButtonEl: HTMLButtonElement | null = null; private formEl: HTMLFormElement | null = null; private inheritedAttributes: Attributes = {}; + private ariaWatcher?: AttributeWatcher; @Element() el!: HTMLElement; @@ -158,28 +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') - @Watch('aria-description') - 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 @@ -221,7 +200,31 @@ 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); + this.inheritedAttributes = inheritAriaAttributes(this.el, ['aria-disabled']); + + /** + * Keeps inherited ARIA attributes in sync with the host element for the + * lifetime of the component, not just at initial load. This replaces the + * previous approach of manually re-declaring @Watch for each aria attribute + * that could change post-load + * + * aria-disabled is excluded here (and from the initial inheritAriaAttributes + * call above) because button.tsx sets it itself on Host based on the `disabled` prop + */ + this.ariaWatcher = watchForAriaAttributeChanges( + this.el, + (changed) => { + this.inheritedAttributes = { ...this.inheritedAttributes, ...changed }; + forceUpdate(this); + }, + ['aria-disabled'] + ); + } + + // Prevents + disconnectedCallback() { + this.ariaWatcher?.disconnect(); + this.ariaWatcher = undefined; } private get hasIconOnly() { diff --git a/core/src/components/button/test/a11y/button.e2e.ts b/core/src/components/button/test/a11y/button.e2e.ts index fb712c99abe..6cdd065828f 100644 --- a/core/src/components/button/test/a11y/button.e2e.ts +++ b/core/src/components/button/test/a11y/button.e2e.ts @@ -1,5 +1,6 @@ import AxeBuilder from '@axe-core/playwright'; import { expect } from '@playwright/test'; +import { ariaAttributes } from '@utils/helpers'; import { configs, test } from '@utils/test/playwright'; configs({ directions: ['ltr'], palettes: ['light', 'dark'] }).forEach(({ title, config }) => { @@ -150,30 +151,34 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); configs({ directions: ['ltr'] }).forEach(({ title, config }) => { - test.describe(title('button: aria description updates'), () => { - test('native button updates aria-description when host attribute changes', async ({ page }) => { - test.info().annotations.push({ - type: 'issue', - description: 'https://github.com/ionic-team/ionic-framework/issues/30626', - }); + test.describe(title('button: aria attribute sync'), () => { + // Mirrors the ignoreList passed to inheritAriaAttributes/watchForAriaAttributeChanges + // in button.tsx. aria-disabled is excluded because button.tsx manages it internally + // via the `disabled` prop. + const watchedAriaAttributes = ariaAttributes.filter((attr) => attr !== 'aria-disabled'); - await page.setContent( - ` - Button - `, - config - ); + for (const attr of watchedAriaAttributes) { + test(`native button updates ${attr} when host attribute changes`, async ({ page }) => { + await page.setContent(`Button`, config); - const host = page.locator('ion-button'); - const nativeButton = host.locator('button'); + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + await expect(nativeButton).toHaveAttribute(attr, 'initial'); - await expect(nativeButton).toHaveAttribute('aria-description', '0'); + await host.evaluate((el, attr) => el.setAttribute(attr, 'updated'), attr); - await host.evaluate((el) => { - el.setAttribute('aria-description', '1'); + await expect(nativeButton).toHaveAttribute(attr, 'updated'); }); + } + + test('does not sync aria-disabled, since button.tsx manages it internally', async ({ page }) => { + await page.setContent(`Button`, config); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); - await expect(nativeButton).toHaveAttribute('aria-description', '1'); + await expect(nativeButton).not.toHaveAttribute('aria-disabled', 'true'); }); }); }); diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index 9c6052b466f..a0703ff35fd 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -122,7 +122,7 @@ export const inheritAttributes = (el: HTMLElement, attributes: string[] = []) => * Removed deprecated attributes. * https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes */ -const ariaAttributes = [ +export const ariaAttributes = [ 'role', 'aria-activedescendant', 'aria-atomic', @@ -191,6 +191,76 @@ export const inheritAriaAttributes = (el: HTMLElement, ignoreList?: string[]) => return inheritAttributes(el, attributesToInherit); }; +export interface AttributeWatcher { + disconnect: () => void; +} + +/** + * Watches an element for changes to a given set of attributes and calls + * onChange whenever one of them is set. Because inheritAttributes() strips + * the attribute from the host as it reads it, any subsequent mutation is + * just checked that the new value isn't null. + */ +export const watchAttributes = ( + el: HTMLElement, + attributes: string[], + onChange: (changed: { [k: string]: string }) => void +): AttributeWatcher => { + if (typeof MutationObserver === 'undefined') { + // Not available in Stencil's mock-doc test environment (used by + // `stencil test --spec`), and, as a defensive fallback, environments + // without native MutationObserver support. + return { disconnect: () => {} }; + } + + // Set up mutation observer to observe attribute changes + const observer = new MutationObserver((mutations) => { + const changed: { [k: string]: string } = {}; + for (const mutation of mutations) { + if (mutation.type !== 'attributes' || !mutation.attributeName) continue; + const name = mutation.attributeName; + if (!attributes.includes(name)) continue; + const value = el.getAttribute(name); + if (value === null) continue; + changed[name] = value; + } + + // If attribute changes, re-strip so the value doesn't live on both host + // and native element. + if (Object.keys(changed).length > 0) { + Object.keys(changed).forEach((name) => el.removeAttribute(name)); + onChange(changed); + } + }); + + // Watch for attribute changes on this element + observer.observe(el, { attributes: true, attributeFilter: attributes }); + + // Stop watching, called by `disconnectedCallback` + return { disconnect: () => observer.disconnect() }; +}; + +/** + * Watches an element for changes to ARIA attributes (and `role`) and invokes + * a callback whenever one is set externally, so that inherited ARIA state + * stays in sync for the lifetime of the component — not just at initial load. + * + * This should be called once in componentWillLoad, alongside the initial + * call to inheritAriaAttributes, and the returned AttributeWatcher must be + * disconnected in disconnectedCallback to avoid leaking the observer. + */ +export const watchForAriaAttributeChanges = ( + el: HTMLElement, + onChange: (changed: { [k: string]: string }) => void, + ignoreList?: string[] +): AttributeWatcher => { + let attributesToWatch = ariaAttributes; + if (ignoreList && ignoreList.length > 0) { + attributesToWatch = attributesToWatch.filter((attr) => !ignoreList.includes(attr)); + } + return watchAttributes(el, attributesToWatch, onChange); +}; + export const addEventListener = (el: any, eventName: string, callback: any, opts?: any) => { return el.addEventListener(eventName, callback, opts); }; From 7210eff90b6bae1e93f5389de7b25c509ca5360f Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Mon, 27 Jul 2026 09:09:01 -0500 Subject: [PATCH 04/14] npm run lint.fix --- core/src/components/button/button.tsx | 7 ++++++- core/src/utils/helpers.ts | 16 ++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/core/src/components/button/button.tsx b/core/src/components/button/button.tsx index af3886277e4..94c29c16bbc 100644 --- a/core/src/components/button/button.tsx +++ b/core/src/components/button/button.tsx @@ -2,7 +2,12 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core'; import { Component, Element, Event, Host, Prop, Watch, State, forceUpdate, h } from '@stencil/core'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; import type { Attributes } from '@utils/helpers'; -import { inheritAriaAttributes, hasShadowDom, watchForAriaAttributeChanges, type AttributeWatcher } from '@utils/helpers'; +import { + inheritAriaAttributes, + hasShadowDom, + watchForAriaAttributeChanges, + type AttributeWatcher, +} from '@utils/helpers'; import { printIonWarning } from '@utils/logging'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index a0703ff35fd..d16a88fdcb0 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -206,13 +206,13 @@ export const watchAttributes = ( attributes: string[], onChange: (changed: { [k: string]: string }) => void ): AttributeWatcher => { - if (typeof MutationObserver === 'undefined') { - // Not available in Stencil's mock-doc test environment (used by - // `stencil test --spec`), and, as a defensive fallback, environments - // without native MutationObserver support. - return { disconnect: () => {} }; - } - + if (typeof MutationObserver === 'undefined') { + // Not available in Stencil's mock-doc test environment (used by + // `stencil test --spec`), and, as a defensive fallback, environments + // without native MutationObserver support. + return { disconnect: () => {} }; + } + // Set up mutation observer to observe attribute changes const observer = new MutationObserver((mutations) => { const changed: { [k: string]: string } = {}; @@ -225,7 +225,7 @@ export const watchAttributes = ( changed[name] = value; } - // If attribute changes, re-strip so the value doesn't live on both host + // If attribute changes, re-strip so the value doesn't live on both host // and native element. if (Object.keys(changed).length > 0) { Object.keys(changed).forEach((name) => el.removeAttribute(name)); From c014ca59ca1a2569f5d41416ed8ad46308554fb7 Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Wed, 5 Aug 2026 11:35:02 -0500 Subject: [PATCH 05/14] fix(helper): update Mutation Observer and add removeAttribute intercept to helper Change disconnect to destroy to match other ionic conventions Update onChange to accept null values Add support for removeAttribute, including if null values triggered --- core/src/utils/helpers.ts | 44 ++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index d16a88fdcb0..643da0d90e2 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -192,7 +192,7 @@ export const inheritAriaAttributes = (el: HTMLElement, ignoreList?: string[]) => }; export interface AttributeWatcher { - disconnect: () => void; + destroy: () => void; } /** @@ -204,15 +204,22 @@ export interface AttributeWatcher { export const watchAttributes = ( el: HTMLElement, attributes: string[], - onChange: (changed: { [k: string]: string }) => void + onChange: (changed: { [k: string]: string | null }) => void ): AttributeWatcher => { if (typeof MutationObserver === 'undefined') { // Not available in Stencil's mock-doc test environment (used by // `stencil test --spec`), and, as a defensive fallback, environments // without native MutationObserver support. - return { disconnect: () => {} }; + return { destroy: () => {} }; } + // Keep a reference to the browser's original implementation. + // removeAttribute is patched below because MutationObserver cannot + // observe removeAttribute() calls once inheritAttributes() has + // already stripped the attribute from the host. In that case the + // browser performs no DOM mutation and emits no MutationRecord. + const originalRemoveAttribute = el.removeAttribute.bind(el); + // Set up mutation observer to observe attribute changes const observer = new MutationObserver((mutations) => { const changed: { [k: string]: string } = {}; @@ -224,11 +231,12 @@ export const watchAttributes = ( if (value === null) continue; changed[name] = value; } - - // If attribute changes, re-strip so the value doesn't live on both host - // and native element. if (Object.keys(changed).length > 0) { - Object.keys(changed).forEach((name) => el.removeAttribute(name)); + // Use the original implementation here. Calling the patched + // removeAttribute would recursively invoke onChange() with + // { [name]: null }, even though we are only stripping the host + // after synchronizing a new value. + Object.keys(changed).forEach((name) => originalRemoveAttribute(name)); onChange(changed); } }); @@ -236,8 +244,24 @@ export const watchAttributes = ( // Watch for attribute changes on this element observer.observe(el, { attributes: true, attributeFilter: attributes }); - // Stop watching, called by `disconnectedCallback` - return { disconnect: () => observer.disconnect() }; + // Intercept removeAttribute so we can notify consumers when an + // already-synced attribute is explicitly cleared. + el.removeAttribute = (name: string) => { + if (attributes.includes(name)) { + originalRemoveAttribute(name); + onChange({ [name]: null }); + return; + } + originalRemoveAttribute(name); + }; + + // Stop watching. Call this from `disconnectedCallback`. + return { + destroy: () => { + observer.disconnect(); + el.removeAttribute = originalRemoveAttribute; + }, + }; }; /** @@ -251,7 +275,7 @@ export const watchAttributes = ( */ export const watchForAriaAttributeChanges = ( el: HTMLElement, - onChange: (changed: { [k: string]: string }) => void, + onChange: (changed: { [k: string]: string | null }) => void, ignoreList?: string[] ): AttributeWatcher => { let attributesToWatch = ariaAttributes; From dab84cb002b5f22ff2a933f7f798aab28c376f43 Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Wed, 5 Aug 2026 11:37:40 -0500 Subject: [PATCH 06/14] fix(button): Add helper to ion-item and ion-card, move inheritedAriaAttributes to connectedCallback Import helper to ion-item and ion-card move inheritedAriaAttributes to connectedCallback in these components to preserve helper call order --- core/src/components/button/button.tsx | 22 +++-- core/src/components/card/card.tsx | 19 +++- core/src/components/item/item.tsx | 120 ++++---------------------- 3 files changed, 49 insertions(+), 112 deletions(-) diff --git a/core/src/components/button/button.tsx b/core/src/components/button/button.tsx index 94c29c16bbc..67daa2480d1 100644 --- a/core/src/components/button/button.tsx +++ b/core/src/components/button/button.tsx @@ -205,17 +205,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'); + } + + connectedCallback() { + /** + * Must run before watchForAriaAttributeChanges: it calls removeAttribute + * internally to strip the host's initial values, and that call must + * happen before removeAttribute is patched below — otherwise this + * strip would itself be treated as an external removal. + */ this.inheritedAttributes = inheritAriaAttributes(this.el, ['aria-disabled']); /** * Keeps inherited ARIA attributes in sync with the host element for the - * lifetime of the component, not just at initial load. This replaces the - * previous approach of manually re-declaring @Watch for each aria attribute - * that could change post-load - * - * aria-disabled is excluded here (and from the initial inheritAriaAttributes - * call above) because button.tsx sets it itself on Host based on the `disabled` prop + * lifetime of the component, not just at initial load. `aria-disabled` is excluded here + * (and from the initial inheritAriaAttributes call above) because button.tsx sets + * it itself on Host based on the `disabled` prop. */ + this.ariaWatcher = watchForAriaAttributeChanges( this.el, (changed) => { @@ -226,9 +233,8 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf ); } - // Prevents disconnectedCallback() { - this.ariaWatcher?.disconnect(); + this.ariaWatcher?.destroy(); this.ariaWatcher = undefined; } diff --git a/core/src/components/card/card.tsx b/core/src/components/card/card.tsx index 68e21d0ba5a..17510a5eeb1 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 { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes } from '@utils/helpers'; -import { inheritAttributes } from '@utils/helpers'; +import type { Attributes, AttributeWatcher } from '@utils/helpers'; +import { inheritAttributes, watchAttributes } from '@utils/helpers'; import { createColorClasses, openURL } from '@utils/theme'; import { getIonMode } from '../../global/ionic-global'; @@ -24,6 +24,7 @@ import type { RouterDirection } from '../router/utils/interface'; }) export class Card implements ComponentInterface, AnchorInterface, ButtonInterface { private inheritedAriaAttributes: Attributes = {}; + private ariaWatcher?: AttributeWatcher; @Element() el!: HTMLElement; /** @@ -91,6 +92,18 @@ export class Card implements ComponentInterface, AnchorInterface, ButtonInterfac this.inheritedAriaAttributes = inheritAttributes(this.el, ['aria-label']); } + connectedCallback() { + this.ariaWatcher = watchAttributes(this.el, ['aria-label'], (changed) => { + this.inheritedAriaAttributes = { ...this.inheritedAriaAttributes, ...changed }; + forceUpdate(this); + }); + } + + disconnectedCallback() { + this.ariaWatcher?.destroy(); + this.ariaWatcher = undefined; + } + private isClickable(): boolean { return this.href !== undefined || this.button; } diff --git a/core/src/components/item/item.tsx b/core/src/components/item/item.tsx index 45eea6867d3..0916f62e825 100644 --- a/core/src/components/item/item.tsx +++ b/core/src/components/item/item.tsx @@ -1,8 +1,8 @@ import type { ComponentInterface } from '@stencil/core'; -import { Build, Component, Element, Host, Listen, Prop, State, Watch, forceUpdate, h } from '@stencil/core'; +import { Component, Element, Host, Listen, Prop, State, Watch, forceUpdate, h } from '@stencil/core'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes } from '@utils/helpers'; -import { inheritAttributes, raf } from '@utils/helpers'; +import type { Attributes, AttributeWatcher } from '@utils/helpers'; +import { inheritAttributes, watchAttributes, raf } from '@utils/helpers'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; import { chevronForward } from 'ionicons/icons'; @@ -10,8 +10,6 @@ import { getIonMode } from '../../global/ionic-global'; import type { AnimationBuilder, Color, CssClassMap, StyleEventDetail } from '../../interface'; import type { RouterDirection } from '../router/utils/interface'; -const INDICATOR_CONTROL_SELECTOR = 'ion-checkbox, ion-radio, ion-toggle'; - /** * @virtualProp {"ios" | "md"} mode - The mode determines which platform styles to use. * @@ -36,15 +34,13 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac private labelColorStyles = {}; private itemStyles = new Map(); private inheritedAriaAttributes: Attributes = {}; - private indicatorControlObserver?: MutationObserver; - private didLoad = false; + private ariaWatcher?: AttributeWatcher; @Element() el!: HTMLIonItemElement; @State() multipleInputs = false; @State() focusable = true; @State() isInteractive = false; - @State() hasSlottedIndicatorControl = false; /** * The color to use from your application's color palette. @@ -169,40 +165,34 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac } } + componentWillLoad() {} + connectedCallback() { this.hasStartEl(); - /** - * `componentDidLoad` doesn't run again when the item is moved, so re-arm the - * observer and re-read the light DOM, which may have changed while detached. - */ - if (this.didLoad) { - this.watchForIndicatorControls(); - this.updateInteractivityOnSlotChange(); - } + // Must run before watchForAriaAttributeChanges: it calls removeAttribute + // internally to strip the host's initial values, and that call must + // happen before removeAttribute is patched below — otherwise this + // strip would itself be treated as an external removal. + this.inheritedAriaAttributes = inheritAttributes(this.el, ['aria-label']); + + this.ariaWatcher = watchAttributes(this.el, ['aria-label'], (changed) => { + this.inheritedAriaAttributes = { ...this.inheritedAriaAttributes, ...changed }; + forceUpdate(this); + }); } - componentWillLoad() { - this.inheritedAriaAttributes = inheritAttributes(this.el, ['aria-label']); + disconnectedCallback() { + this.ariaWatcher?.destroy(); + this.ariaWatcher = undefined; } componentDidLoad() { raf(() => { this.setMultipleInputs(); this.setIsInteractive(); - this.setHasSlottedIndicatorControl(); this.focusable = this.isFocusable(); }); - - this.watchForIndicatorControls(); - this.didLoad = true; - } - - disconnectedCallback() { - if (this.indicatorControlObserver) { - this.indicatorControlObserver.disconnect(); - this.indicatorControlObserver = undefined; - } } private totalNestedInputs() { @@ -247,61 +237,10 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac this.isInteractive = covers.length > 0 || inputs.length > 0 || clickables.length > 0; } - /** - * `slotchange` only fires for nodes assigned directly to a slot, so a control - * inside a slotted wrapper (`
`) never reaches - * `updateInteractivityOnSlotChange`. The light DOM is observed instead. - * - * The callback runs the whole handler because a control below a wrapper can also - * make the item multi-input, which is what decides whether the controls draw - * their own indicator at all. - * - * `:host(:has())` would avoid the observer, but the `:has()` fallback in - * `core.scss` is still open as FW-6106 and it's unreliable for slotted content - * in Android WebView. Worth revisiting when FW-6106 closes. - */ - private watchForIndicatorControls() { - if (!Build.isBrowser || typeof MutationObserver === 'undefined') { - return; - } - - // `Node.moveBefore` relocates the item without either callback firing, so - // never leave a previous observer behind - this.indicatorControlObserver?.disconnect(); - - this.indicatorControlObserver = new MutationObserver((records) => { - // The subtree observer also fires for text and hidden input churn, so only - // re-read the DOM when a control was added or removed - if (records.some(touchesIndicatorControl)) { - this.updateInteractivityOnSlotChange(); - } - }); - this.indicatorControlObserver.observe(this.el, { childList: true, subtree: true }); - } - - // These controls paint a focus indicator that overhangs their own bounds, and - // only the default slot is clipped, so only a control there needs extra room. - private setHasSlottedIndicatorControl() { - const controls = this.el.querySelectorAll(INDICATOR_CONTROL_SELECTOR); - - this.hasSlottedIndicatorControl = Array.from(controls).some((control) => { - // The control isn't always a direct child, so walk up to the element the item - // slots, which is the one carrying the slot name. - let slotted: HTMLElement | null = control; - - while (slotted !== null && slotted.parentElement !== this.el) { - slotted = slotted.parentElement; - } - - return slotted !== null && !slotted.getAttribute('slot'); - }); - } - // slot change listener updates state to reflect how/if item should be interactive private updateInteractivityOnSlotChange = () => { this.setIsInteractive(); this.setMultipleInputs(); - this.setHasSlottedIndicatorControl(); }; // If the item contains an input including a checkbox, datetime, select, or radio @@ -437,13 +376,6 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac const firstInteractiveNeedsPointerCursor = firstInteractive !== undefined && !['ION-INPUT', 'ION-TEXTAREA'].includes(firstInteractive.tagName); - /** - * A control in a single-input item defers its indicator to the item, so there's - * nothing to clip and nothing to make room for. It draws its own indicator in a - * multi-input item, and in a clickable item, which is a second tab stop. - */ - const slottedIndicatorNeedsRoom = this.hasSlottedIndicatorControl && (multipleInputs || this.isClickable()); - return ( { - if (node.nodeType !== Node.ELEMENT_NODE) { - return false; - } - - const el = node as Element; - - return el.matches(INDICATOR_CONTROL_SELECTOR) || el.querySelector(INDICATOR_CONTROL_SELECTOR) !== null; -}; - -const touchesIndicatorControl = (record: MutationRecord): boolean => - Array.from(record.addedNodes).some(isIndicatorControl) || Array.from(record.removedNodes).some(isIndicatorControl); From 610c1a6e7bb2e5849c0eafa207787e3d5555e1e6 Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Wed, 5 Aug 2026 11:39:57 -0500 Subject: [PATCH 07/14] test(button): Add e2e tests Update tests for ion-button with annotations Add tests for removeAttribute and attribute sync to ion-button, ion-card, and ion-item --- .../components/button/test/a11y/button.e2e.ts | 78 +++++++++++++++- .../src/components/card/test/a11y/card.e2e.ts | 74 +++++++++++++++ .../src/components/item/test/a11y/item.e2e.ts | 91 +++++++++++++++++++ 3 files changed, 240 insertions(+), 3 deletions(-) diff --git a/core/src/components/button/test/a11y/button.e2e.ts b/core/src/components/button/test/a11y/button.e2e.ts index 6cdd065828f..79674ef0197 100644 --- a/core/src/components/button/test/a11y/button.e2e.ts +++ b/core/src/components/button/test/a11y/button.e2e.ts @@ -152,13 +152,16 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { configs({ directions: ['ltr'] }).forEach(({ title, config }) => { test.describe(title('button: aria attribute sync'), () => { - // Mirrors the ignoreList passed to inheritAriaAttributes/watchForAriaAttributeChanges - // in button.tsx. aria-disabled is excluded because button.tsx manages it internally - // via the `disabled` prop. + // aria-disabled is excluded because button.tsx manages it internally via the `disabled` prop. const watchedAriaAttributes = ariaAttributes.filter((attr) => attr !== 'aria-disabled'); for (const attr of watchedAriaAttributes) { test(`native button updates ${attr} when host attribute changes`, 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'); @@ -173,6 +176,10 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { } test('does not sync aria-disabled, since button.tsx manages it internally', 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'); @@ -180,5 +187,70 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { await expect(nativeButton).not.toHaveAttribute('aria-disabled', 'true'); }); + + test('aria sync survives detach and reattach', async ({ page }) => { + await page.setContent( + ` +
+ Button +
+ `, + config + ); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + await expect(nativeButton).toHaveAttribute('aria-label', 'label'); + + // Detach and reattach + await host.evaluate((buttonEl) => { + const parent = buttonEl.parentElement!; + parent.removeChild(buttonEl); + parent.appendChild(buttonEl); + }); + + await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); + await expect(nativeButton).toHaveAttribute('aria-label', 'updated'); + }); + + test('helper strips host attribute and syncs native element through set, empty, and remove', async ({ page }) => { + page.on('console', (msg) => { + console.log(`[browser] ${msg.type()}: ${msg.text()}`); + }); + + await page.setContent( + ` + Button + `, + config + ); + + const host = page.locator('ion-button'); + const nativeButton = host.locator('button'); + + // Initial load: inheritAriaAttributes should have stripped aria-label + // from the host and copied it onto the native button. + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); + + // Setting a new value on the host: watcher should capture it, sync it + // to native, and re-strip it from the host. + await host.evaluate((el) => el.setAttribute('aria-label', 'second')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).toHaveAttribute('aria-label', 'second'); + + // Setting to empty string: empty string is a valid, non-null value. + await host.evaluate((el) => el.setAttribute('aria-label', '')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).toHaveAttribute('aria-label', ''); + + // Removing the attribute directly: the patched removeAttribute should + // fire onChange with null, which should remove aria-label from native + // and host. + await host.evaluate((el) => el.removeAttribute('aria-label')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).not.toHaveAttribute('aria-label'); + }); }); }); diff --git a/core/src/components/card/test/a11y/card.e2e.ts b/core/src/components/card/test/a11y/card.e2e.ts index 6902037f998..ebb0a7967df 100644 --- a/core/src/components/card/test/a11y/card.e2e.ts +++ b/core/src/components/card/test/a11y/card.e2e.ts @@ -32,3 +32,77 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); }); }); + +configs({ directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('card: aria attribute sync'), () => { + test('aria sync survives detach and reattach', 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'); + + // Detach and reattach + await host.evaluate((cardEl) => { + const parent = cardEl.parentElement!; + parent.removeChild(cardEl); + parent.appendChild(cardEl); + }); + + await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); + await expect(nativeCard).toHaveAttribute('aria-label', 'updated'); + }); + + test('helper strips host attribute and syncs native element through set, empty, and remove', async ({ page }) => { + page.on('console', (msg) => { + console.log(`[browser] ${msg.type()}: ${msg.text()}`); + }); + + await page.setContent( + ` + Button + `, + config + ); + + const host = page.locator('ion-card'); + const nativeButton = host.locator('[part="native"]'); + + // Initial load: inheritAriaAttributes should have stripped aria-label + // from the host and copied it onto the native element. + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); + + // Setting a new value on the host: watcher should capture it, sync it + // to native, and re-strip it from the host. + await host.evaluate((el) => el.setAttribute('aria-label', 'second')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).toHaveAttribute('aria-label', 'second'); + + // Setting to empty string: empty string is a valid, non-null value. + await host.evaluate((el) => el.setAttribute('aria-label', '')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).toHaveAttribute('aria-label', ''); + + // Removing the attribute directly: the patched removeAttribute should + // fire onChange with null, which should remove aria-label from native + // and host. + await host.evaluate((el) => el.removeAttribute('aria-label')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).not.toHaveAttribute('aria-label'); + }); + }); +}); diff --git a/core/src/components/item/test/a11y/item.e2e.ts b/core/src/components/item/test/a11y/item.e2e.ts index 20536beb71a..72581a7af54 100644 --- a/core/src/components/item/test/a11y/item.e2e.ts +++ b/core/src/components/item/test/a11y/item.e2e.ts @@ -153,3 +153,94 @@ configs({ directions: ['ltr'] }).forEach(({ config, screenshot, title }) => { }); }); }); + +configs({ directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('item: aria attribute sync'), () => { + test('native element updates aria-label when host attribute changes', 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('aria-label sync survives detach and reattach', 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((itemEl) => { + const parent = itemEl.parentElement!; + parent.removeChild(itemEl); + parent.appendChild(itemEl); + }); + + await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); + await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); + }); + + test('helper strips host attribute and syncs native element through set, empty, and remove', async ({ page }) => { + page.on('console', (msg) => { + console.log(`[browser] ${msg.type()}: ${msg.text()}`); + }); + + await page.setContent( + ` + Button + `, + config + ); + + const host = page.locator('ion-item'); + const nativeButton = host.locator('[part="native"]'); + + // Initial load: inheritAriaAttributes should have stripped aria-label + // from the host and copied it onto the native element. + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); + + // Setting a new value on the host: watcher should capture it, sync it + // to native, and re-strip it from the host. + await host.evaluate((el) => el.setAttribute('aria-label', 'second')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).toHaveAttribute('aria-label', 'second'); + + // Setting to empty string: empty string is a valid, non-null value. + await host.evaluate((el) => el.setAttribute('aria-label', '')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).toHaveAttribute('aria-label', ''); + + // Removing the attribute directly: the patched removeAttribute should + // fire onChange with null, which should remove aria-label from native + // and host. + await host.evaluate((el) => el.removeAttribute('aria-label')); + await expect(host).not.toHaveAttribute('aria-label'); + await expect(nativeButton).not.toHaveAttribute('aria-label'); + }); + }); +}); From 1c94795b44decf6d1bff71114fb0798fac6d1dee Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Wed, 26 Aug 2026 13:47:09 -0500 Subject: [PATCH 08/14] fix(helper): simplify Mutation Observer for aria attribute watching --- core/src/utils/helpers.ts | 63 ++++++++++++--------------------------- 1 file changed, 19 insertions(+), 44 deletions(-) diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index 643da0d90e2..e2f7442036f 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -122,7 +122,7 @@ export const inheritAttributes = (el: HTMLElement, attributes: string[] = []) => * Removed deprecated attributes. * https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes */ -export const ariaAttributes = [ +const ariaAttributes = [ 'role', 'aria-activedescendant', 'aria-atomic', @@ -197,9 +197,8 @@ export interface AttributeWatcher { /** * Watches an element for changes to a given set of attributes and calls - * onChange whenever one of them is set. Because inheritAttributes() strips - * the attribute from the host as it reads it, any subsequent mutation is - * just checked that the new value isn't null. + * onChange whenever one changes. Returns null when an attribute is removed. + * Call destroy() from disconnectedCallback to stop watching. */ export const watchAttributes = ( el: HTMLElement, @@ -213,55 +212,32 @@ export const watchAttributes = ( return { destroy: () => {} }; } - // Keep a reference to the browser's original implementation. - // removeAttribute is patched below because MutationObserver cannot - // observe removeAttribute() calls once inheritAttributes() has - // already stripped the attribute from the host. In that case the - // browser performs no DOM mutation and emits no MutationRecord. - const originalRemoveAttribute = el.removeAttribute.bind(el); - - // Set up mutation observer to observe attribute changes const observer = new MutationObserver((mutations) => { - const changed: { [k: string]: string } = {}; + const changed: { [k: string]: string | null } = {}; + for (const mutation of mutations) { if (mutation.type !== 'attributes' || !mutation.attributeName) continue; const name = mutation.attributeName; if (!attributes.includes(name)) continue; - const value = el.getAttribute(name); - if (value === null) continue; - changed[name] = value; + + // getAttribute returns null when the attribute was removed — + // passed through to onChange so consumers can clear the value + // from the native element. + changed[name] = el.getAttribute(name); } + if (Object.keys(changed).length > 0) { - // Use the original implementation here. Calling the patched - // removeAttribute would recursively invoke onChange() with - // { [name]: null }, even though we are only stripping the host - // after synchronizing a new value. - Object.keys(changed).forEach((name) => originalRemoveAttribute(name)); onChange(changed); } }); - // Watch for attribute changes on this element - observer.observe(el, { attributes: true, attributeFilter: attributes }); - - // Intercept removeAttribute so we can notify consumers when an - // already-synced attribute is explicitly cleared. - el.removeAttribute = (name: string) => { - if (attributes.includes(name)) { - originalRemoveAttribute(name); - onChange({ [name]: null }); - return; - } - originalRemoveAttribute(name); - }; + observer.observe(el, { + attributes: true, + attributeFilter: attributes, + attributeOldValue: true, + }); - // Stop watching. Call this from `disconnectedCallback`. - return { - destroy: () => { - observer.disconnect(); - el.removeAttribute = originalRemoveAttribute; - }, - }; + return { destroy: () => observer.disconnect() }; }; /** @@ -269,9 +245,8 @@ export const watchAttributes = ( * a callback whenever one is set externally, so that inherited ARIA state * stays in sync for the lifetime of the component — not just at initial load. * - * This should be called once in componentWillLoad, alongside the initial - * call to inheritAriaAttributes, and the returned AttributeWatcher must be - * disconnected in disconnectedCallback to avoid leaking the observer. + * Call this in connectedCallback, alongside the initial inheritAriaAttributes + * call, and call destroy() on the returned watcher in disconnectedCallback. */ export const watchForAriaAttributeChanges = ( el: HTMLElement, From 31ae4245cecf4aa2355e67a2625089b4bca13414 Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Wed, 26 Aug 2026 13:49:12 -0500 Subject: [PATCH 09/14] fix(button): sync aria attributes to native button reactively on button, item, and card --- core/src/components/button/button.tsx | 38 ++++---- core/src/components/card/card.tsx | 32 ++++-- core/src/components/item/item.tsx | 135 ++++++++++++++++++++++---- 3 files changed, 163 insertions(+), 42 deletions(-) diff --git a/core/src/components/button/button.tsx b/core/src/components/button/button.tsx index 67daa2480d1..2e3c994cb62 100644 --- a/core/src/components/button/button.tsx +++ b/core/src/components/button/button.tsx @@ -40,6 +40,7 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf private formButtonEl: HTMLButtonElement | null = null; private formEl: HTMLFormElement | null = null; private inheritedAttributes: Attributes = {}; + private didLoad = false; private ariaWatcher?: AttributeWatcher; @Element() el!: HTMLElement; @@ -205,24 +206,30 @@ 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); } connectedCallback() { - /** - * Must run before watchForAriaAttributeChanges: it calls removeAttribute - * internally to strip the host's initial values, and that call must - * happen before removeAttribute is patched below — otherwise this - * strip would itself be treated as an external removal. - */ - this.inheritedAttributes = inheritAriaAttributes(this.el, ['aria-disabled']); + // Only run the initial snapshot once. On subsequent reconnects the + // host has already been stripped, so inheritAriaAttributes would + // return {} and overwrite previously captured values. - /** - * Keeps inherited ARIA attributes in sync with the host element for the - * lifetime of the component, not just at initial load. `aria-disabled` is excluded here - * (and from the initial inheritAriaAttributes call above) because button.tsx sets - * it itself on Host based on the `disabled` prop. - */ + if (this.didLoad) { + this.startAriaWatcher(); + } + } + + componentDidLoad() { + this.didLoad = true; + this.startAriaWatcher(); + } + + disconnectedCallback() { + this.ariaWatcher?.destroy(); + this.ariaWatcher = undefined; + } + private startAriaWatcher() { this.ariaWatcher = watchForAriaAttributeChanges( this.el, (changed) => { @@ -233,11 +240,6 @@ export class Button implements ComponentInterface, AnchorInterface, ButtonInterf ); } - disconnectedCallback() { - this.ariaWatcher?.destroy(); - this.ariaWatcher = undefined; - } - private get hasIconOnly() { return !!this.el.querySelector('[slot="icon-only"]'); } diff --git a/core/src/components/card/card.tsx b/core/src/components/card/card.tsx index 17510a5eeb1..fd584677130 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, forceUpdate } from '@stencil/core'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes, AttributeWatcher } from '@utils/helpers'; -import { inheritAttributes, watchAttributes } from '@utils/helpers'; +import type { Attributes } from '@utils/helpers'; +import { inheritAttributes, watchForAriaAttributeChanges, type AttributeWatcher } from '@utils/helpers'; import { createColorClasses, openURL } from '@utils/theme'; import { getIonMode } from '../../global/ionic-global'; @@ -24,6 +24,7 @@ import type { RouterDirection } from '../router/utils/interface'; }) export class Card implements ComponentInterface, AnchorInterface, ButtonInterface { private inheritedAriaAttributes: Attributes = {}; + private didLoad = false; private ariaWatcher?: AttributeWatcher; @Element() el!: HTMLElement; @@ -93,10 +94,18 @@ export class Card implements ComponentInterface, AnchorInterface, ButtonInterfac } connectedCallback() { - this.ariaWatcher = watchAttributes(this.el, ['aria-label'], (changed) => { - this.inheritedAriaAttributes = { ...this.inheritedAriaAttributes, ...changed }; - forceUpdate(this); - }); + // Only run the initial snapshot once. On subsequent reconnects the + // host has already been stripped, so inheritAriaAttributes would + // return {} and overwrite previously captured values. + + if (this.didLoad) { + this.startAriaWatcher(); + } + } + + componentDidLoad() { + this.didLoad = true; + this.startAriaWatcher(); } disconnectedCallback() { @@ -104,6 +113,17 @@ export class Card implements ComponentInterface, AnchorInterface, ButtonInterfac this.ariaWatcher = undefined; } + private startAriaWatcher() { + this.ariaWatcher = watchForAriaAttributeChanges( + this.el, + (changed) => { + this.inheritedAriaAttributes = { ...this.inheritedAriaAttributes, ...changed }; + forceUpdate(this); + }, + ['aria-disabled'] + ); + } + private isClickable(): boolean { return this.href !== undefined || this.button; } diff --git a/core/src/components/item/item.tsx b/core/src/components/item/item.tsx index 0916f62e825..aa5746e250d 100644 --- a/core/src/components/item/item.tsx +++ b/core/src/components/item/item.tsx @@ -1,8 +1,8 @@ import type { ComponentInterface } from '@stencil/core'; -import { Component, Element, Host, Listen, Prop, State, Watch, forceUpdate, h } from '@stencil/core'; +import { Build, Component, Element, Host, Listen, Prop, State, Watch, forceUpdate, h } from '@stencil/core'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes, AttributeWatcher } from '@utils/helpers'; -import { inheritAttributes, watchAttributes, raf } from '@utils/helpers'; +import type { Attributes } from '@utils/helpers'; +import { inheritAttributes, raf, watchForAriaAttributeChanges, type AttributeWatcher } from '@utils/helpers'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; import { chevronForward } from 'ionicons/icons'; @@ -10,6 +10,8 @@ import { getIonMode } from '../../global/ionic-global'; import type { AnimationBuilder, Color, CssClassMap, StyleEventDetail } from '../../interface'; import type { RouterDirection } from '../router/utils/interface'; +const INDICATOR_CONTROL_SELECTOR = 'ion-checkbox, ion-radio, ion-toggle'; + /** * @virtualProp {"ios" | "md"} mode - The mode determines which platform styles to use. * @@ -34,6 +36,8 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac private labelColorStyles = {}; private itemStyles = new Map(); private inheritedAriaAttributes: Attributes = {}; + private indicatorControlObserver?: MutationObserver; + private didLoad = false; private ariaWatcher?: AttributeWatcher; @Element() el!: HTMLIonItemElement; @@ -41,6 +45,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac @State() multipleInputs = false; @State() focusable = true; @State() isInteractive = false; + @State() hasSlottedIndicatorControl = false; /** * The color to use from your application's color palette. @@ -165,34 +170,56 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac } } - componentWillLoad() {} - connectedCallback() { this.hasStartEl(); - // Must run before watchForAriaAttributeChanges: it calls removeAttribute - // internally to strip the host's initial values, and that call must - // happen before removeAttribute is patched below — otherwise this - // strip would itself be treated as an external removal. - this.inheritedAriaAttributes = inheritAttributes(this.el, ['aria-label']); - - this.ariaWatcher = watchAttributes(this.el, ['aria-label'], (changed) => { - this.inheritedAriaAttributes = { ...this.inheritedAriaAttributes, ...changed }; - forceUpdate(this); - }); + /** + * `componentDidLoad` doesn't run again when the item is moved, so re-arm the + * observer and re-read the light DOM, which may have changed while detached. + */ + if (this.didLoad) { + this.watchForIndicatorControls(); + this.updateInteractivityOnSlotChange(); + this.startAriaWatcher(); + } } - disconnectedCallback() { - this.ariaWatcher?.destroy(); - this.ariaWatcher = undefined; + componentWillLoad() { + this.inheritedAriaAttributes = inheritAttributes(this.el, ['aria-label']); } componentDidLoad() { raf(() => { this.setMultipleInputs(); this.setIsInteractive(); + this.setHasSlottedIndicatorControl(); this.focusable = this.isFocusable(); }); + + this.watchForIndicatorControls(); + this.startAriaWatcher(); + this.didLoad = true; + } + + disconnectedCallback() { + if (this.indicatorControlObserver) { + this.indicatorControlObserver.disconnect(); + this.indicatorControlObserver = undefined; + } + + this.ariaWatcher?.destroy(); + this.ariaWatcher = undefined; + } + + private startAriaWatcher() { + this.ariaWatcher = watchForAriaAttributeChanges( + this.el, + (changed) => { + this.inheritedAriaAttributes = { ...this.inheritedAriaAttributes, ...changed }; + forceUpdate(this); + }, + ['aria-disabled'] + ); } private totalNestedInputs() { @@ -237,10 +264,61 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac this.isInteractive = covers.length > 0 || inputs.length > 0 || clickables.length > 0; } + /** + * `slotchange` only fires for nodes assigned directly to a slot, so a control + * inside a slotted wrapper (`
`) never reaches + * `updateInteractivityOnSlotChange`. The light DOM is observed instead. + * + * The callback runs the whole handler because a control below a wrapper can also + * make the item multi-input, which is what decides whether the controls draw + * their own indicator at all. + * + * `:host(:has())` would avoid the observer, but the `:has()` fallback in + * `core.scss` is still open as FW-6106 and it's unreliable for slotted content + * in Android WebView. Worth revisiting when FW-6106 closes. + */ + private watchForIndicatorControls() { + if (!Build.isBrowser || typeof MutationObserver === 'undefined') { + return; + } + + // `Node.moveBefore` relocates the item without either callback firing, so + // never leave a previous observer behind + this.indicatorControlObserver?.disconnect(); + + this.indicatorControlObserver = new MutationObserver((records) => { + // The subtree observer also fires for text and hidden input churn, so only + // re-read the DOM when a control was added or removed + if (records.some(touchesIndicatorControl)) { + this.updateInteractivityOnSlotChange(); + } + }); + this.indicatorControlObserver.observe(this.el, { childList: true, subtree: true }); + } + + // These controls paint a focus indicator that overhangs their own bounds, and + // only the default slot is clipped, so only a control there needs extra room. + private setHasSlottedIndicatorControl() { + const controls = this.el.querySelectorAll(INDICATOR_CONTROL_SELECTOR); + + this.hasSlottedIndicatorControl = Array.from(controls).some((control) => { + // The control isn't always a direct child, so walk up to the element the item + // slots, which is the one carrying the slot name. + let slotted: HTMLElement | null = control; + + while (slotted !== null && slotted.parentElement !== this.el) { + slotted = slotted.parentElement; + } + + return slotted !== null && !slotted.getAttribute('slot'); + }); + } + // slot change listener updates state to reflect how/if item should be interactive private updateInteractivityOnSlotChange = () => { this.setIsInteractive(); this.setMultipleInputs(); + this.setHasSlottedIndicatorControl(); }; // If the item contains an input including a checkbox, datetime, select, or radio @@ -376,6 +454,13 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac const firstInteractiveNeedsPointerCursor = firstInteractive !== undefined && !['ION-INPUT', 'ION-TEXTAREA'].includes(firstInteractive.tagName); + /** + * A control in a single-input item defers its indicator to the item, so there's + * nothing to clip and nothing to make room for. It draws its own indicator in a + * multi-input item, and in a clickable item, which is a second tab stop. + */ + const slottedIndicatorNeedsRoom = this.hasSlottedIndicatorControl && (multipleInputs || this.isClickable()); + return ( { + if (node.nodeType !== Node.ELEMENT_NODE) { + return false; + } + + const el = node as Element; + + return el.matches(INDICATOR_CONTROL_SELECTOR) || el.querySelector(INDICATOR_CONTROL_SELECTOR) !== null; +}; + +const touchesIndicatorControl = (record: MutationRecord): boolean => + Array.from(record.addedNodes).some(isIndicatorControl) || Array.from(record.removedNodes).some(isIndicatorControl); From 962baac7ddc1c1703a95ab23906c2d57343769fd Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Wed, 26 Aug 2026 13:50:00 -0500 Subject: [PATCH 10/14] test(button): Fix e2e tests for button, item, and card changes --- .../components/button/test/a11y/button.e2e.ts | 63 ++++++++------- .../src/components/card/test/a11y/card.e2e.ts | 80 ++++++++++++------- .../src/components/item/test/a11y/item.e2e.ts | 39 ++++----- 3 files changed, 105 insertions(+), 77 deletions(-) diff --git a/core/src/components/button/test/a11y/button.e2e.ts b/core/src/components/button/test/a11y/button.e2e.ts index 79674ef0197..011e92b1401 100644 --- a/core/src/components/button/test/a11y/button.e2e.ts +++ b/core/src/components/button/test/a11y/button.e2e.ts @@ -1,6 +1,5 @@ import AxeBuilder from '@axe-core/playwright'; import { expect } from '@playwright/test'; -import { ariaAttributes } from '@utils/helpers'; import { configs, test } from '@utils/test/playwright'; configs({ directions: ['ltr'], palettes: ['light', 'dark'] }).forEach(({ title, config }) => { @@ -152,8 +151,7 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { configs({ directions: ['ltr'] }).forEach(({ title, config }) => { test.describe(title('button: aria attribute sync'), () => { - // aria-disabled is excluded because button.tsx manages it internally via the `disabled` prop. - const watchedAriaAttributes = ariaAttributes.filter((attr) => attr !== 'aria-disabled'); + const watchedAriaAttributes = ['aria-checked', 'aria-label', 'aria-pressed', 'aria-description']; for (const attr of watchedAriaAttributes) { test(`native button updates ${attr} when host attribute changes`, async ({ page }) => { @@ -175,20 +173,29 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { }); } - test('does not sync aria-disabled, since button.tsx manages it internally', async ({ page }) => { - test - .info() - .annotations.push({ type: 'issue', description: 'https://github.com/ionic-team/ionic-framework/issues/30626' }); + test('should not sync aria-disabled from 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).not.toHaveAttribute('aria-disabled', 'true'); + // Initial inheritance moves the developer-provided value to native. + // The host's aria-disabled is subsequently owned by the disabled prop. + await expect(host).not.toHaveAttribute('aria-disabled'); + await expect(nativeButton).toHaveAttribute('aria-disabled', 'true'); }); - test('aria sync survives detach and reattach', async ({ page }) => { + test('preserves inherited aria-label after detach and reattach', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + }); + await page.setContent( `
@@ -203,20 +210,22 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { await expect(nativeButton).toHaveAttribute('aria-label', 'label'); - // Detach and reattach - await host.evaluate((buttonEl) => { - const parent = buttonEl.parentElement!; - parent.removeChild(buttonEl); - parent.appendChild(buttonEl); + // Detach, reattach, and force a render via a prop change. + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + parent.appendChild(el); + (el as HTMLIonButtonElement).color = 'primary'; }); - await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); - await expect(nativeButton).toHaveAttribute('aria-label', 'updated'); + // Assert the original value survived + await expect(nativeButton).toHaveAttribute('aria-label', 'label'); }); - test('helper strips host attribute and syncs native element through set, empty, and remove', async ({ page }) => { - page.on('console', (msg) => { - console.log(`[browser] ${msg.type()}: ${msg.text()}`); + test('syncs aria-label updates and removal after initial inheritance', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', }); await page.setContent( @@ -229,25 +238,21 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { const host = page.locator('ion-button'); const nativeButton = host.locator('button'); - // Initial load: inheritAriaAttributes should have stripped aria-label - // from the host and copied it onto the native button. + // Initial inheritance moves the value from the host to the native button. await expect(host).not.toHaveAttribute('aria-label'); await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); - // Setting a new value on the host: watcher should capture it, sync it - // to native, and re-strip it from the host. + // Post-load writes remain on the host and are synchronized to native await host.evaluate((el) => el.setAttribute('aria-label', 'second')); - await expect(host).not.toHaveAttribute('aria-label'); + await expect(host).toHaveAttribute('aria-label'); await expect(nativeButton).toHaveAttribute('aria-label', 'second'); - // Setting to empty string: empty string is a valid, non-null value. + // An empty string is a valid ARIA attribute value and remains synchronized. await host.evaluate((el) => el.setAttribute('aria-label', '')); - await expect(host).not.toHaveAttribute('aria-label'); + await expect(host).toHaveAttribute('aria-label'); await expect(nativeButton).toHaveAttribute('aria-label', ''); - // Removing the attribute directly: the patched removeAttribute should - // fire onChange with null, which should remove aria-label from native - // and host. + // Native MutationObserver behavior sees a real removal after a post-load write. await host.evaluate((el) => el.removeAttribute('aria-label')); await expect(host).not.toHaveAttribute('aria-label'); await expect(nativeButton).not.toHaveAttribute('aria-label'); diff --git a/core/src/components/card/test/a11y/card.e2e.ts b/core/src/components/card/test/a11y/card.e2e.ts index ebb0a7967df..3d9a2be9f04 100644 --- a/core/src/components/card/test/a11y/card.e2e.ts +++ b/core/src/components/card/test/a11y/card.e2e.ts @@ -34,8 +34,8 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); configs({ directions: ['ltr'] }).forEach(({ title, config }) => { - test.describe(title('card: aria attribute sync'), () => { - test('aria sync survives detach and reattach', async ({ page }) => { + test.describe(title('item: aria attribute sync'), () => { + test('native element updates aria-label when host attribute changes', async ({ page }) => { test.info().annotations.push({ type: 'issue', description: 'https://github.com/ionic-team/ionic-framework/issues/30626', @@ -43,37 +43,61 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { await page.setContent( ` -
- Card -
- `, + Card + `, config ); const host = page.locator('ion-card'); - const nativeCard = host.locator('[part="native"]'); + const nativeItem = host.locator('[part="native"]'); - await expect(nativeCard).toHaveAttribute('aria-label', 'label'); - - // Detach and reattach - await host.evaluate((cardEl) => { - const parent = cardEl.parentElement!; - parent.removeChild(cardEl); - parent.appendChild(cardEl); - }); + await expect(nativeItem).toHaveAttribute('aria-label', 'label'); await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); - await expect(nativeCard).toHaveAttribute('aria-label', 'updated'); + + await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); }); - test('helper strips host attribute and syncs native element through set, empty, and remove', async ({ page }) => { - page.on('console', (msg) => { - console.log(`[browser] ${msg.type()}: ${msg.text()}`); + test('preserves inherited aria-label after detach and reattach', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', }); await page.setContent( ` - Button +
+ Card +
+ `, + config + ); + + const host = page.locator('ion-card'); + const nativeItem = host.locator('[part="native"]'); + + await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + + // Detach, reattach, and force a render via a prop change. + await host.evaluate((itemEl) => { + const parent = itemEl.parentElement!; + parent.removeChild(itemEl); + parent.appendChild(itemEl); + (itemEl as HTMLIonButtonElement).color = 'primary'; + }); + + // Assert the original value survived + await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + }); + + test('syncs aria-label updates and removal after initial inheritance', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + }); + await page.setContent( + ` + Card `, config ); @@ -81,25 +105,21 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { const host = page.locator('ion-card'); const nativeButton = host.locator('[part="native"]'); - // Initial load: inheritAriaAttributes should have stripped aria-label - // from the host and copied it onto the native element. + // Initial inheritance moves the value from the host to the native button. await expect(host).not.toHaveAttribute('aria-label'); await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); - // Setting a new value on the host: watcher should capture it, sync it - // to native, and re-strip it from the host. + // Post-load writes remain on the host and are synchronized to native await host.evaluate((el) => el.setAttribute('aria-label', 'second')); - await expect(host).not.toHaveAttribute('aria-label'); + await expect(host).toHaveAttribute('aria-label'); await expect(nativeButton).toHaveAttribute('aria-label', 'second'); - // Setting to empty string: empty string is a valid, non-null value. + // An empty string is a valid ARIA attribute value and remains synchronized. await host.evaluate((el) => el.setAttribute('aria-label', '')); - await expect(host).not.toHaveAttribute('aria-label'); + await expect(host).toHaveAttribute('aria-label'); await expect(nativeButton).toHaveAttribute('aria-label', ''); - // Removing the attribute directly: the patched removeAttribute should - // fire onChange with null, which should remove aria-label from native - // and host. + // Native MutationObserver behavior sees a real removal after a post-load write. await host.evaluate((el) => el.removeAttribute('aria-label')); await expect(host).not.toHaveAttribute('aria-label'); await expect(nativeButton).not.toHaveAttribute('aria-label'); diff --git a/core/src/components/item/test/a11y/item.e2e.ts b/core/src/components/item/test/a11y/item.e2e.ts index 72581a7af54..01f06991870 100644 --- a/core/src/components/item/test/a11y/item.e2e.ts +++ b/core/src/components/item/test/a11y/item.e2e.ts @@ -179,7 +179,12 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); }); - test('aria-label sync survives detach and reattach', async ({ page }) => { + test('preserves inherited aria-label after detach and reattach', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + }); + await page.setContent( `
@@ -194,24 +199,26 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + // Detach, reattach, and force a render via a prop change. await host.evaluate((itemEl) => { const parent = itemEl.parentElement!; parent.removeChild(itemEl); parent.appendChild(itemEl); + (itemEl as HTMLIonButtonElement).color = 'primary'; }); - await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); - await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); + // Assert the original value survived + await expect(nativeItem).toHaveAttribute('aria-label', 'label'); }); - test('helper strips host attribute and syncs native element through set, empty, and remove', async ({ page }) => { - page.on('console', (msg) => { - console.log(`[browser] ${msg.type()}: ${msg.text()}`); + test('syncs aria-label updates and removal after initial inheritance', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30626', }); - await page.setContent( ` - Button + Item `, config ); @@ -219,25 +226,21 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { const host = page.locator('ion-item'); const nativeButton = host.locator('[part="native"]'); - // Initial load: inheritAriaAttributes should have stripped aria-label - // from the host and copied it onto the native element. + // Initial inheritance moves the value from the host to the native button. await expect(host).not.toHaveAttribute('aria-label'); await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); - // Setting a new value on the host: watcher should capture it, sync it - // to native, and re-strip it from the host. + // Post-load writes remain on the host and are synchronized to native await host.evaluate((el) => el.setAttribute('aria-label', 'second')); - await expect(host).not.toHaveAttribute('aria-label'); + await expect(host).toHaveAttribute('aria-label'); await expect(nativeButton).toHaveAttribute('aria-label', 'second'); - // Setting to empty string: empty string is a valid, non-null value. + // An empty string is a valid ARIA attribute value and remains synchronized. await host.evaluate((el) => el.setAttribute('aria-label', '')); - await expect(host).not.toHaveAttribute('aria-label'); + await expect(host).toHaveAttribute('aria-label'); await expect(nativeButton).toHaveAttribute('aria-label', ''); - // Removing the attribute directly: the patched removeAttribute should - // fire onChange with null, which should remove aria-label from native - // and host. + // Native MutationObserver behavior sees a real removal after a post-load write. await host.evaluate((el) => el.removeAttribute('aria-label')); await expect(host).not.toHaveAttribute('aria-label'); await expect(nativeButton).not.toHaveAttribute('aria-label'); From f142f3604cd7dcc4fac9aef6ccef85f187550065 Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Wed, 26 Aug 2026 15:09:28 -0500 Subject: [PATCH 11/14] fix(test): Update Item and Card native test variable Consistency based on element being tested --- .../src/components/card/test/a11y/card.e2e.ts | 22 +++++++++---------- .../src/components/item/test/a11y/item.e2e.ts | 10 ++++----- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/core/src/components/card/test/a11y/card.e2e.ts b/core/src/components/card/test/a11y/card.e2e.ts index 3d9a2be9f04..25e65142769 100644 --- a/core/src/components/card/test/a11y/card.e2e.ts +++ b/core/src/components/card/test/a11y/card.e2e.ts @@ -49,13 +49,13 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { ); const host = page.locator('ion-card'); - const nativeItem = host.locator('[part="native"]'); + const nativeCard = host.locator('[part="native"]'); - await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + await expect(nativeCard).toHaveAttribute('aria-label', 'label'); await host.evaluate((el) => el.setAttribute('aria-label', 'updated')); - await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); + await expect(nativeCard).toHaveAttribute('aria-label', 'updated'); }); test('preserves inherited aria-label after detach and reattach', async ({ page }) => { @@ -74,9 +74,9 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { ); const host = page.locator('ion-card'); - const nativeItem = host.locator('[part="native"]'); + const nativeCard = host.locator('[part="native"]'); - await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + await expect(nativeCard).toHaveAttribute('aria-label', 'label'); // Detach, reattach, and force a render via a prop change. await host.evaluate((itemEl) => { @@ -87,7 +87,7 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { }); // Assert the original value survived - await expect(nativeItem).toHaveAttribute('aria-label', 'label'); + await expect(nativeCard).toHaveAttribute('aria-label', 'label'); }); test('syncs aria-label updates and removal after initial inheritance', async ({ page }) => { @@ -103,26 +103,26 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { ); const host = page.locator('ion-card'); - const nativeButton = host.locator('[part="native"]'); + const nativeCard = host.locator('[part="native"]'); // Initial inheritance moves the value from the host to the native button. await expect(host).not.toHaveAttribute('aria-label'); - await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); + await expect(nativeCard).toHaveAttribute('aria-label', 'initial'); // Post-load writes remain on the host and are synchronized to native await host.evaluate((el) => el.setAttribute('aria-label', 'second')); await expect(host).toHaveAttribute('aria-label'); - await expect(nativeButton).toHaveAttribute('aria-label', 'second'); + await expect(nativeCard).toHaveAttribute('aria-label', 'second'); // An empty string is a valid ARIA attribute value and remains synchronized. await host.evaluate((el) => el.setAttribute('aria-label', '')); await expect(host).toHaveAttribute('aria-label'); - await expect(nativeButton).toHaveAttribute('aria-label', ''); + await expect(nativeCard).toHaveAttribute('aria-label', ''); // Native MutationObserver behavior sees a real removal after a post-load write. await host.evaluate((el) => el.removeAttribute('aria-label')); await expect(host).not.toHaveAttribute('aria-label'); - await expect(nativeButton).not.toHaveAttribute('aria-label'); + await expect(nativeCard).not.toHaveAttribute('aria-label'); }); }); }); diff --git a/core/src/components/item/test/a11y/item.e2e.ts b/core/src/components/item/test/a11y/item.e2e.ts index 01f06991870..2014239078a 100644 --- a/core/src/components/item/test/a11y/item.e2e.ts +++ b/core/src/components/item/test/a11y/item.e2e.ts @@ -224,26 +224,26 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { ); const host = page.locator('ion-item'); - const nativeButton = host.locator('[part="native"]'); + const nativeItem = host.locator('[part="native"]'); // Initial inheritance moves the value from the host to the native button. await expect(host).not.toHaveAttribute('aria-label'); - await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); + await expect(nativeItem).toHaveAttribute('aria-label', 'initial'); // Post-load writes remain on the host and are synchronized to native await host.evaluate((el) => el.setAttribute('aria-label', 'second')); await expect(host).toHaveAttribute('aria-label'); - await expect(nativeButton).toHaveAttribute('aria-label', 'second'); + await expect(nativeItem).toHaveAttribute('aria-label', 'second'); // An empty string is a valid ARIA attribute value and remains synchronized. await host.evaluate((el) => el.setAttribute('aria-label', '')); await expect(host).toHaveAttribute('aria-label'); - await expect(nativeButton).toHaveAttribute('aria-label', ''); + await expect(nativeItem).toHaveAttribute('aria-label', ''); // Native MutationObserver behavior sees a real removal after a post-load write. await host.evaluate((el) => el.removeAttribute('aria-label')); await expect(host).not.toHaveAttribute('aria-label'); - await expect(nativeButton).not.toHaveAttribute('aria-label'); + await expect(nativeItem).not.toHaveAttribute('aria-label'); }); }); }); From 8bea648c5e9f44e702499a9e83df2bf419e1ce40 Mon Sep 17 00:00:00 2001 From: Zac-Smucker-Bryan Date: Mon, 31 Aug 2026 15:05:46 -0500 Subject: [PATCH 12/14] fix lint build error - duplicate imports @utils/helpers was imported twice and passed npm run lint but failed build. Now, it is imported only once. --- core/src/components/button/button.tsx | 2 +- core/src/components/card/card.tsx | 8 ++++++-- core/src/components/item/item.tsx | 9 +++++++-- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/core/src/components/button/button.tsx b/core/src/components/button/button.tsx index 2e3c994cb62..34d000971fe 100644 --- a/core/src/components/button/button.tsx +++ b/core/src/components/button/button.tsx @@ -1,12 +1,12 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core'; import { Component, Element, Event, Host, Prop, Watch, State, forceUpdate, h } from '@stencil/core'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes } from '@utils/helpers'; import { inheritAriaAttributes, hasShadowDom, watchForAriaAttributeChanges, type AttributeWatcher, + type Attributes, } from '@utils/helpers'; import { printIonWarning } from '@utils/logging'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; diff --git a/core/src/components/card/card.tsx b/core/src/components/card/card.tsx index fd584677130..5d90e22aa58 100644 --- a/core/src/components/card/card.tsx +++ b/core/src/components/card/card.tsx @@ -1,8 +1,12 @@ import type { ComponentInterface } from '@stencil/core'; import { Element, Component, Host, Prop, h, forceUpdate } from '@stencil/core'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes } from '@utils/helpers'; -import { inheritAttributes, watchForAriaAttributeChanges, type AttributeWatcher } from '@utils/helpers'; +import { + inheritAttributes, + watchForAriaAttributeChanges, + type AttributeWatcher, + type Attributes, +} from '@utils/helpers'; import { createColorClasses, openURL } from '@utils/theme'; import { getIonMode } from '../../global/ionic-global'; diff --git a/core/src/components/item/item.tsx b/core/src/components/item/item.tsx index aa5746e250d..b1486f5cd13 100644 --- a/core/src/components/item/item.tsx +++ b/core/src/components/item/item.tsx @@ -1,8 +1,13 @@ import type { ComponentInterface } from '@stencil/core'; import { Build, Component, Element, Host, Listen, Prop, State, Watch, forceUpdate, h } from '@stencil/core'; import type { AnchorInterface, ButtonInterface } from '@utils/element-interface'; -import type { Attributes } from '@utils/helpers'; -import { inheritAttributes, raf, watchForAriaAttributeChanges, type AttributeWatcher } from '@utils/helpers'; +import { + inheritAttributes, + raf, + watchForAriaAttributeChanges, + type AttributeWatcher, + type Attributes, +} from '@utils/helpers'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; import { chevronForward } from 'ionicons/icons'; From 1ab1c95ca7f3bae1176ec522d3f73954c89a5446 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Mon, 14 Sep 2026 14:20:40 -0700 Subject: [PATCH 13/14] fix(button, card, item): keep aria attributes in sync with the host --- core/src/components/button/button.tsx | 69 ++--- .../components/button/test/a11y/button.e2e.ts | 158 +++++++--- core/src/components/card/card.tsx | 44 +-- .../src/components/card/test/a11y/card.e2e.ts | 119 +++++--- core/src/components/item/item.tsx | 41 +-- .../src/components/item/test/a11y/item.e2e.ts | 102 ++++--- core/src/utils/attribute-controller.ts | 171 +++++++++++ core/src/utils/helpers.ts | 77 +---- .../utils/test/attribute-controller.spec.ts | 286 ++++++++++++++++++ docs/component-guide.md | 66 +++- 10 files changed, 799 insertions(+), 334 deletions(-) create mode 100644 core/src/utils/attribute-controller.ts create mode 100644 core/src/utils/test/attribute-controller.spec.ts diff --git a/core/src/components/button/button.tsx b/core/src/components/button/button.tsx index 34d000971fe..eff2143f9aa 100644 --- a/core/src/components/button/button.tsx +++ b/core/src/components/button/button.tsx @@ -1,13 +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 { - inheritAriaAttributes, - hasShadowDom, - watchForAriaAttributeChanges, - type AttributeWatcher, - type Attributes, -} from '@utils/helpers'; +import { hasShadowDom } from '@utils/helpers'; import { printIonWarning } from '@utils/logging'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; @@ -39,9 +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 didLoad = false; - private ariaWatcher?: AttributeWatcher; + private ariaController?: AttributeController; @Element() el!: HTMLElement; @@ -206,38 +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); - } - - connectedCallback() { - // Only run the initial snapshot once. On subsequent reconnects the - // host has already been stripped, so inheritAriaAttributes would - // return {} and overwrite previously captured values. - if (this.didLoad) { - this.startAriaWatcher(); - } + /** + * 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']); } - componentDidLoad() { - this.didLoad = true; - this.startAriaWatcher(); + connectedCallback() { + this.ariaController?.init(); } disconnectedCallback() { - this.ariaWatcher?.destroy(); - this.ariaWatcher = undefined; - } - - private startAriaWatcher() { - this.ariaWatcher = watchForAriaAttributeChanges( - this.el, - (changed) => { - this.inheritedAttributes = { ...this.inheritedAttributes, ...changed }; - forceUpdate(this); - }, - ['aria-disabled'] - ); + this.ariaController?.destroy(); } private get hasIconOnly() { @@ -356,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 011e92b1401..03cf866534c 100644 --- a/core/src/components/button/test/a11y/button.e2e.ts +++ b/core/src/components/button/test/a11y/button.e2e.ts @@ -149,12 +149,19 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); }); -configs({ directions: ['ltr'] }).forEach(({ title, 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'), () => { - const watchedAriaAttributes = ['aria-checked', 'aria-label', 'aria-pressed', 'aria-description']; - - for (const attr of watchedAriaAttributes) { - test(`native button updates ${attr} when host attribute changes`, async ({ page }) => { + /** + * 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', @@ -174,32 +181,65 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { } test('should not sync aria-disabled from 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'); - // Initial inheritance moves the developer-provided value to native. - // The host's aria-disabled is subsequently owned by the disabled prop. - await expect(host).not.toHaveAttribute('aria-disabled'); + // 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('preserves inherited aria-label after detach and reattach', async ({ page }) => { - test.info().annotations.push({ - type: 'issue', - description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + 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 + Button
`, config @@ -208,54 +248,76 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { const host = page.locator('ion-button'); const nativeButton = host.locator('button'); - await expect(nativeButton).toHaveAttribute('aria-label', 'label'); + await expect(nativeButton).toHaveAttribute('aria-description', 'described'); - // Detach, reattach, and force a render via a prop change. await host.evaluate((el) => { const parent = el.parentElement!; parent.removeChild(el); parent.appendChild(el); - (el as HTMLIonButtonElement).color = 'primary'; }); + await page.waitForChanges(); - // Assert the original value survived - await expect(nativeButton).toHaveAttribute('aria-label', 'label'); - }); + // The value captured at load survives the move. + await expect(nativeButton).toHaveAttribute('aria-description', 'described'); - test('syncs aria-label updates and removal after initial inheritance', async ({ page }) => { - test.info().annotations.push({ - type: 'issue', - description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + // 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'); + }); - await page.setContent( - ` - Button - `, - config - ); + 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'); - // Initial inheritance moves the value from the host to the native button. - await expect(host).not.toHaveAttribute('aria-label'); - await expect(nativeButton).toHaveAttribute('aria-label', 'initial'); + // 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 remain on the host and are synchronized to native - await host.evaluate((el) => el.setAttribute('aria-label', 'second')); - await expect(host).toHaveAttribute('aria-label'); - await expect(nativeButton).toHaveAttribute('aria-label', 'second'); + // 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 and remains synchronized. - await host.evaluate((el) => el.setAttribute('aria-label', '')); - await expect(host).toHaveAttribute('aria-label'); - await expect(nativeButton).toHaveAttribute('aria-label', ''); + // An empty string is a valid ARIA attribute value. + await host.evaluate((el) => el.setAttribute('aria-description', '')); + await expect(nativeButton).toHaveAttribute('aria-description', ''); - // Native MutationObserver behavior sees a real removal after a post-load write. + // 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')); - await expect(host).not.toHaveAttribute('aria-label'); - await expect(nativeButton).not.toHaveAttribute('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 5d90e22aa58..d6e8653a924 100644 --- a/core/src/components/card/card.tsx +++ b/core/src/components/card/card.tsx @@ -1,12 +1,8 @@ import type { ComponentInterface } 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 { - inheritAttributes, - watchForAriaAttributeChanges, - type AttributeWatcher, - type Attributes, -} from '@utils/helpers'; import { createColorClasses, openURL } from '@utils/theme'; import { getIonMode } from '../../global/ionic-global'; @@ -27,9 +23,7 @@ import type { RouterDirection } from '../router/utils/interface'; shadow: true, }) export class Card implements ComponentInterface, AnchorInterface, ButtonInterface { - private inheritedAriaAttributes: Attributes = {}; - private didLoad = false; - private ariaWatcher?: AttributeWatcher; + private ariaController?: AttributeController; @Element() el!: HTMLElement; /** @@ -94,38 +88,15 @@ export class Card implements ComponentInterface, AnchorInterface, ButtonInterfac @Prop() target: string | undefined; componentWillLoad() { - this.inheritedAriaAttributes = inheritAttributes(this.el, ['aria-label']); + this.ariaController = createAttributeController(this.el, ['aria-label'], () => forceUpdate(this)); } connectedCallback() { - // Only run the initial snapshot once. On subsequent reconnects the - // host has already been stripped, so inheritAriaAttributes would - // return {} and overwrite previously captured values. - - if (this.didLoad) { - this.startAriaWatcher(); - } - } - - componentDidLoad() { - this.didLoad = true; - this.startAriaWatcher(); + this.ariaController?.init(); } disconnectedCallback() { - this.ariaWatcher?.destroy(); - this.ariaWatcher = undefined; - } - - private startAriaWatcher() { - this.ariaWatcher = watchForAriaAttributeChanges( - this.el, - (changed) => { - this.inheritedAriaAttributes = { ...this.inheritedAriaAttributes, ...changed }; - forceUpdate(this); - }, - ['aria-disabled'] - ); + this.ariaController?.destroy(); } private isClickable(): boolean { @@ -138,7 +109,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 25e65142769..7adb42ed5e7 100644 --- a/core/src/components/card/test/a11y/card.e2e.ts +++ b/core/src/components/card/test/a11y/card.e2e.ts @@ -33,20 +33,18 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); }); -configs({ directions: ['ltr'] }).forEach(({ title, config }) => { - test.describe(title('item: aria attribute sync'), () => { - test('native element updates aria-label when host attribute changes', async ({ page }) => { +/** + * 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 - ); + await page.setContent(`Card`, config); const host = page.locator('ion-card'); const nativeCard = host.locator('[part="native"]'); @@ -58,18 +56,13 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { await expect(nativeCard).toHaveAttribute('aria-label', 'updated'); }); - test('preserves inherited aria-label after detach and reattach', async ({ page }) => { - test.info().annotations.push({ - type: 'issue', - description: 'https://github.com/ionic-team/ionic-framework/issues/30626', - }); - + test('should keep syncing after the card is detached and reattached', async ({ page }) => { await page.setContent( ` -
- Card -
- `, +
+ Card +
+ `, config ); @@ -78,51 +71,91 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { await expect(nativeCard).toHaveAttribute('aria-label', 'label'); - // Detach, reattach, and force a render via a prop change. - await host.evaluate((itemEl) => { - const parent = itemEl.parentElement!; - parent.removeChild(itemEl); - parent.appendChild(itemEl); - (itemEl as HTMLIonButtonElement).color = 'primary'; + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + parent.appendChild(el); }); + await page.waitForChanges(); - // Assert the original value survived + // The value captured at load survives the move. await expect(nativeCard).toHaveAttribute('aria-label', 'label'); - }); - test('syncs aria-label updates and removal after initial inheritance', async ({ page }) => { - test.info().annotations.push({ - type: 'issue', - description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + // 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 page.setContent( - ` - Card - `, - config - ); + 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"]'); - // Initial inheritance moves the value from the host to the native button. + // 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 remain on the host and are synchronized to native + // 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'); + await expect(host).toHaveAttribute('aria-label', 'second'); await expect(nativeCard).toHaveAttribute('aria-label', 'second'); - // An empty string is a valid ARIA attribute value and remains synchronized. + // An empty string is a valid ARIA attribute value. await host.evaluate((el) => el.setAttribute('aria-label', '')); - await expect(host).toHaveAttribute('aria-label'); await expect(nativeCard).toHaveAttribute('aria-label', ''); - // Native MutationObserver behavior sees a real removal after a post-load write. + // 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 b1486f5cd13..fa7de421fc2 100644 --- a/core/src/components/item/item.tsx +++ b/core/src/components/item/item.tsx @@ -1,13 +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 { - inheritAttributes, - raf, - watchForAriaAttributeChanges, - type AttributeWatcher, - type Attributes, -} from '@utils/helpers'; +import { raf } from '@utils/helpers'; import { createColorClasses, hostContext, openURL } from '@utils/theme'; import { chevronForward } from 'ionicons/icons'; @@ -40,10 +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 ariaWatcher?: AttributeWatcher; + private ariaController?: AttributeController; @Element() el!: HTMLIonItemElement; @@ -185,12 +180,19 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac if (this.didLoad) { this.watchForIndicatorControls(); this.updateInteractivityOnSlotChange(); - this.startAriaWatcher(); } + + 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() { @@ -202,7 +204,6 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac }); this.watchForIndicatorControls(); - this.startAriaWatcher(); this.didLoad = true; } @@ -212,19 +213,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac this.indicatorControlObserver = undefined; } - this.ariaWatcher?.destroy(); - this.ariaWatcher = undefined; - } - - private startAriaWatcher() { - this.ariaWatcher = watchForAriaAttributeChanges( - this.el, - (changed) => { - this.inheritedAriaAttributes = { ...this.inheritedAriaAttributes, ...changed }; - forceUpdate(this); - }, - ['aria-disabled'] - ); + this.ariaController?.destroy(); } private totalNestedInputs() { @@ -377,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 2014239078a..084e4d3b65e 100644 --- a/core/src/components/item/test/a11y/item.e2e.ts +++ b/core/src/components/item/test/a11y/item.e2e.ts @@ -154,20 +154,18 @@ configs({ directions: ['ltr'] }).forEach(({ config, screenshot, title }) => { }); }); -configs({ directions: ['ltr'] }).forEach(({ title, config }) => { +/** + * 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('native element updates aria-label when host attribute changes', async ({ page }) => { + 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 - ); + await page.setContent(`Item`, config); const host = page.locator('ion-item'); const nativeItem = host.locator('[part="native"]'); @@ -179,18 +177,13 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { await expect(nativeItem).toHaveAttribute('aria-label', 'updated'); }); - test('preserves inherited aria-label after detach and reattach', async ({ page }) => { - test.info().annotations.push({ - type: 'issue', - description: 'https://github.com/ionic-team/ionic-framework/issues/30626', - }); - + test('should keep syncing after the item is detached and reattached', async ({ page }) => { await page.setContent( ` -
- Item -
- `, +
+ Item +
+ `, config ); @@ -199,51 +192,76 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => { await expect(nativeItem).toHaveAttribute('aria-label', 'label'); - // Detach, reattach, and force a render via a prop change. - await host.evaluate((itemEl) => { - const parent = itemEl.parentElement!; - parent.removeChild(itemEl); - parent.appendChild(itemEl); - (itemEl as HTMLIonButtonElement).color = 'primary'; + await host.evaluate((el) => { + const parent = el.parentElement!; + parent.removeChild(el); + parent.appendChild(el); }); + await page.waitForChanges(); - // Assert the original value survived + // The value captured at load survives the move. await expect(nativeItem).toHaveAttribute('aria-label', 'label'); - }); - test('syncs aria-label updates and removal after initial inheritance', async ({ page }) => { - test.info().annotations.push({ - type: 'issue', - description: 'https://github.com/ionic-team/ionic-framework/issues/30626', + // 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 page.setContent( - ` - Item - `, - config - ); + 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"]'); - // Initial inheritance moves the value from the host to the native button. + // 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 remain on the host and are synchronized to native + // 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'); + await expect(host).toHaveAttribute('aria-label', 'second'); await expect(nativeItem).toHaveAttribute('aria-label', 'second'); - // An empty string is a valid ARIA attribute value and remains synchronized. + // An empty string is a valid ARIA attribute value. await host.evaluate((el) => el.setAttribute('aria-label', '')); - await expect(host).toHaveAttribute('aria-label'); await expect(nativeItem).toHaveAttribute('aria-label', ''); - // Native MutationObserver behavior sees a real removal after a post-load write. + // 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 e2f7442036f..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', @@ -191,75 +193,6 @@ export const inheritAriaAttributes = (el: HTMLElement, ignoreList?: string[]) => return inheritAttributes(el, attributesToInherit); }; -export interface AttributeWatcher { - destroy: () => void; -} - -/** - * Watches an element for changes to a given set of attributes and calls - * onChange whenever one changes. Returns null when an attribute is removed. - * Call destroy() from disconnectedCallback to stop watching. - */ -export const watchAttributes = ( - el: HTMLElement, - attributes: string[], - onChange: (changed: { [k: string]: string | null }) => void -): AttributeWatcher => { - if (typeof MutationObserver === 'undefined') { - // Not available in Stencil's mock-doc test environment (used by - // `stencil test --spec`), and, as a defensive fallback, environments - // without native MutationObserver support. - return { destroy: () => {} }; - } - - const observer = new MutationObserver((mutations) => { - const changed: { [k: string]: string | null } = {}; - - for (const mutation of mutations) { - if (mutation.type !== 'attributes' || !mutation.attributeName) continue; - const name = mutation.attributeName; - if (!attributes.includes(name)) continue; - - // getAttribute returns null when the attribute was removed — - // passed through to onChange so consumers can clear the value - // from the native element. - changed[name] = el.getAttribute(name); - } - - if (Object.keys(changed).length > 0) { - onChange(changed); - } - }); - - observer.observe(el, { - attributes: true, - attributeFilter: attributes, - attributeOldValue: true, - }); - - return { destroy: () => observer.disconnect() }; -}; - -/** - * Watches an element for changes to ARIA attributes (and `role`) and invokes - * a callback whenever one is set externally, so that inherited ARIA state - * stays in sync for the lifetime of the component — not just at initial load. - * - * Call this in connectedCallback, alongside the initial inheritAriaAttributes - * call, and call destroy() on the returned watcher in disconnectedCallback. - */ -export const watchForAriaAttributeChanges = ( - el: HTMLElement, - onChange: (changed: { [k: string]: string | null }) => void, - ignoreList?: string[] -): AttributeWatcher => { - let attributesToWatch = ariaAttributes; - if (ignoreList && ignoreList.length > 0) { - attributesToWatch = attributesToWatch.filter((attr) => !ignoreList.includes(attr)); - } - return watchAttributes(el, attributesToWatch, onChange); -}; - export const addEventListener = (el: any, eventName: string, callback: any, opts?: any) => { return el.addEventListener(eventName, callback, opts); }; 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() { ) From 9a63bc26b133837baba580286a7bb2648c7d2b8a Mon Sep 17 00:00:00 2001 From: ShaneK Date: Thu, 17 Sep 2026 08:23:52 -0700 Subject: [PATCH 14/14] docs(card): explain why the leftover aria-label is harmless --- core/src/components/card/card.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/src/components/card/card.tsx b/core/src/components/card/card.tsx index d6e8653a924..5572a82cdf4 100644 --- a/core/src/components/card/card.tsx +++ b/core/src/components/card/card.tsx @@ -88,6 +88,11 @@ export class Card implements ComponentInterface, AnchorInterface, ButtonInterfac @Prop() target: string | undefined; componentWillLoad() { + /** + * 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)); }