From 506ae72840dd02e3f1aec1067310d125fccc5c23 Mon Sep 17 00:00:00 2001 From: anna-lach Date: Thu, 14 May 2026 13:08:52 +0200 Subject: [PATCH 01/11] fix(checkbox-group): fix the checkbox-group to get it working properly with tooltips, unit tests and WithTooltips story --- .../checkbox/src/checkbox-group.spec.ts | 26 ++++++++++++++++ .../checkbox/src/checkbox-group.stories.ts | 30 +++++++++++++++++++ .../components/checkbox/src/checkbox-group.ts | 2 +- 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/packages/components/checkbox/src/checkbox-group.spec.ts b/packages/components/checkbox/src/checkbox-group.spec.ts index cad3f96582..399ea6aa05 100644 --- a/packages/components/checkbox/src/checkbox-group.spec.ts +++ b/packages/components/checkbox/src/checkbox-group.spec.ts @@ -1,5 +1,6 @@ import { type SlFormControlEvent } from '@sl-design-system/form'; import '@sl-design-system/form/register.js'; +import '@sl-design-system/tooltip/register.js'; import { fixture } from '@sl-design-system/vitest-browser-lit'; import { LitElement, type TemplateResult, html } from 'lit'; import { spy } from 'sinon'; @@ -437,4 +438,29 @@ describe('sl-checkbox-group', () => { expect(fitc.shadowRoot!.activeElement).to.equal(input); }); }); + + describe('with tooltips', () => { + beforeEach(async () => { + el = await fixture(html` + + A + B + Tooltip + C + + `); + }); + + it('should only contain checkbox elements in the boxes property', () => { + expect(el.boxes).to.have.lengthOf(3); + expect(el.boxes?.every(box => box.tagName.toLowerCase() === 'sl-checkbox')).to.be.true; + }); + + it('should correctly reflect only checkbox values', async () => { + await userEvent.click(el.querySelector('sl-checkbox[value="b"]')!); + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(el.value).to.deep.equal([null, 'b', null]); + }); + }); }); diff --git a/packages/components/checkbox/src/checkbox-group.stories.ts b/packages/components/checkbox/src/checkbox-group.stories.ts index 8fc5f1e8ed..7a477adc83 100644 --- a/packages/components/checkbox/src/checkbox-group.stories.ts +++ b/packages/components/checkbox/src/checkbox-group.stories.ts @@ -1,6 +1,8 @@ import '@sl-design-system/button/register.js'; import '@sl-design-system/button-bar/register.js'; import '@sl-design-system/form/register.js'; +import { tooltip } from '@sl-design-system/tooltip'; +import '@sl-design-system/tooltip/register.js'; import { type Meta, type StoryObj } from '@storybook/web-components-vite'; import { type TemplateResult, html } from 'lit'; import '../register.js'; @@ -191,3 +193,31 @@ export const CustomAsyncValidity: Story = { } } }; + +export const WithTooltips: Story = { + render: () => { + const onClick = (event: Event & { target: HTMLElement }): void => { + event.target.closest('sl-form')?.reportValidity(); + }; + + return html` + + + + Newsletter + Promotions + Promotions tooltip + Product updates + + + + Report validity + + + `; + } +}; diff --git a/packages/components/checkbox/src/checkbox-group.ts b/packages/components/checkbox/src/checkbox-group.ts index 3d5b6db374..57b5b3b9be 100644 --- a/packages/components/checkbox/src/checkbox-group.ts +++ b/packages/components/checkbox/src/checkbox-group.ts @@ -75,7 +75,7 @@ export class CheckboxGroup extends FormControlMixin(LitElement) { readonly internals = this.attachInternals(); /** @internal The slotted checkboxes. */ - @queryAssignedElements() boxes?: Array>; + @queryAssignedElements({ selector: 'sl-checkbox' }) boxes?: Array>; /** @internal Emits when the component loses focus. */ @event({ name: 'sl-blur' }) blurEvent!: EventEmitter; From 7a95c95fa493893893f79d789f56e004afa01e6f Mon Sep 17 00:00:00 2001 From: anna-lach Date: Thu, 14 May 2026 14:24:40 +0200 Subject: [PATCH 02/11] Checkbox group story changes, new checkbox story with tooltips --- .../checkbox/src/checkbox-group.stories.ts | 6 +++ .../checkbox/src/checkbox.stories.ts | 43 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/packages/components/checkbox/src/checkbox-group.stories.ts b/packages/components/checkbox/src/checkbox-group.stories.ts index 7a477adc83..de866ac7ad 100644 --- a/packages/components/checkbox/src/checkbox-group.stories.ts +++ b/packages/components/checkbox/src/checkbox-group.stories.ts @@ -201,6 +201,12 @@ export const WithTooltips: Story = { }; return html` +

+ This story demonstrates how to use tooltips with checkboxes inside a checkbox group using + both the tooltip() directive and the manual + aria-describedby approach with a separate sl-tooltip element. +

+ diff --git a/packages/components/checkbox/src/checkbox.stories.ts b/packages/components/checkbox/src/checkbox.stories.ts index fd7fe9a664..5aaa5171be 100644 --- a/packages/components/checkbox/src/checkbox.stories.ts +++ b/packages/components/checkbox/src/checkbox.stories.ts @@ -1,6 +1,8 @@ import '@sl-design-system/button/register.js'; import '@sl-design-system/button-bar/register.js'; import '@sl-design-system/form/register.js'; +import { tooltip } from '@sl-design-system/tooltip'; +import '@sl-design-system/tooltip/register.js'; import { type Meta, type StoryObj } from '@storybook/web-components-vite'; import { type TemplateResult, html, nothing } from 'lit'; import { ifDefined } from 'lit/directives/if-defined.js'; @@ -314,3 +316,44 @@ export const CustomAsyncValidity: Story = { } } }; + +export const WithTooltip: Story = { + render: () => { + const onClick = (event: Event & { target: HTMLElement }): void => { + event.target.closest('sl-form')?.reportValidity(); + }; + + return html` +

+ This story demonstrates how to use tooltips with checkboxes using both the + tooltip() directive and the manual aria-describedby approach with + a separate sl-tooltip element. +

+ +

Using the tooltip directive

+ + + Newsletter + + + Report validity + + + +

Using aria-describedby

+ + + Promotions + Promotions tooltip + + + Report validity + + + `; + } +}; From 08cd838be8459ab638536305be49cab94e9d33b6 Mon Sep 17 00:00:00 2001 From: anna-lach Date: Thu, 14 May 2026 14:40:53 +0200 Subject: [PATCH 03/11] Changeset --- .changeset/empty-cougars-cross.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/empty-cougars-cross.md diff --git a/.changeset/empty-cougars-cross.md b/.changeset/empty-cougars-cross.md new file mode 100644 index 0000000000..a5c44f056a --- /dev/null +++ b/.changeset/empty-cougars-cross.md @@ -0,0 +1,5 @@ +--- +'@sl-design-system/checkbox': patch +--- + +Fix checkbox group not working with tooltips. From 71c44e8ac1e6b36774f16042b75b81810650130c Mon Sep 17 00:00:00 2001 From: Anna Sobczak <111562742+anna-lach@users.noreply.github.com> Date: Thu, 14 May 2026 15:03:39 +0200 Subject: [PATCH 04/11] changeset update --- .changeset/empty-cougars-cross.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/empty-cougars-cross.md b/.changeset/empty-cougars-cross.md index a5c44f056a..f63e02871a 100644 --- a/.changeset/empty-cougars-cross.md +++ b/.changeset/empty-cougars-cross.md @@ -2,4 +2,4 @@ '@sl-design-system/checkbox': patch --- -Fix checkbox group not working with tooltips. +Fix checkbox group not working with tooltips (only `sl-checkbox` elements are tracked by `@queryAssignedElements`). From dc4dc052828ab381e094c2876aeb85dee5c970a1 Mon Sep 17 00:00:00 2001 From: anna-lach Date: Fri, 15 May 2026 12:46:08 +0200 Subject: [PATCH 05/11] Get the single checkbox working with tooltip --- .changeset/empty-cougars-cross.md | 1 + .../components/checkbox/src/checkbox.spec.ts | 16 ++++++++++++++++ packages/components/checkbox/src/checkbox.ts | 14 ++++++++++++-- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/.changeset/empty-cougars-cross.md b/.changeset/empty-cougars-cross.md index f63e02871a..68d7de66b6 100644 --- a/.changeset/empty-cougars-cross.md +++ b/.changeset/empty-cougars-cross.md @@ -3,3 +3,4 @@ --- Fix checkbox group not working with tooltips (only `sl-checkbox` elements are tracked by `@queryAssignedElements`). +Forward `aria-describedby` to the inner input element for screen reader support. diff --git a/packages/components/checkbox/src/checkbox.spec.ts b/packages/components/checkbox/src/checkbox.spec.ts index ffd7c7e591..a1aba31fe4 100644 --- a/packages/components/checkbox/src/checkbox.spec.ts +++ b/packages/components/checkbox/src/checkbox.spec.ts @@ -23,6 +23,10 @@ describe('sl-checkbox', () => { expect(input.type).to.equal('checkbox'); }); + it('should have aria-describedby in observedAttributes', () => { + expect((el.constructor as typeof Checkbox).observedAttributes).to.include('aria-describedby'); + }); + it('should not be checked', () => { expect(el.checked).not.to.be.true; expect(input.checked).not.to.be.true; @@ -128,6 +132,18 @@ describe('sl-checkbox', () => { expect(el.input).to.have.attribute('aria-labelledby', 'id'); }); + it('should proxy the aria-describedby attribute to the input element', async () => { + el.setAttribute('aria-describedby', 'tooltip-id'); + await new Promise(resolve => setTimeout(resolve, 50)); + + expect(el).to.not.have.attribute('aria-describedby'); + expect(el.input).to.have.attribute('aria-describedby', 'tooltip-id'); + }); + + it('should return the input element from getProxyTarget', () => { + expect(el.getProxyTarget()).to.equal(el.input); + }); + it('should be pristine', () => { expect(el.dirty).not.to.be.true; }); diff --git a/packages/components/checkbox/src/checkbox.ts b/packages/components/checkbox/src/checkbox.ts index bc2d9528cf..f007dd22e9 100644 --- a/packages/components/checkbox/src/checkbox.ts +++ b/packages/components/checkbox/src/checkbox.ts @@ -46,6 +46,7 @@ let nextUniqueId = 0; @localized() // eslint-disable-next-line @typescript-eslint/no-explicit-any export class Checkbox extends ObserveAttributesMixin(FormControlMixin(LitElement), [ + 'aria-describedby', 'aria-disabled', 'aria-label', 'aria-labelledby' @@ -166,6 +167,7 @@ export class Checkbox extends ObserveAttributesMixin(FormControlMixin(L } this.setFormControlElement(this.input); + this.setAttributesTarget(this.input); this.#onLabelSlotChange(); } @@ -221,6 +223,15 @@ export class Checkbox extends ObserveAttributesMixin(FormControlMixin(L this.input.blur(); } + /** + * Returns the proxy target for the tooltip to anchor to. + * + * @internal + */ + getProxyTarget(): HTMLInputElement { + return this.input; + } + override getLocalizedValidationMessage(): string { if (!this.validity.customError && this.validity.valueMissing) { return msg('Please check this box.', { id: 'sl.checkbox.validation.valueMissing' }); @@ -283,6 +294,7 @@ export class Checkbox extends ObserveAttributesMixin(FormControlMixin(L this.#syncInput(this.input); this.setFormControlElement(this.input); + this.setAttributesTarget(this.input); } } @@ -336,7 +348,5 @@ export class Checkbox extends ObserveAttributesMixin(FormControlMixin(L input.checked = !!this.checked; input.indeterminate = !!this.indeterminate; - - this.setAttributesTarget(input); } } From 7311a370b6ee0af36c5495c433f2dddcfd8032aa Mon Sep 17 00:00:00 2001 From: anna-lach Date: Fri, 15 May 2026 13:22:33 +0200 Subject: [PATCH 06/11] Using ForwardAriaMixin in checkbox --- .changeset/empty-cougars-cross.md | 2 +- .../components/checkbox/src/checkbox.spec.ts | 26 ++++++++++++++----- packages/components/checkbox/src/checkbox.ts | 17 +++--------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/.changeset/empty-cougars-cross.md b/.changeset/empty-cougars-cross.md index 68d7de66b6..ae4891c710 100644 --- a/.changeset/empty-cougars-cross.md +++ b/.changeset/empty-cougars-cross.md @@ -3,4 +3,4 @@ --- Fix checkbox group not working with tooltips (only `sl-checkbox` elements are tracked by `@queryAssignedElements`). -Forward `aria-describedby` to the inner input element for screen reader support. +Use `ForwardAriaMixin` to forward `aria-describedby` as element references to the inner input element, enabling screen reader support across shadow DOM boundaries. diff --git a/packages/components/checkbox/src/checkbox.spec.ts b/packages/components/checkbox/src/checkbox.spec.ts index a1aba31fe4..8a153e1d2c 100644 --- a/packages/components/checkbox/src/checkbox.spec.ts +++ b/packages/components/checkbox/src/checkbox.spec.ts @@ -124,20 +124,34 @@ describe('sl-checkbox', () => { expect(el.input).to.have.attribute('aria-label', 'Label'); }); - it('should proxy the aria-labelledby attribute to the input element', async () => { - el.setAttribute('aria-labelledby', 'id'); + it('should proxy the aria-labelledby to ariaLabelledByElements on the input element', async () => { + const label = document.createElement('span'); + label.id = 'my-label'; + label.textContent = 'My label'; + el.parentElement!.appendChild(label); + + el.setAttribute('aria-labelledby', 'my-label'); await new Promise(resolve => setTimeout(resolve, 50)); expect(el).to.not.have.attribute('aria-labelledby'); - expect(el.input).to.have.attribute('aria-labelledby', 'id'); + expect(el.input.ariaLabelledByElements).to.deep.equal([label]); + + label.remove(); }); - it('should proxy the aria-describedby attribute to the input element', async () => { - el.setAttribute('aria-describedby', 'tooltip-id'); + it('should proxy the aria-describedby to ariaDescribedByElements on the input element', async () => { + const tooltip = document.createElement('span'); + tooltip.id = 'my-tooltip'; + tooltip.textContent = 'My tooltip'; + el.parentElement!.appendChild(tooltip); + + el.setAttribute('aria-describedby', 'my-tooltip'); await new Promise(resolve => setTimeout(resolve, 50)); expect(el).to.not.have.attribute('aria-describedby'); - expect(el.input).to.have.attribute('aria-describedby', 'tooltip-id'); + expect(el.input.ariaDescribedByElements).to.deep.equal([tooltip]); + + tooltip.remove(); }); it('should return the input element from getProxyTarget', () => { diff --git a/packages/components/checkbox/src/checkbox.ts b/packages/components/checkbox/src/checkbox.ts index f007dd22e9..413f3beaa3 100644 --- a/packages/components/checkbox/src/checkbox.ts +++ b/packages/components/checkbox/src/checkbox.ts @@ -3,7 +3,7 @@ import { FormControlMixin } from '@sl-design-system/form'; import { type EventEmitter, EventsController, - ObserveAttributesMixin, + ForwardAriaMixin, event } from '@sl-design-system/shared'; import { @@ -45,7 +45,7 @@ let nextUniqueId = 0; */ @localized() // eslint-disable-next-line @typescript-eslint/no-explicit-any -export class Checkbox extends ObserveAttributesMixin(FormControlMixin(LitElement), [ +export class Checkbox extends ForwardAriaMixin(FormControlMixin(LitElement), [ 'aria-describedby', 'aria-disabled', 'aria-label', @@ -167,7 +167,7 @@ export class Checkbox extends ObserveAttributesMixin(FormControlMixin(L } this.setFormControlElement(this.input); - this.setAttributesTarget(this.input); + this.setProxyTarget(this.input); this.#onLabelSlotChange(); } @@ -223,15 +223,6 @@ export class Checkbox extends ObserveAttributesMixin(FormControlMixin(L this.input.blur(); } - /** - * Returns the proxy target for the tooltip to anchor to. - * - * @internal - */ - getProxyTarget(): HTMLInputElement { - return this.input; - } - override getLocalizedValidationMessage(): string { if (!this.validity.customError && this.validity.valueMissing) { return msg('Please check this box.', { id: 'sl.checkbox.validation.valueMissing' }); @@ -294,7 +285,7 @@ export class Checkbox extends ObserveAttributesMixin(FormControlMixin(L this.#syncInput(this.input); this.setFormControlElement(this.input); - this.setAttributesTarget(this.input); + this.setProxyTarget(this.input); } } From 7582f375f6aaab3da1ba3a304c44a2444cbafecb Mon Sep 17 00:00:00 2001 From: anna-lach Date: Mon, 18 May 2026 16:01:09 +0200 Subject: [PATCH 07/11] New example of checkbox with tooltip connected via aria-labelledby, trying to get tooltips working properly with checkboxes --- .../checkbox/src/checkbox.stories.ts | 93 ++++++++++++------- packages/components/checkbox/src/checkbox.ts | 6 +- .../shared/src/mixins/forward-aria-mixin.ts | 82 ++++++++++++---- packages/components/tooltip/src/tooltip.ts | 25 ++++- 4 files changed, 153 insertions(+), 53 deletions(-) diff --git a/packages/components/checkbox/src/checkbox.stories.ts b/packages/components/checkbox/src/checkbox.stories.ts index 5aaa5171be..2f99f58c75 100644 --- a/packages/components/checkbox/src/checkbox.stories.ts +++ b/packages/components/checkbox/src/checkbox.stories.ts @@ -87,8 +87,9 @@ export default { .showValid=${showValid} .value=${value} size=${ifDefined(size)} - >${text} + ${text} + `} ${reportValidity @@ -195,21 +196,21 @@ export const Indeterminate: StoryObj = {
  • - Andre + + Andre +
  • - Paul Bunyan + + Paul Bunyan +
  • - Two sandwiches + + Two sandwiches +
    • @@ -220,14 +221,14 @@ export const Indeterminate: StoryObj = { Smurfs
  • - Mushrooms + + Mushrooms +
  • - One Sandwich + + One Sandwich +
    • @@ -241,8 +242,10 @@ export const NoVisibleLabel: StoryObj = { return html`

    This checkbox has no internal or external label. It only has an - aria-label attribute. That attribute is automatically applied to the - input element. + aria-label + attribute. That attribute is automatically applied to the + input + element.

    `; @@ -283,9 +286,9 @@ export const CustomValidity: Story = { }; return html` - I agree to all terms & conditions + + I agree to all terms & conditions + `; } } @@ -309,9 +312,9 @@ export const CustomAsyncValidity: Story = { }; return html` - I agree to all terms & conditions + + I agree to all terms & conditions + `; } } @@ -325,17 +328,26 @@ export const WithTooltip: Story = { return html`

    - This story demonstrates how to use tooltips with checkboxes using both the - tooltip() directive and the manual aria-describedby approach with - a separate sl-tooltip element. + This story demonstrates how to use tooltips with checkboxes using the + tooltip() + directive, + aria-describedby + , and + aria-labelledby + with a separate + sl-tooltip + element. The + ForwardAriaMixin + resolves IDs to element references and forwards them to the focusable input element, which + properly crosses the shadow DOM boundary.

    Using the tooltip directive

    - Newsletter + + Newsletter + Report validity @@ -345,10 +357,23 @@ export const WithTooltip: Story = {

    Using aria-describedby

    - Promotions - Promotions tooltip + + Promotions + + Promotions tooltip + + + Report validity + + + +

    Using aria-labelledby

    + + + + Updates + + Updates tooltip Report validity diff --git a/packages/components/checkbox/src/checkbox.ts b/packages/components/checkbox/src/checkbox.ts index 413f3beaa3..c447fde11d 100644 --- a/packages/components/checkbox/src/checkbox.ts +++ b/packages/components/checkbox/src/checkbox.ts @@ -318,7 +318,11 @@ export class Checkbox extends ForwardAriaMixin(FormControlMixin(LitElem } requestAnimationFrame(() => { - if (!this.input.hasAttribute('aria-labelledby') && this.input.labels?.length) { + if ( + !this.input.hasAttribute('aria-labelledby') && + !this.input.ariaLabelledByElements?.length && + this.input.labels?.length + ) { this.input.setAttribute( 'aria-labelledby', Array.from(this.input.labels) diff --git a/packages/components/shared/src/mixins/forward-aria-mixin.ts b/packages/components/shared/src/mixins/forward-aria-mixin.ts index 0fa2ba6dac..61b312fdbb 100644 --- a/packages/components/shared/src/mixins/forward-aria-mixin.ts +++ b/packages/components/shared/src/mixins/forward-aria-mixin.ts @@ -80,6 +80,12 @@ export function ForwardAriaMixin< #observer?: MutationObserver; #pendingAttributes = new Set(); + /** + * Stores attribute values at the time they were observed, as a fallback when Lit's ARIA + * reflection removes the attribute before resolution. + */ + #storedValues = new Map(); + static override get observedAttributes(): string[] { return [...(super.observedAttributes ?? []), ...(observedAttributes ?? [])]; } @@ -97,7 +103,11 @@ export function ForwardAriaMixin< const stored = propertyStorage.get(this); if (stored) { for (const [prop, value] of stored) { - (target as unknown as Record)[prop] = value; + // Skip null/empty values to avoid adding empty aria-* content attributes + const isEmpty = value === null || (Array.isArray(value) && value.length === 0); + if (!isEmpty) { + (target as unknown as Record)[prop] = value; + } } } @@ -123,16 +133,21 @@ export function ForwardAriaMixin< // before the element upgraded or before the target is available). if (!observedAttributes) { // Collect any aria-* attributes already present on the element - for (const { name } of this.attributes) { + for (const { name, value } of this.attributes) { if (name.startsWith('aria-')) { this.#pendingAttributes.add(name); + this.#storedValues.set(name, value); } } this.#observer = new MutationObserver(mutations => { for (const { attributeName } of mutations) { if (attributeName?.startsWith('aria-')) { - this.#pendingAttributes.add(attributeName); + const value = this.getAttribute(attributeName); + if (value !== null) { + this.#pendingAttributes.add(attributeName); + this.#storedValues.set(attributeName, value); + } } } @@ -158,8 +173,9 @@ export function ForwardAriaMixin< // Only intercept attributes from our explicit list (the MutationObserver // path handles the "observe everything" case in connectedCallback). - if (observedAttributes?.includes(name) && newValue !== null) { + if (observedAttributes?.includes(name) && newValue) { this.#pendingAttributes.add(name); + this.#storedValues.set(name, newValue); this.#forwardAttributes(); } } @@ -174,9 +190,12 @@ export function ForwardAriaMixin< // Resolve IDs from the host's root (light DOM document or parent shadow root), // not from our own shadow root. const root = this.getRootNode() as Document | ShadowRoot; + const unresolved = new Set(); for (const name of this.#pendingAttributes) { - const value = this.getAttribute(name); + // Read from the host attribute first; fall back to stored value if Lit's + // built-in ARIA reflection removed the attribute before we could process it. + const value = this.getAttribute(name) ?? this.#storedValues.get(name); if (!value) { continue; } @@ -185,25 +204,48 @@ export function ForwardAriaMixin< if (elementsProp) { // Reference attribute: resolve space-separated IDs to DOM elements and // assign via the element reference property (singular or plural). - const elements = value - .split(/\s+/) - .map(id => root.querySelector(`#${CSS.escape(id)}`)) - .filter((el): el is HTMLElement => el !== null); - - if (elementsProp.endsWith('Elements')) { - (targetElement as unknown as Record)[elementsProp] = elements; - } else { - (targetElement as unknown as Record)[elementsProp] = - elements[0] ?? null; + const ids = value.split(/\s+/), + elements = ids + .map(id => root.querySelector(`#${CSS.escape(id)}`)) + .filter((el): el is HTMLElement => el !== null); + + // If not all IDs could be resolved, defer until the referenced elements + // are in the DOM (e.g. when a sibling is connected after this element). + if (elements.length < ids.length) { + unresolved.add(name); + continue; + } + + // Mirror the element reference in propertyStorage so that external code + // (e.g. tooltip anchor matching) can discover the relation via the host. + let stored = propertyStorage.get(this); + if (!stored) { + stored = new Map(); + propertyStorage.set(this, stored); } + const refValue = elementsProp.endsWith('Elements') ? elements : (elements[0] ?? null); + stored.set(elementsProp, refValue); + + // Set element references on the target for cross-shadow-DOM accessibility. + (targetElement as unknown as Record)[elementsProp] = + refValue; } else { targetElement.setAttribute(name, value); } this.removeAttribute(name); + this.#storedValues.delete(name); } - this.#pendingAttributes.clear(); + this.#pendingAttributes = unresolved; + + // If some IDs could not be resolved, retry to give sibling elements time to + // connect to the DOM. Use both setTimeout (new macrotask, after all synchronous + // rendering) and rAF (next frame) to cover different rendering pipelines. + if (unresolved.size > 0) { + setTimeout(() => this.#forwardAttributes(), 0); + requestAnimationFrame(() => this.#forwardAttributes()); + } } } @@ -252,7 +294,13 @@ export function ForwardAriaMixin< const target = targetElements.get(this); if (target) { - (target as unknown as Record)[prop] = value; + // Only forward non-empty values to the target. Forwarding null or empty arrays + // can cause the browser to add an empty aria-describedby/aria-labelledby content + // attribute on the target, which interferes with deferred ID resolution. + const isEmpty = value === null || (Array.isArray(value) && value.length === 0); + if (!isEmpty) { + (target as unknown as Record)[prop] = value; + } } } }); diff --git a/packages/components/tooltip/src/tooltip.ts b/packages/components/tooltip/src/tooltip.ts index 7e093cda5f..8246e44537 100644 --- a/packages/components/tooltip/src/tooltip.ts +++ b/packages/components/tooltip/src/tooltip.ts @@ -118,7 +118,7 @@ export class Tooltip extends LitElement { } }; - const createTooltip = (): void => { + const createTooltip = (event?: Event): void => { if (created) { return; } @@ -166,6 +166,15 @@ export class Tooltip extends LitElement { // We only need to create the tooltip once, so ignore all future events. removeListeners(); + + // When the tooltip is created in response to a focusin event, the tooltip's own + // document-level focusin listener missed the original event. Re-dispatch a synthetic + // focusin so the tooltip's normal show logic kicks in. + if (event?.type === 'focusin') { + requestAnimationFrame(() => { + target.dispatchEvent(new FocusEvent('focusin', { bubbles: true, composed: true })); + }); + } }; const cleanup = () => { @@ -1313,6 +1322,20 @@ export class Tooltip extends LitElement { while (true) { const rootNode = this.#getShadowRoot(normalized.getRootNode()); if (!rootNode) { + // For light DOM proxy targets: if the element's parent has getProxyTarget() + // pointing to this element, normalize to the parent (e.g. an in the + // light DOM of sl-checkbox where sl-checkbox is the actual anchor). + const parent = normalized.parentElement; + if (parent instanceof HTMLElement) { + const parentProxy = ( + parent as HTMLElement & { getProxyTarget?(): Element | null } + ).getProxyTarget?.(); + + if (parentProxy === normalized && this.#matchesAnchor(parent)) { + return parent; + } + } + return normalized; } From 0e5b560aaa058f18026a7e4f6de430b4b7b7861a Mon Sep 17 00:00:00 2001 From: anna-lach Date: Tue, 19 May 2026 11:14:04 +0200 Subject: [PATCH 08/11] Tooltip connected to checkbox via aria-describedby and aria-labelledby (issues with plain label) --- packages/components/checkbox/src/checkbox.ts | 2 + .../shared/src/mixins/forward-aria-mixin.ts | 67 +++++++++++++++++-- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/packages/components/checkbox/src/checkbox.ts b/packages/components/checkbox/src/checkbox.ts index c447fde11d..59e0efbd76 100644 --- a/packages/components/checkbox/src/checkbox.ts +++ b/packages/components/checkbox/src/checkbox.ts @@ -319,8 +319,10 @@ export class Checkbox extends ForwardAriaMixin(FormControlMixin(LitElem requestAnimationFrame(() => { if ( + !this.hasAttribute('aria-labelledby') && !this.input.hasAttribute('aria-labelledby') && !this.input.ariaLabelledByElements?.length && + !this.ariaLabelledByElements?.length && this.input.labels?.length ) { this.input.setAttribute( diff --git a/packages/components/shared/src/mixins/forward-aria-mixin.ts b/packages/components/shared/src/mixins/forward-aria-mixin.ts index 61b312fdbb..d5701f8166 100644 --- a/packages/components/shared/src/mixins/forward-aria-mixin.ts +++ b/packages/components/shared/src/mixins/forward-aria-mixin.ts @@ -99,13 +99,37 @@ export function ForwardAriaMixin< setProxyTarget(target: HTMLElement): void { targetElements.set(this, target); - // Forward any element reference properties that were set before the target was available + // Forward any element reference properties that were set before the target was available. + // Apply the same scope-aware logic as #forwardAttributes: when the target shares the + // same root as the host, use string attributes (IDs) instead of element references, + // because Chrome clears the content attribute when ariaLabelledByElements is assigned. const stored = propertyStorage.get(this); if (stored) { + const root = this.getRootNode(); + const targetRoot = target.getRootNode(); + for (const [prop, value] of stored) { // Skip null/empty values to avoid adding empty aria-* content attributes const isEmpty = value === null || (Array.isArray(value) && value.length === 0); - if (!isEmpty) { + if (isEmpty) { + continue; + } + + if (targetRoot === root) { + // Same scope: convert element references to a string ID attribute. + const attrName = Object.entries(ELEMENT_REFERENCES).find(([, p]) => p === prop)?.[0]; + if (attrName) { + const elements = Array.isArray(value) ? value : [value]; + const ids = elements + .map(el => (el as HTMLElement).id) + .filter(Boolean) + .join(' '); + if (ids) { + target.setAttribute(attrName, ids); + } + } + } else { + // Cross-scope: element references are required. (target as unknown as Record)[prop] = value; } } @@ -226,9 +250,21 @@ export function ForwardAriaMixin< const refValue = elementsProp.endsWith('Elements') ? elements : (elements[0] ?? null); stored.set(elementsProp, refValue); - // Set element references on the target for cross-shadow-DOM accessibility. - (targetElement as unknown as Record)[elementsProp] = - refValue; + // Determine the target's root to decide how to forward the relationship. + const targetRoot = targetElement.getRootNode(); + + if (targetRoot === root) { + // Same scope: the string attribute reliably resolves IDs. Do NOT set + // element reference properties here — Chrome replaces the content attribute + // with an empty marker when ariaLabelledByElements/ariaDescribedByElements + // is assigned, which breaks the accessibility tree. + targetElement.setAttribute(name, value); + } else { + // Cross-scope (target is inside a shadow root): string IDs cannot resolve + // across the shadow boundary, so element references are required. + (targetElement as unknown as Record)[elementsProp] = + refValue; + } } else { targetElement.setAttribute(name, value); } @@ -299,7 +335,26 @@ export function ForwardAriaMixin< // attribute on the target, which interferes with deferred ID resolution. const isEmpty = value === null || (Array.isArray(value) && value.length === 0); if (!isEmpty) { - (target as unknown as Record)[prop] = value; + const root = (this as unknown as Element).getRootNode(); + const targetRoot = target.getRootNode(); + + if (targetRoot === root) { + // Same scope: use string attribute to avoid Chrome clearing it. + const attrName = Object.entries(ELEMENT_REFERENCES).find(([, p]) => p === prop)?.[0]; + if (attrName) { + const elements = Array.isArray(value) ? value : [value]; + const ids = elements + .map(el => (el as HTMLElement).id) + .filter(Boolean) + .join(' '); + if (ids) { + target.setAttribute(attrName, ids); + } + } + } else { + // Cross-scope: element references required. + (target as unknown as Record)[prop] = value; + } } } } From a711b3c1d8447ed44064b7ca43fcfd588588a672 Mon Sep 17 00:00:00 2001 From: anna-lach Date: Tue, 19 May 2026 12:05:01 +0200 Subject: [PATCH 09/11] Fix the tooltip connected to checkbox via aria-labelledby, working with labels properly --- .../components/checkbox/src/checkbox.spec.ts | 26 ++++++++++---- packages/components/checkbox/src/checkbox.ts | 35 +++++++++++++------ 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/packages/components/checkbox/src/checkbox.spec.ts b/packages/components/checkbox/src/checkbox.spec.ts index 8a153e1d2c..946ecf6d74 100644 --- a/packages/components/checkbox/src/checkbox.spec.ts +++ b/packages/components/checkbox/src/checkbox.spec.ts @@ -13,7 +13,9 @@ describe('sl-checkbox', () => { describe('defaults', () => { beforeEach(async () => { - el = await fixture(html`Hello world`); + el = await fixture(html` + Hello world + `); input = el.querySelector('input')!; }); @@ -134,7 +136,11 @@ describe('sl-checkbox', () => { await new Promise(resolve => setTimeout(resolve, 50)); expect(el).to.not.have.attribute('aria-labelledby'); - expect(el.input.ariaLabelledByElements).to.deep.equal([label]); + + // In same-scope mode, ForwardAriaMixin uses string attributes and + // #onLabelSlotChange appends the checkbox's own label IDs. + const ids = el.input.getAttribute('aria-labelledby')!.split(/\s+/); + expect(ids).to.include('my-label'); label.remove(); }); @@ -149,7 +155,9 @@ describe('sl-checkbox', () => { await new Promise(resolve => setTimeout(resolve, 50)); expect(el).to.not.have.attribute('aria-describedby'); - expect(el.input.ariaDescribedByElements).to.deep.equal([tooltip]); + + // In same-scope mode, ForwardAriaMixin uses string attributes. + expect(el.input.getAttribute('aria-describedby')).to.equal('my-tooltip'); tooltip.remove(); }); @@ -314,7 +322,9 @@ describe('sl-checkbox', () => { describe('disabled', () => { beforeEach(async () => { - el = await fixture(html`Hello world`); + el = await fixture(html` + Hello world + `); input = el.querySelector('input')!; }); @@ -353,7 +363,9 @@ describe('sl-checkbox', () => { describe('validation', () => { beforeEach(async () => { - el = await fixture(html`Hello world`); + el = await fixture(html` + Hello world + `); }); it('should be invalid when required and no option is selected', async () => { @@ -464,7 +476,9 @@ describe('sl-checkbox', () => { // empty } - el = await fixture(html``); + el = await fixture(html` + + `); }); it('should emit an sl-form-control event after first render', () => { diff --git a/packages/components/checkbox/src/checkbox.ts b/packages/components/checkbox/src/checkbox.ts index 59e0efbd76..609c5ab30d 100644 --- a/packages/components/checkbox/src/checkbox.ts +++ b/packages/components/checkbox/src/checkbox.ts @@ -319,18 +319,31 @@ export class Checkbox extends ForwardAriaMixin(FormControlMixin(LitElem requestAnimationFrame(() => { if ( - !this.hasAttribute('aria-labelledby') && - !this.input.hasAttribute('aria-labelledby') && - !this.input.ariaLabelledByElements?.length && - !this.ariaLabelledByElements?.length && - this.input.labels?.length + (this.hasAttribute('aria-labelledby') || this.ariaLabelledByElements?.length) && + !this.input.labels?.length ) { - this.input.setAttribute( - 'aria-labelledby', - Array.from(this.input.labels) - .map(label => label.id) - .join(' ') - ); + // ForwardAriaMixin will handle it; no labels to append. + return; + } + + if (this.input.labels?.length) { + const labelIds = Array.from(this.input.labels) + .map(label => label.id) + .filter(Boolean) + .join(' '); + + const existing = this.input.getAttribute('aria-labelledby') ?? ''; + const existingIds = existing.split(/\s+/).filter(Boolean); + + // Append label IDs that aren't already present + const newIds = labelIds.split(/\s+/).filter(id => id && !existingIds.includes(id)); + + if (newIds.length) { + const combined = [...existingIds, ...newIds].join(' '); + this.input.setAttribute('aria-labelledby', combined); + } else if (!existing && labelIds) { + this.input.setAttribute('aria-labelledby', labelIds); + } } }); From 662975028fcb9ad51dd78d29e1ab8106dd9128f0 Mon Sep 17 00:00:00 2001 From: anna-lach Date: Tue, 19 May 2026 15:51:14 +0200 Subject: [PATCH 10/11] Changes after Copilot review --- .../shared/src/mixins/forward-aria-mixin.ts | 83 +++++++++++++++---- packages/components/tooltip/src/tooltip.ts | 9 +- 2 files changed, 74 insertions(+), 18 deletions(-) diff --git a/packages/components/shared/src/mixins/forward-aria-mixin.ts b/packages/components/shared/src/mixins/forward-aria-mixin.ts index d5701f8166..97e02f0e7e 100644 --- a/packages/components/shared/src/mixins/forward-aria-mixin.ts +++ b/packages/components/shared/src/mixins/forward-aria-mixin.ts @@ -1,6 +1,17 @@ import { type Constructor } from '@open-wc/dedupe-mixin'; import { type ReactiveElement } from 'lit'; +/** Counter for generating stable unique IDs for elements that lack one. */ +let nextAutoId = 0; + +/** Ensures the element has an `id` attribute, auto-assigning one if missing. */ +function ensureId(el: HTMLElement): string { + if (!el.id) { + el.id = `sl-aria-ref-${nextAutoId++}`; + } + return el.id; +} + export interface ForwardAriaMixinInterface { getProxyTarget(): HTMLElement | undefined; setProxyTarget(target: HTMLElement): void; @@ -77,9 +88,15 @@ export function ForwardAriaMixin< ariaDisabledStorage = new WeakMap(); class ForwardAriaImpl extends constructor { + /** Maximum number of deferred retries before giving up on unresolved IDs. */ + static readonly #MAX_RETRIES = 3; + #observer?: MutationObserver; #pendingAttributes = new Set(); + /** Tracks the number of deferred resolution attempts per attribute. */ + #retryCounts = new Map(); + /** * Stores attribute values at the time they were observed, as a fallback when Lit's ARIA * reflection removes the attribute before resolution. @@ -120,10 +137,7 @@ export function ForwardAriaMixin< const attrName = Object.entries(ELEMENT_REFERENCES).find(([, p]) => p === prop)?.[0]; if (attrName) { const elements = Array.isArray(value) ? value : [value]; - const ids = elements - .map(el => (el as HTMLElement).id) - .filter(Boolean) - .join(' '); + const ids = elements.map(el => ensureId(el as HTMLElement)).join(' '); if (ids) { target.setAttribute(attrName, ids); } @@ -193,13 +207,23 @@ export function ForwardAriaMixin< oldValue: string | null, newValue: string | null ): void { + // Store the value BEFORE calling super, since Lit's ARIA reflection may + // remove or empty the attribute during super.attributeChangedCallback. + if (observedAttributes?.includes(name) && newValue) { + this.#pendingAttributes.add(name); + this.#storedValues.set(name, newValue); + } + super.attributeChangedCallback(name, oldValue, newValue); // Only intercept attributes from our explicit list (the MutationObserver // path handles the "observe everything" case in connectedCallback). if (observedAttributes?.includes(name) && newValue) { - this.#pendingAttributes.add(name); - this.#storedValues.set(name, newValue); + // Attempt to forward; if #pendingAttributes was already consumed (e.g. by a + // re-entrant call triggered from super), re-add and retry. + if (!this.#pendingAttributes.has(name)) { + this.#pendingAttributes.add(name); + } this.#forwardAttributes(); } } @@ -218,8 +242,9 @@ export function ForwardAriaMixin< for (const name of this.#pendingAttributes) { // Read from the host attribute first; fall back to stored value if Lit's - // built-in ARIA reflection removed the attribute before we could process it. - const value = this.getAttribute(name) ?? this.#storedValues.get(name); + // built-in ARIA reflection removed the attribute (or set it to empty string) + // before we could process it. + const value = this.getAttribute(name) || this.#storedValues.get(name); if (!value) { continue; } @@ -235,11 +260,26 @@ export function ForwardAriaMixin< // If not all IDs could be resolved, defer until the referenced elements // are in the DOM (e.g. when a sibling is connected after this element). - if (elements.length < ids.length) { - unresolved.add(name); - continue; + // Only defer when at least one element was found (partial resolution), + // indicating siblings are still rendering. When nothing resolves at all, + // forward immediately so consumers get a deterministic empty result. + // After MAX_RETRIES attempts, give up and forward whatever was resolved + // so permanently-missing IDs don't block indefinitely. + if (elements.length < ids.length && elements.length > 0) { + const retries = this.#retryCounts.get(name) ?? 0; + if (retries < ForwardAriaImpl.#MAX_RETRIES) { + this.#retryCounts.set(name, retries + 1); + unresolved.add(name); + continue; + } + // Fallback: forward partially-resolved elements. + // Clean up retry state since we're done trying. + this.#retryCounts.delete(name); } + // Clean up retry count on successful resolution. + this.#retryCounts.delete(name); + // Mirror the element reference in propertyStorage so that external code // (e.g. tooltip anchor matching) can discover the relation via the host. let stored = propertyStorage.get(this); @@ -250,6 +290,22 @@ export function ForwardAriaMixin< const refValue = elementsProp.endsWith('Elements') ? elements : (elements[0] ?? null); stored.set(elementsProp, refValue); + // If no elements could be resolved at all and the target is in the same + // scope, just remove the attribute from the host — there's nothing useful + // to set as a string attribute. For cross-scope targets, still forward the + // empty array so consumers reading the native property get a deterministic []. + if (elements.length === 0) { + const targetRoot = targetElement.getRootNode(); + if (targetRoot !== root) { + (targetElement as unknown as Record)[ + elementsProp + ] = refValue; + } + this.removeAttribute(name); + this.#storedValues.delete(name); + continue; + } + // Determine the target's root to decide how to forward the relationship. const targetRoot = targetElement.getRootNode(); @@ -343,10 +399,7 @@ export function ForwardAriaMixin< const attrName = Object.entries(ELEMENT_REFERENCES).find(([, p]) => p === prop)?.[0]; if (attrName) { const elements = Array.isArray(value) ? value : [value]; - const ids = elements - .map(el => (el as HTMLElement).id) - .filter(Boolean) - .join(' '); + const ids = elements.map(el => ensureId(el as HTMLElement)).join(' '); if (ids) { target.setAttribute(attrName, ids); } diff --git a/packages/components/tooltip/src/tooltip.ts b/packages/components/tooltip/src/tooltip.ts index 8246e44537..90e09c65ab 100644 --- a/packages/components/tooltip/src/tooltip.ts +++ b/packages/components/tooltip/src/tooltip.ts @@ -168,11 +168,14 @@ export class Tooltip extends LitElement { removeListeners(); // When the tooltip is created in response to a focusin event, the tooltip's own - // document-level focusin listener missed the original event. Re-dispatch a synthetic - // focusin so the tooltip's normal show logic kicks in. + // document-level focusin listener missed the original event. Dispatch a synthetic + // focusin on the owning document so the tooltip's normal show logic kicks in + // without re-triggering focusin listeners on the anchor or its ancestors. if (event?.type === 'focusin') { requestAnimationFrame(() => { - target.dispatchEvent(new FocusEvent('focusin', { bubbles: true, composed: true })); + (target.ownerDocument ?? document).dispatchEvent( + new FocusEvent('focusin', { bubbles: true, composed: true }) + ); }); } }; From 3bb66db7b082156aa5766ca430c849e54331261c Mon Sep 17 00:00:00 2001 From: anna-lach Date: Wed, 20 May 2026 08:32:13 +0200 Subject: [PATCH 11/11] Changes after Copilot review --- .changeset/empty-cougars-cross.md | 2 +- packages/components/checkbox/src/checkbox.spec.ts | 4 ++-- .../shared/src/mixins/forward-aria-mixin.ts | 15 ++++++++++----- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/.changeset/empty-cougars-cross.md b/.changeset/empty-cougars-cross.md index ae4891c710..c123591880 100644 --- a/.changeset/empty-cougars-cross.md +++ b/.changeset/empty-cougars-cross.md @@ -3,4 +3,4 @@ --- Fix checkbox group not working with tooltips (only `sl-checkbox` elements are tracked by `@queryAssignedElements`). -Use `ForwardAriaMixin` to forward `aria-describedby` as element references to the inner input element, enabling screen reader support across shadow DOM boundaries. +Use `ForwardAriaMixin` to forward `aria-describedby` as element references to the inner input element, enabling screen reader support. diff --git a/packages/components/checkbox/src/checkbox.spec.ts b/packages/components/checkbox/src/checkbox.spec.ts index 946ecf6d74..f9af794cff 100644 --- a/packages/components/checkbox/src/checkbox.spec.ts +++ b/packages/components/checkbox/src/checkbox.spec.ts @@ -126,7 +126,7 @@ describe('sl-checkbox', () => { expect(el.input).to.have.attribute('aria-label', 'Label'); }); - it('should proxy the aria-labelledby to ariaLabelledByElements on the input element', async () => { + it('should proxy the aria-labelledby attribute to the input element', async () => { const label = document.createElement('span'); label.id = 'my-label'; label.textContent = 'My label'; @@ -145,7 +145,7 @@ describe('sl-checkbox', () => { label.remove(); }); - it('should proxy the aria-describedby to ariaDescribedByElements on the input element', async () => { + it('should proxy the aria-describedby attribute to the input element', async () => { const tooltip = document.createElement('span'); tooltip.id = 'my-tooltip'; tooltip.textContent = 'My tooltip'; diff --git a/packages/components/shared/src/mixins/forward-aria-mixin.ts b/packages/components/shared/src/mixins/forward-aria-mixin.ts index 97e02f0e7e..1d3271cea3 100644 --- a/packages/components/shared/src/mixins/forward-aria-mixin.ts +++ b/packages/components/shared/src/mixins/forward-aria-mixin.ts @@ -290,13 +290,18 @@ export function ForwardAriaMixin< const refValue = elementsProp.endsWith('Elements') ? elements : (elements[0] ?? null); stored.set(elementsProp, refValue); - // If no elements could be resolved at all and the target is in the same - // scope, just remove the attribute from the host — there's nothing useful - // to set as a string attribute. For cross-scope targets, still forward the - // empty array so consumers reading the native property get a deterministic []. + // If no elements could be resolved at all, handle based on scope: + // - Same scope: forward the string IDREF attribute anyway — the browser's + // native resolution works dynamically and will pick up elements that + // appear in the DOM later. + // - Cross scope: forward an empty array so consumers reading the native + // property get a deterministic []. Element references can't resolve + // natively across shadow boundaries. if (elements.length === 0) { const targetRoot = targetElement.getRootNode(); - if (targetRoot !== root) { + if (targetRoot === root) { + targetElement.setAttribute(name, value); + } else { (targetElement as unknown as Record)[ elementsProp ] = refValue;