Skip to content
Draft
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/empty-cougars-cross.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sl-design-system/checkbox': patch
---

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.
Comment thread
anna-lach marked this conversation as resolved.
Outdated
26 changes: 26 additions & 0 deletions packages/components/checkbox/src/checkbox-group.spec.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -437,4 +438,29 @@ describe('sl-checkbox-group', () => {
expect(fitc.shadowRoot!.activeElement).to.equal(input);
});
});

describe('with tooltips', () => {
beforeEach(async () => {
el = await fixture(html`
<sl-checkbox-group required>
<sl-checkbox value="a">A</sl-checkbox>
<sl-checkbox value="b" aria-describedby="tip1">B</sl-checkbox>
<sl-tooltip id="tip1">Tooltip</sl-tooltip>
<sl-checkbox value="c">C</sl-checkbox>
</sl-checkbox-group>
`);
});

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]);
});
});
});
36 changes: 36 additions & 0 deletions packages/components/checkbox/src/checkbox-group.stories.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -191,3 +193,37 @@ export const CustomAsyncValidity: Story = {
}
}
};

export const WithTooltips: Story = {
render: () => {
const onClick = (event: Event & { target: HTMLElement }): void => {
event.target.closest('sl-form')?.reportValidity();
};

return html`
<p>
This story demonstrates how to use tooltips with checkboxes inside a checkbox group using
both the <code>tooltip()</code> directive and the manual
<code>aria-describedby</code> approach with a separate <code>sl-tooltip</code> element.
</p>

<sl-form>
<sl-form-field label="Subscriptions">
<sl-checkbox-group name="subscriptions" required>
<sl-checkbox ${tooltip('Newsletter tooltip')} value="newsletter"
>Newsletter</sl-checkbox
>
<sl-checkbox value="promotions" aria-describedby="tooltip1">Promotions</sl-checkbox>
<sl-tooltip id="tooltip1">Promotions tooltip</sl-tooltip>
<sl-checkbox ${tooltip('Product updates tooltip')} value="updates"
>Product updates</sl-checkbox
>
Comment on lines +213 to +220
</sl-checkbox-group>
</sl-form-field>
<sl-button-bar>
<sl-button @click=${onClick}>Report validity</sl-button>
</sl-button-bar>
</sl-form>
`;
}
};
2 changes: 1 addition & 1 deletion packages/components/checkbox/src/checkbox-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export class CheckboxGroup<T = any> extends FormControlMixin(LitElement) {
readonly internals = this.attachInternals();

/** @internal The slotted checkboxes. */
@queryAssignedElements() boxes?: Array<Checkbox<T>>;
@queryAssignedElements({ selector: 'sl-checkbox' }) boxes?: Array<Checkbox<T>>;

/** @internal Emits when the component loses focus. */
@event({ name: 'sl-blur' }) blurEvent!: EventEmitter<SlBlurEvent>;
Expand Down
36 changes: 33 additions & 3 deletions packages/components/checkbox/src/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -120,12 +124,38 @@ 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 () => {
Comment thread
anna-lach marked this conversation as resolved.
Outdated
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 to ariaDescribedByElements on the input element', async () => {
Comment thread
anna-lach marked this conversation as resolved.
Outdated
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.ariaDescribedByElements).to.deep.equal([tooltip]);

tooltip.remove();
});

it('should return the input element from getProxyTarget', () => {
expect(el.getProxyTarget()).to.equal(el.input);
});

it('should be pristine', () => {
Expand Down
116 changes: 92 additions & 24 deletions packages/components/checkbox/src/checkbox.stories.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -85,8 +87,9 @@ export default {
.showValid=${showValid}
.value=${value}
size=${ifDefined(size)}
>${text}</sl-checkbox
>
${text}
</sl-checkbox>
`}
</sl-form-field>
${reportValidity
Expand Down Expand Up @@ -193,21 +196,21 @@ export const Indeterminate: StoryObj = {

<ul>
<li>
<sl-checkbox @sl-change=${onChange} name="tall-2-1" id="tall-2-1"
>Andre</sl-checkbox
>
<sl-checkbox @sl-change=${onChange} name="tall-2-1" id="tall-2-1">
Andre
</sl-checkbox>
</li>
<li>
<sl-checkbox @sl-change=${onChange} name="tall-2-2" id="tall-2-2"
>Paul Bunyan</sl-checkbox
>
<sl-checkbox @sl-change=${onChange} name="tall-2-2" id="tall-2-2">
Paul Bunyan
</sl-checkbox>
</li>
</ul>
</li>
<li>
<sl-checkbox @sl-change=${onChange} name="tall-3" id="tall-3"
>Two sandwiches</sl-checkbox
>
<sl-checkbox @sl-change=${onChange} name="tall-3" id="tall-3">
Two sandwiches
</sl-checkbox>
</li>
</ul>
</li>
Expand All @@ -218,14 +221,14 @@ export const Indeterminate: StoryObj = {
<sl-checkbox @sl-change=${onChange} name="short-1" id="short-1">Smurfs</sl-checkbox>
</li>
<li>
<sl-checkbox @sl-change=${onChange} name="short-2" id="short-2"
>Mushrooms</sl-checkbox
>
<sl-checkbox @sl-change=${onChange} name="short-2" id="short-2">
Mushrooms
</sl-checkbox>
</li>
<li>
<sl-checkbox @sl-change=${onChange} name="short-3" id="short-3"
>One Sandwich</sl-checkbox
>
<sl-checkbox @sl-change=${onChange} name="short-3" id="short-3">
One Sandwich
</sl-checkbox>
</li>
</ul>
</li>
Expand All @@ -239,8 +242,10 @@ export const NoVisibleLabel: StoryObj = {
return html`
<p style="margin: 0 0 1rem 0">
This checkbox has no internal or external label. It only has an
<code>aria-label</code> attribute. That attribute is automatically applied to the
<code>input</code> element.
<code>aria-label</code>
attribute. That attribute is automatically applied to the
<code>input</code>
element.
</p>
<sl-checkbox aria-label="Check me"></sl-checkbox>
`;
Expand Down Expand Up @@ -281,9 +286,9 @@ export const CustomValidity: Story = {
};

return html`
<sl-checkbox @sl-validate=${onValidate} required value="1"
>I agree to all terms &amp; conditions</sl-checkbox
>
<sl-checkbox @sl-validate=${onValidate} required value="1">
I agree to all terms &amp; conditions
</sl-checkbox>
`;
}
}
Expand All @@ -307,10 +312,73 @@ export const CustomAsyncValidity: Story = {
};

return html`
<sl-checkbox @sl-validate=${onValidate} required value="1"
>I agree to all terms &amp; conditions</sl-checkbox
>
<sl-checkbox @sl-validate=${onValidate} required value="1">
I agree to all terms &amp; conditions
</sl-checkbox>
`;
}
}
};

export const WithTooltip: Story = {
render: () => {
const onClick = (event: Event & { target: HTMLElement }): void => {
event.target.closest('sl-form')?.reportValidity();
};

return html`
<p>
This story demonstrates how to use tooltips with checkboxes using the
<code>tooltip()</code>
directive,
<code>aria-describedby</code>
, and
<code>aria-labelledby</code>
with a separate
<code>sl-tooltip</code>
element. The
<code>ForwardAriaMixin</code>
resolves IDs to element references and forwards them to the focusable input element, which
properly crosses the shadow DOM boundary.
Comment on lines +341 to +342
</p>

<h3>Using the tooltip directive</h3>
<sl-form>
<sl-form-field label="Subscriptions">
<sl-checkbox ${tooltip('Newsletter tooltip')} value="newsletter" required>
Newsletter
</sl-checkbox>
</sl-form-field>
<sl-button-bar>
<sl-button @click=${onClick}>Report validity</sl-button>
</sl-button-bar>
</sl-form>

<h3>Using aria-describedby</h3>
<sl-form>
<sl-form-field label="Subscriptions">
<sl-checkbox aria-describedby="tooltip-promotions" value="promotions" required>
Promotions
</sl-checkbox>
<sl-tooltip id="tooltip-promotions">Promotions tooltip</sl-tooltip>
</sl-form-field>
<sl-button-bar>
<sl-button @click=${onClick}>Report validity</sl-button>
</sl-button-bar>
</sl-form>

<h3>Using aria-labelledby</h3>
<sl-form>
<sl-form-field label="Subscriptions">
<sl-checkbox aria-labelledby="tooltip-updates" value="updates" required>
Updates
</sl-checkbox>
<sl-tooltip id="tooltip-updates">Updates tooltip</sl-tooltip>
</sl-form-field>
<sl-button-bar>
<sl-button @click=${onClick}>Report validity</sl-button>
</sl-button-bar>
</sl-form>
`;
}
};
17 changes: 12 additions & 5 deletions packages/components/checkbox/src/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { FormControlMixin } from '@sl-design-system/form';
import {
type EventEmitter,
EventsController,
ObserveAttributesMixin,
ForwardAriaMixin,
event
} from '@sl-design-system/shared';
import {
Expand Down Expand Up @@ -45,7 +45,8 @@ let nextUniqueId = 0;
*/
@localized()
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export class Checkbox<T = any> extends ObserveAttributesMixin(FormControlMixin(LitElement), [
export class Checkbox<T = any> extends ForwardAriaMixin(FormControlMixin(LitElement), [
'aria-describedby',
'aria-disabled',
'aria-label',
'aria-labelledby'
Expand Down Expand Up @@ -166,6 +167,7 @@ export class Checkbox<T = any> extends ObserveAttributesMixin(FormControlMixin(L
}

this.setFormControlElement(this.input);
this.setProxyTarget(this.input);

this.#onLabelSlotChange();
}
Expand Down Expand Up @@ -283,6 +285,7 @@ export class Checkbox<T = any> extends ObserveAttributesMixin(FormControlMixin(L
this.#syncInput(this.input);

this.setFormControlElement(this.input);
this.setProxyTarget(this.input);
}
}

Expand Down Expand Up @@ -315,7 +318,13 @@ export class Checkbox<T = any> extends ObserveAttributesMixin(FormControlMixin(L
}

requestAnimationFrame(() => {
if (!this.input.hasAttribute('aria-labelledby') && this.input.labels?.length) {
if (
!this.hasAttribute('aria-labelledby') &&
!this.input.hasAttribute('aria-labelledby') &&
!this.input.ariaLabelledByElements?.length &&
!this.ariaLabelledByElements?.length &&
this.input.labels?.length
) {
this.input.setAttribute(
'aria-labelledby',
Array.from(this.input.labels)
Expand All @@ -336,7 +345,5 @@ export class Checkbox<T = any> extends ObserveAttributesMixin(FormControlMixin(L

input.checked = !!this.checked;
input.indeterminate = !!this.indeterminate;

this.setAttributesTarget(input);
}
}
Loading
Loading