diff --git a/.changeset/chatty-badgers-dream.md b/.changeset/chatty-badgers-dream.md new file mode 100644 index 0000000000..ceacf093ea --- /dev/null +++ b/.changeset/chatty-badgers-dream.md @@ -0,0 +1,11 @@ +--- +'@sl-design-system/tag': patch +--- + +Accessibility improvements to `` and ``: + +- The remove button now has a proper accessible label ("Remove tag 'X'") instead of being `aria-hidden` +- The remove button uses `aria-disabled` instead of `disabled`, keeping it keyboard-reachable when the tag is disabled +- Focus is delegated to the remove button via `delegatesFocus`; `:state(focus-visible)` tracks focus for styling +- Overflow tooltip `aria-describedby` is now on the label part instead of the host +- `` correctly sets `role="listitem"` on each tag diff --git a/.changeset/stupid-cougars-serve.md b/.changeset/stupid-cougars-serve.md new file mode 100644 index 0000000000..f3943fc0ca --- /dev/null +++ b/.changeset/stupid-cougars-serve.md @@ -0,0 +1,5 @@ +--- +'@sl-design-system/locales': patch +--- + +Replace `sl.tag.removalInstructions` with `sl.tag.remove`, a parameterized string for the remove button label ("Remove tag 'X'"). diff --git a/packages/components/tag/src/tag-list.ts b/packages/components/tag/src/tag-list.ts index 4315c67951..f1ca34e0bb 100644 --- a/packages/components/tag/src/tag-list.ts +++ b/packages/components/tag/src/tag-list.ts @@ -27,6 +27,8 @@ declare global { * * ``` * + * @customElement sl-tag-list + * * @slot default - The place for tags. */ @localized() @@ -337,6 +339,7 @@ export class TagList extends ScopedElementsMixin(LitElement) { ); this.tags.forEach(tag => { + tag.role = 'listitem'; tag.size = this.size; tag.variant = this.variant; tag.setAttribute('role', 'listitem'); diff --git a/packages/components/tag/src/tag.scss b/packages/components/tag/src/tag.scss index 8222ecf524..197927c254 100644 --- a/packages/components/tag/src/tag.scss +++ b/packages/components/tag/src/tag.scss @@ -21,12 +21,12 @@ } } -:host([removable]) slot { +:host([removable]) [part='label'] { border-inline-end: var(--sl-size-borderWidth-default) solid var(--_br-color); } :host([size='lg']) { - slot { + [part='label'] { padding: calc(var(--sl-size-100) - var(--sl-size-borderWidth-default)) var(--sl-size-150); } @@ -47,21 +47,20 @@ --_br-color: var(--sl-color-border-disabled); color: var(--sl-color-foreground-disabled); - pointer-events: none; - slot, + [part='label'], button { background: var(--sl-color-background-disabled); } } -:host(:focus-visible) { +:host(:state(focus-visible)) { outline-color: var(--sl-color-border-focused); position: relative; z-index: 1; // Make sure the focus ring is above other elements } -slot { +[part='label'] { background: var(--_bg-color); display: block; flex: 1; @@ -84,6 +83,7 @@ button { flex-shrink: 0; inline-size: calc(var(--sl-size-300) - var(--sl-size-borderWidth-default) * 2); justify-content: center; + outline: 0; padding: 0; @media (prefers-reduced-motion: no-preference) { diff --git a/packages/components/tag/src/tag.spec.ts b/packages/components/tag/src/tag.spec.ts index 3bbd9de867..e93216dd2c 100644 --- a/packages/components/tag/src/tag.spec.ts +++ b/packages/components/tag/src/tag.spec.ts @@ -14,7 +14,7 @@ describe('sl-tag', () => { el = await fixture(html`My label`); }); - it('should not have an explicit', () => { + it('should not have an explicit size', () => { expect(el).not.to.have.attribute('size'); expect(el.size).to.be.undefined; }); @@ -47,63 +47,84 @@ describe('sl-tag', () => { el.removable = true; await el.updateComplete; + expect(el).to.have.attribute('removable'); expect(el.renderRoot.querySelector('button')).to.exist; }); - it('should not have a tabindex', () => { - expect(el).not.to.have.attribute('tabindex'); + it('should not have a tooltip', async () => { + el.focus(); + await el.updateComplete; + + expect(el.renderRoot.querySelector('[part="label"]')).not.to.have.attribute('aria-describedby'); + expect(el.renderRoot.querySelector('sl-tooltip')).not.to.exist; }); - it('should not have a tooltip', async () => { + it('should not be focusable', async () => { el.focus(); await el.updateComplete; - expect(el).not.to.have.attribute('aria-describedby'); + expect(el).not.to.match(':focus'); + expect(el).not.to.match(':state(focus-visible)'); }); }); describe('removable', () => { + let button: HTMLButtonElement; + beforeEach(async () => { el = await fixture(html`My label`); + button = el.renderRoot.querySelector('button')!; }); - it('should have an ARIA description indicating how to remove the tag', () => { - expect(el).to.have.attribute('aria-description', 'Press the delete or backspace key to remove this item'); + it('should not have the focus-visible state', () => { + expect(el).not.to.match(':state(focus-visible)'); }); - it('should have a tabindex of 0', () => { - expect(el).to.have.attribute('tabindex', '0'); + it('should have the focus-visible state when focused', async () => { + el.focus(); + await el.updateComplete; + + expect(el).to.match(':state(focus-visible)'); }); - it('should have a tabindex of -1 when disabled', async () => { - el.disabled = true; + it('should have a button', () => { + expect(button).to.exist; + }); + + it('should focus the button when the tag is focused', async () => { + el.focus(); await el.updateComplete; - expect(el).to.have.attribute('tabindex', '-1'); + expect(button).to.match(':focus'); }); - it('should have a button', () => { - expect(el.renderRoot.querySelector('button')).to.exist; + it('should have an accessible label on the remove button', () => { + expect(button).to.have.attribute('aria-label', "Remove tag 'My label'"); }); - it('should hide the button for ARIA', () => { - expect(el.renderRoot.querySelector('button')).to.have.attribute('aria-hidden', 'true'); + it('should mark the button as aria-disabled when the tag is disabled', async () => { + el.disabled = true; + await el.updateComplete; + + expect(button).to.have.attribute('aria-disabled', 'true'); }); - it('should not be be removed when it is disabled and remove button is clicked', async () => { - el.setAttribute('disabled', ''); + it('should not be removed when it is disabled and remove button is clicked', async () => { + const onRemove = spy(el, 'remove'); + + el.disabled = true; await el.updateComplete; - el.renderRoot.querySelector('button')?.click(); + button.click(); await el.updateComplete; - expect(el).to.exist; + expect(onRemove).not.to.have.been.called; }); it('should be removed when the button is clicked using the keyboard', async () => { const onRemove = spy(el, 'remove'); - el.renderRoot.querySelector('button')?.focus(); + el.focus(); await userEvent.keyboard('{Enter}'); expect(onRemove).to.have.been.calledOnce; @@ -131,7 +152,7 @@ describe('sl-tag', () => { const onRemove = spy(); el.addEventListener('sl-remove', onRemove); - el.renderRoot.querySelector('button')?.click(); + button.click(); await el.updateComplete; expect(onRemove).to.have.been.calledOnce; @@ -150,9 +171,10 @@ describe('sl-tag', () => { el.focus(); await el.updateComplete; - expect(el).to.have.attribute('aria-describedby'); + const label = el.renderRoot.querySelector('[part="label"]')!; + expect(label).to.have.attribute('aria-describedby'); - const tooltip = document.getElementById(el.getAttribute('aria-describedby')!); + const tooltip = el.renderRoot.querySelector('sl-tooltip'); expect(tooltip).to.exist; expect(tooltip).to.have.trimmed.text('My label is very long'); }); diff --git a/packages/components/tag/src/tag.stories.ts b/packages/components/tag/src/tag.stories.ts index eade687608..2e6187acfc 100644 --- a/packages/components/tag/src/tag.stories.ts +++ b/packages/components/tag/src/tag.stories.ts @@ -1,7 +1,6 @@ import { type Meta, type StoryObj } from '@storybook/web-components-vite'; import { html } from 'lit'; import { ifDefined } from 'lit/directives/if-defined.js'; -import { styleMap } from 'lit/directives/style-map.js'; import '../register.js'; import { type Tag } from './tag.js'; @@ -44,7 +43,7 @@ export default { ?disabled=${disabled} ?removable=${removable} size=${ifDefined(size)} - style=${styleMap({ maxWidth })} + style=${ifDefined(maxWidth ? `max-inline-size: ${maxWidth}` : undefined)} variant=${ifDefined(variant)} > ${label} @@ -73,8 +72,22 @@ export const Overflow: Story = { } }; +export const OverflowRemovable: Story = { + args: { + ...Overflow.args, + removable: true + } +}; + export const Removable: Story = { args: { removable: true } }; + +export const RemovableDisabled: Story = { + args: { + disabled: true, + removable: true + } +}; diff --git a/packages/components/tag/src/tag.ts b/packages/components/tag/src/tag.ts index b8190b2f01..d098c494e2 100644 --- a/packages/components/tag/src/tag.ts +++ b/packages/components/tag/src/tag.ts @@ -1,10 +1,11 @@ -import { localized, msg } from '@lit/localize'; +import { localized, msg, str } from '@lit/localize'; import { type ScopedElementsMap, ScopedElementsMixin } from '@open-wc/scoped-elements/lit-element.js'; import { Icon } from '@sl-design-system/icon'; -import { EventEmitter, EventsController, event } from '@sl-design-system/shared'; +import { EventEmitter, event } from '@sl-design-system/shared'; import { Tooltip } from '@sl-design-system/tooltip'; -import { type CSSResultGroup, LitElement, type PropertyValues, type TemplateResult, html, nothing } from 'lit'; +import { type CSSResultGroup, LitElement, type TemplateResult, html, nothing } from 'lit'; import { property, state } from 'lit/decorators.js'; +import { ifDefined } from 'lit/directives/if-defined.js'; import styles from './tag.scss.js'; declare global { @@ -29,7 +30,12 @@ export type TagVariant = 'neutral' | 'info'; * Tag label * ``` * + * @customElement sl-tag + * * @slot default - The tag label. + * + * @csspart label - The wrapper around the tag label. + * @csspart button - The remove button. */ @localized() export class Tag extends ScopedElementsMixin(LitElement) { @@ -41,11 +47,17 @@ export class Tag extends ScopedElementsMixin(LitElement) { }; } + /** @internal */ + static override shadowRootOptions: ShadowRootInit = { + ...LitElement.shadowRootOptions, + delegatesFocus: true + }; + /** @internal */ static override styles: CSSResultGroup = styles; - // eslint-disable-next-line no-unused-private-class-members - #events = new EventsController(this, { keydown: this.#onKeydown }); + /** @internal */ + #internals = this.attachInternals(); /** * Observe changes in size, so we can check whether we need to show tooltips @@ -53,15 +65,15 @@ export class Tag extends ScopedElementsMixin(LitElement) { */ #observer = new ResizeObserver(() => this.#onResize()); - /** Either an instanceof of Tooltip, or a cleanup function. */ - #tooltip?: Tooltip | (() => void); - /** * Whether the tag component is disabled, when set no interaction is possible. * @default false */ @property({ type: Boolean, reflect: true }) disabled?: boolean; + /** @internal Whether a tooltip should be shown. */ + @state() tooltip?: boolean; + /** @internal The label of the tag component. */ @state() label = ''; @@ -69,7 +81,7 @@ export class Tag extends ScopedElementsMixin(LitElement) { * Whether the tag component is removable. * @default false */ - @property({ type: Boolean }) removable?: boolean; + @property({ type: Boolean, reflect: true }) removable?: boolean; /** @internal Emits when the tag is removed. */ @event({ name: 'sl-remove' }) removeEvent!: EventEmitter; @@ -95,46 +107,34 @@ export class Tag extends ScopedElementsMixin(LitElement) { override disconnectedCallback(): void { this.#observer.disconnect(); - if (this.#tooltip instanceof Tooltip) { - this.#tooltip?.remove(); - } else if (this.#tooltip) { - this.#tooltip(); - } - - this.#tooltip = undefined; - super.disconnectedCallback(); } - override updated(changes: PropertyValues): void { - super.updated(changes); - - if (changes.has('disabled') || changes.has('removable')) { - if (this.removable) { - this.setAttribute('tabindex', this.disabled ? '-1' : '0'); - } else if (this.disabled || changes.get('removable')) { - this.removeAttribute('tabindex'); - } - } - - if (changes.has('removable')) { - if (this.removable) { - this.setAttribute( - 'aria-description', - msg('Press the delete or backspace key to remove this item', { id: 'sl.tag.removalInstructions' }) - ); - } else { - this.removeAttribute('aria-description'); - } - } - } - override render(): TemplateResult { + const hasTabindex = !this.disabled && !this.removable && this.tooltip; + return html` - - ${this.removable && !this.disabled + ${this.tooltip ? html`${this.label}` : nothing} +
+ +
+ ${this.removable ? html` - ` @@ -142,8 +142,16 @@ export class Tag extends ScopedElementsMixin(LitElement) { `; } + #onBlur(): void { + this.#internals.states.delete('focus-visible'); + } + + #onFocus(): void { + this.#internals.states.add('focus-visible'); + } + #onKeydown(event: KeyboardEvent): void { - if (this.removable && (event.key === 'Backspace' || event.key === 'Delete')) { + if (event.key === 'Backspace' || event.key === 'Delete') { this.#onRemove(event); } } @@ -162,32 +170,9 @@ export class Tag extends ScopedElementsMixin(LitElement) { } #onResize(): void { - const slot = this.renderRoot.querySelector('slot'); - - if (slot && slot.clientWidth < slot.scrollWidth) { - this.#tooltip ||= Tooltip.lazy( - this, - tooltip => { - this.#tooltip = tooltip; - tooltip.textContent = this.label; - }, - { context: this.shadowRoot! } - ); - } else if (this.#tooltip instanceof Tooltip) { - this.#tooltip.remove(); - this.#tooltip = undefined; - } else if (this.#tooltip) { - this.#tooltip(); - this.#tooltip = undefined; - } + const label = this.renderRoot.querySelector('[part="label"]'); - // If the contents of the tag overflows, make sure it is keyboard focusable, - // so the user can tab to it. - if (!this.disabled && (this.removable || this.#tooltip)) { - this.setAttribute('tabindex', '0'); - } else if (!this.hasAttribute('aria-labelledby')) { - this.removeAttribute('tabindex'); - } + this.tooltip = !!(label && label.clientWidth < label.scrollWidth); } #onSlotChange(event: Event & { target: HTMLSlotElement }): void { diff --git a/packages/locales/src/nl.ts b/packages/locales/src/nl.ts index b9f665ed89..6f6d23ec0d 100644 --- a/packages/locales/src/nl.ts +++ b/packages/locales/src/nl.ts @@ -100,7 +100,7 @@ export const templates = { 'sl.select.validation.valueMissing': 'Kies een optie uit de lijst.', 'sl.tabs.showAll': 'Toon alles', 'sl.tag.listOfHiddenElements': 'Lijst met verborgen elementen', - 'sl.tag.removalInstructions': 'Druk op de delete- of backspacetoets om dit item te verwijderen', + 'sl.tag.remove': str`Verwijder tag '${0}'`, 'sl.timeField.empty': 'Leeg', 'sl.timeField.rangeOverflow': str`Voer een tijd in die niet later is dan ${0}.`, 'sl.timeField.rangeUnderflow': str`Voer een tijd in die niet eerder is dan ${0}.`, diff --git a/packages/locales/src/nl.xlf b/packages/locales/src/nl.xlf index ce750db890..8a36d3d62d 100644 --- a/packages/locales/src/nl.xlf +++ b/packages/locales/src/nl.xlf @@ -10,10 +10,6 @@ Loading Laden - - Press the delete or backspace key to remove this item - Druk op de delete- of backspacetoets om dit item te verwijderen - Selected Geselecteerd @@ -426,6 +422,10 @@ No later than Uiterlijk + + Remove tag '' + Verwijder tag '' + More information Meer informatie