Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions packages/components/tag/src/tag.scss
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,14 @@
--_br-color: var(--sl-color-border-disabled);

color: var(--sl-color-foreground-disabled);
pointer-events: none;

slot,
button {
background: var(--sl-color-background-disabled);
}
}
Comment thread
jpzwarte marked this conversation as resolved.

: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
Expand Down Expand Up @@ -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) {
Expand Down
62 changes: 41 additions & 21 deletions packages/components/tag/src/tag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('sl-tag', () => {
el = await fixture(html`<sl-tag>My label</sl-tag>`);
});

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;
});
Expand Down Expand Up @@ -47,63 +47,83 @@ 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).not.to.have.attribute('aria-describedby');
});

it('should not be focusable', async () => {
el.focus();
await el.updateComplete;

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`<sl-tag removable>My label</sl-tag>`);
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 () => {
Comment thread
jpzwarte marked this conversation as resolved.
Outdated
el.setAttribute('disabled', '');
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;
Expand Down Expand Up @@ -131,7 +151,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;
Expand Down
10 changes: 8 additions & 2 deletions packages/components/tag/src/tag.stories.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -78,3 +77,10 @@ export const Removable: Story = {
removable: true
}
};

export const RemovableDisabled: Story = {
args: {
disabled: true,
removable: true
}
};
77 changes: 37 additions & 40 deletions packages/components/tag/src/tag.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -29,7 +30,12 @@ export type TagVariant = 'neutral' | 'info';
* <sl-tag>Tag label</sl-tag>
* ```
*
* @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) {
Expand All @@ -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
Expand All @@ -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<SlRemoveEvent>;
Expand Down Expand Up @@ -106,44 +118,37 @@ export class Tag extends ScopedElementsMixin(LitElement) {
super.disconnectedCallback();
}

override updated(changes: PropertyValues<this>): 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 {
return html`
<slot @slotchange=${this.#onSlotChange} part="label"></slot>
${this.removable && !this.disabled
${this.removable
? html`
<button @click=${this.#onRemove} ?disabled=${this.disabled} aria-hidden="true" part="button" tabindex="-1">
<button
@blur=${this.#onBlur}
@click=${this.#onRemove}
@focus=${this.#onFocus}
@keydown=${this.#onKeydown}
aria-disabled=${ifDefined(this.disabled ? 'true' : undefined)}
aria-label=${msg(str`Remove tag '${this.label}'`, { id: 'sl.tag.remove' })}
part="button"
Comment thread
jpzwarte marked this conversation as resolved.
Comment thread
jpzwarte marked this conversation as resolved.
>
<sl-icon name="xmark"></sl-icon>
</button>
`
: nothing}
`;
}

#onBlur(): void {
this.#internals.states.delete('focus-visible');
}

#onFocus(): void {
this.#internals.states.add('focus-visible');
Comment thread
jpzwarte marked this conversation as resolved.
Comment thread
jpzwarte marked this conversation as resolved.
}

#onKeydown(event: KeyboardEvent): void {
if (this.removable && (event.key === 'Backspace' || event.key === 'Delete')) {
if (event.key === 'Backspace' || event.key === 'Delete') {
Comment thread
jpzwarte marked this conversation as resolved.
this.#onRemove(event);
}
}
Expand Down Expand Up @@ -180,14 +185,6 @@ export class Tag extends ScopedElementsMixin(LitElement) {
this.#tooltip();
this.#tooltip = undefined;
}

// 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');
}
}
Comment thread
jpzwarte marked this conversation as resolved.
Outdated

#onSlotChange(event: Event & { target: HTMLSlotElement }): void {
Expand Down
4 changes: 2 additions & 2 deletions packages/locales/src/nl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ 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.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}.`,
Expand All @@ -109,5 +108,6 @@ export const templates = {
'sl.timeField.typeMismatch': 'Voer een geldige tijd in.',
'sl.timeField.valueMissing': 'Voer een tijd in.',
'sl.toolBar.showMore': 'Meer tonen',
'sl.tree.loadingMessage': 'Laden'
'sl.tree.loadingMessage': 'Laden',
'sl.tag.removeLabel': str`Remove tag '${0}'`
Comment thread
jpzwarte marked this conversation as resolved.
Outdated
};
8 changes: 4 additions & 4 deletions packages/locales/src/nl.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@
<source>Loading</source>
<target>Laden</target>
</trans-unit>
<trans-unit id="sl.tag.removalInstructions">
<source>Press the delete or backspace key to remove this item</source>
<target>Druk op de delete- of backspacetoets om dit item te verwijderen</target>
</trans-unit>
Copy link
Copy Markdown
Collaborator

@Diaan Diaan May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we keep this in as deprecated? If people update the locales but don't update the tag list component it will break the translation.

<trans-unit id="sl.common.selected">
<source>Selected</source>
<target>Geselecteerd</target>
Expand Down Expand Up @@ -426,6 +422,10 @@
<source>No later than <x id="0" equiv-text="${format(this.max, this.locale, this.#helperTextFormatOptions)}"/></source>
<target>Uiterlijk <x id="0" equiv-text="${format(this.max, this.locale, this.#helperTextFormatOptions)}"/></target>
</trans-unit>
<trans-unit id="sl.tag.remove">
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we not add it to spanish and polish as well?

<source>Remove tag '<x id="0" equiv-text="${this.label}"/>'</source>
<target>Verwijder tag '<x id="0" equiv-text="${this.label}"/>'</target>
</trans-unit>
</body>
</file>
</xliff>
Loading