Skip to content

Commit 909bef6

Browse files
fix: disabled removabled when selected or toggleable (#2759)
## Description: Closes: #2398. Previous PR: #2682 Added a variable _isRemovable_ to the sd-tag component. This variable represents the _removable_ attribute being enabled, while checking if the _toggleable_ or _selected_ attributes are disabled. Then, replaced every place that checked if the removable attribute was enabled for this new variable. Also added a note to the docs about this. Let me know if it is enough or if we should create more emphasis. ## Definition of Reviewable: - [x] Documentation is created/updated - [x] relevant tickets are linked --------- Co-authored-by: Sérgio Fonseca <42741644+smfonseca@users.noreply.github.com>
1 parent 3501268 commit 909bef6

5 files changed

Lines changed: 63 additions & 6 deletions

File tree

.changeset/fine-ways-take.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@solid-design-system/components': minor
3+
'@solid-design-system/docs': minor
4+
---
5+
6+
Changed `sd-tag`'s attribute `removable` to be disabled if the attributes `selected` or `toggleable` are enabled

packages/components/src/components/tag/tag.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,32 @@ describe('<sd-tag>', () => {
4040
});
4141
});
4242

43+
describe('removable attribute behaviour with selected or toggleable', () => {
44+
it('should ignore removable when selected', async () => {
45+
el = await fixture<SdTag>(html`<sd-tag removable selected>Tag</sd-tag>`);
46+
const removable = el.shadowRoot!.querySelector('[part="removable-indicator"]');
47+
expect(removable).to.not.exist;
48+
});
49+
50+
it('should ignore removable when toggleable', async () => {
51+
el = await fixture<SdTag>(html`<sd-tag removable toggleable>Tag</sd-tag>`);
52+
const removable = el.shadowRoot!.querySelector('[part="removable-indicator"]');
53+
expect(removable).to.not.exist;
54+
});
55+
56+
it('should ignore removable when both selected and toggleable', async () => {
57+
el = await fixture<SdTag>(html`<sd-tag removable selected toggleable>Tag</sd-tag>`);
58+
const removable = el.shadowRoot!.querySelector('[part="removable-indicator"]');
59+
expect(removable).to.not.exist;
60+
});
61+
62+
it('should show removable when neither selected nor toggleable', async () => {
63+
el = await fixture<SdTag>(html`<sd-tag removable>Tag</sd-tag>`);
64+
const removable = el.shadowRoot!.querySelector('[part="removable-indicator"]');
65+
expect(removable).to.exist;
66+
});
67+
});
68+
4369
sizes.forEach(size => {
4470
describe(`when passed a size attribute ${size}`, () => {
4571
it(`should be accessible when size is "${size}"`, async () => {

packages/components/src/components/tag/tag.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@ export default class SdTag extends SolidElement {
122122

123123
render() {
124124
const isLink = this.isLink();
125-
const tag = isLink ? literal`a` : this.removable ? literal`div` : literal`button`;
125+
const isRemovable = this.removable && !this.selected && !this.toggleable;
126+
const tag = isLink ? literal`a` : isRemovable ? literal`div` : literal`button`;
126127

127128
/* eslint-disable lit/no-invalid-html */
128129
/* eslint-disable lit/binding-positions */
@@ -138,7 +139,7 @@ export default class SdTag extends SolidElement {
138139
aria-labelledby="content"
139140
aria-disabled=${this.disabled ? 'true' : 'false'}
140141
aria-pressed=${ifDefined(this.toggleable ? this.selected : undefined)}
141-
tabindex=${this.disabled || this.removable ? '-1' : '0'}
142+
tabindex=${this.disabled || isRemovable ? '-1' : '0'}
142143
@blur=${this.handleBlur}
143144
@focus=${this.handleFocus}
144145
class=${cx(
@@ -151,14 +152,14 @@ export default class SdTag extends SolidElement {
151152
}[this.size],
152153
{
153154
/* padding */
154-
lg: !this.removable ? 'px-4 py-2' : 'pl-4 pr-3 py-2',
155-
sm: !this.removable ? 'px-3 py-[5px]' : 'pl-3 pr-2 py-2'
155+
lg: !isRemovable ? 'px-4 py-2' : 'pl-4 pr-3 py-2',
156+
sm: !isRemovable ? 'px-3 py-[5px]' : 'pl-3 pr-2 py-2'
156157
}[this.size],
157158
/* colors */
158159
!this.selected
159160
? cx(
160161
'sd-tag--default-color-border sd-tag--default-color-text disabled:sd-tag--disabled-color-border disabled:text-neutral-500',
161-
!this.removable
162+
!isRemovable
162163
? 'sd-tag--default-color-background hover:border-primary-500 hover:text-primary-500 hover:sd-tag--default--hover-color-background'
163164
: 'sd-tag--default-color-background has-[button:hover]:border-primary-500 has-[button:hover]:text-primary-500'
164165
)
@@ -169,7 +170,7 @@ export default class SdTag extends SolidElement {
169170
<slot name="icon-left" class=${cx('flex-shrink-0', this.size === 'lg' ? 'text-base' : 'text-[0.75rem]')}></slot>
170171
<slot id="content" part='content'></slot>
171172
${
172-
this.removable && !isLink
173+
isRemovable && !isLink
173174
? html` <button class="sd-interactive flex items-center" type="button" @click=${this.handleRemove}>
174175
<slot part="removable-indicator" name="removable-indicator">
175176
<sd-icon library="_internal" name="close" label=${this.localize.term('remove')}></sd-icon>

packages/docs/src/stories/components/tag.stories.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ export const Selected = {
7070

7171
/**
7272
* Use the `removable` attribute to include the removability indicator.
73+
* This attribute is disabled when using the attributes `selected` or `toggleable`.
7374
*
7475
* __Hint:__ Combine the `sd-remove` event and `hide` method to visually hide the tag
7576
* when the removable button is triggered.

packages/docs/src/stories/components/tag.test.stories.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,28 @@ export const Disabled = {
115115
}
116116
};
117117

118+
export const RemovableAndSelectedAndToggleable = {
119+
name: 'Removable x Selected x Toggleable',
120+
render: (args: any) => {
121+
return html`
122+
${generateTemplate({
123+
axis: {
124+
x: { type: 'attribute', name: 'selected' },
125+
y: { type: 'attribute', name: 'removable' }
126+
},
127+
args
128+
})}
129+
${generateTemplate({
130+
axis: {
131+
x: { type: 'attribute', name: 'toggleable' },
132+
y: { type: 'attribute', name: 'removable' }
133+
},
134+
args
135+
})}
136+
`;
137+
}
138+
};
139+
118140
/**
119141
* Use the `default` slot to add content to the tag.
120142
* Use the `removable-indicator` slot to change the removability indicator.
@@ -225,6 +247,7 @@ export const Combination = generateScreenshotStory([
225247
removableAndSize,
226248
iconLeft,
227249
Disabled,
250+
RemovableAndSelectedAndToggleable,
228251
Slots,
229252
Parts,
230253
Mouseless

0 commit comments

Comments
 (0)