feat(tag): migrate to S2 #3682

rise-erpelding · 2025-04-22T21:33:53Z

Description

There was a previous version of this work in #2649 that had been shelved because icons had not been updated yet. That PR will be closed once this one merges.

This work migrates the tag component to Spectrum 2 tokens and specifications:

The invalid variant has been removed.
Borders remain in high contrast but are otherwise transparent.
Thumbnail has been added as a visual option.
Tag now has a max-inline-size.
Read-only tags are now supported.

Additionally, Tag has an expanded testing grid to test more states and state combinations, an updated Docs page in Storybook, and updated Storybook controls to demonstrate Tag's supported content and states.

There are a few outstanding questions that I have, but I don't consider these to be blocking and will make follow-up tickets as needed to address these items:

Down state seems to behave erratically, but this appears to happen with other components too, navigating to and from the testing grid appears to affect this (we now have a ticket for this).
We don't have WHCM specs for S2, so the styles here are best-guess. I did end up leaving the clear button colors as they were in S1 for now.
The neutral-selected-background-color-default and its accompanying hover, key-focus, down tokens don't appear to exist, I used --spectrum-neutral-background-color-default colors, which seem to mostly match the spec colors.
I saw that the S1 guidelines mentioned a read-only Tag, but this doesn't appear to have been implemented. This is pretty minor to support, it won't affect implementations if they don't use it, and could be removed easily. I'll follow up on whether or not we want this and if there are any additional guidelines on what it should look like. For now, read-only tags do not change if selected, hovered, focused, or active, but disabled does win out over read-only. The text is selectable now in all states (including disabled).
Emphasized tags - there's no clarification on what emphasized & selected vs. emphasized & unselected should look like, my guess is that the tag only appears emphasized when it's selected. This is not how it's implemented in S1, but it was easy to change and would be easy to change back if that's not correct.
Clear button - there aren't clear guidelines on how this should behave, should it change color on hover or have a focus outline? The only design guideline is about which icon to use.
We changed the avatar size for the medium tag to size-75, this is what React appears to be doing and it fits the tag better, is this acceptable or do we want to have the avatar size-100 as spec'd?
The default t-shirt size has been changed to small per design note, but might be good to clarify.

Questions I have now:

I made the text selectable, but should I make it unselectable for the disabled state?

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

Here are some things that I generally look at when reviewing other people's migrations that could be used to review this migration, plus a few specific callouts for this work that I'd love feedback on:

Check against token spec:
- Validate that all tokens used appropriately/as spec'd?
- Specifically, does the spacing match the spec?
Check through changed files:
- Nothing out of scope appears to have been changed
- Double-check for typos or anything else that feels inaccurate or illogical
Check Windows High Contrast Mode (I noted that this looked different in AssistivLabs and forced-colors rendering in Chrome, it's not a bad idea to check both if you have the bandwidth):
- Colors are high contrast and make sense given the way other components are often styled
Check the Docs page:
- No typos, information covers the changes appropriately
- I left documentation about the removal of the invalid tag out, but could add it, open to opinions here!
Check the Storybook controls:
- They work and cover all variants, states and combinations thereof
Check the testing grid:
- Is there sufficient test coverage? Are there any states that aren't covered? Any ideas for organizing this better?
- Specifically, I didn't cover focus + hover, is there any need to for Tag?
View read-only state. I'll likely start a Slack conversation about this, but I think this work can be reviewed without knowing the answer:
- Read-only behaves as expected - they appear as normal tags, but you can't interact with them, disabled should always take precedence if a tag is read-only and disabled.
Clear button
- Does the clear button behave as expected?

Regression testing

Validate:

The documentation pages for at least two other components are still loading, including:

The pages render correctly, are accessible, and are responsive.

Screenshots

Spectrum 2:

Spectrum 1:

To-do list

I have read the contribution guidelines.
I have updated relevant storybook stories and templates.
I have tested these changes in Windows High Contrast mode.
If my change impacts documentation, I have updated the documentation accordingly.
✨ This pull request is ready to merge. ✨

changeset-bot · 2025-04-22T21:33:56Z

🦋 Changeset detected

Latest commit: 67a2f8d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@spectrum-css/tag	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2025-04-22T21:38:53Z

File metrics

Summary

Total size: 1.38 MB^*

Package	Size	Minified	Gzipped
tag	20.85 KB	19.95 KB	2.87 KB

tag

Filename	Head	Minified	Gzipped	Compared to base
index.css	20.85 KB	19.95 KB	2.87 KB	🟢 ⬇ 6.67 KB
metadata.json	10.28 KB	-	-	🟢 ⬇ 3.93 KB

* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

github-actions · 2025-04-22T21:39:38Z

🚀 Deployed on https://pr-3682--spectrum-css.netlify.app

components/tag/index.css

components/tag/stories/tag.stories.js

rise-erpelding · 2025-04-23T16:50:14Z

components/tag/stories/tag.stories.js

+/**
+ * Tags have a read-only option for when content in the disabled state still needs to be shown.
+ * Read-only tags cannot be interacted with or changed.
+ */
+export const ReadOnly = TagsDefaultOptions.bind({});
+ReadOnly.storyName = "Read only";
+ReadOnly.tags = ["!dev"];
+ReadOnly.args = {
+	isReadOnly: true,
+};
+ReadOnly.parameters = {
+	chromatic: { disableSnapshot: true },
+};
+


This is a follow-up item also, it wasn't in the design spec but has existed in the S1 design guidelines.

The guidelines also show the label text as being selectable and copyable, but currently it can't be selected and copied. The intended behavior needs some clarification; maybe something else to note for the follow-up.

I removed user-select: none; from the tag, so now the text is selectable (but not copy-able, is there a native way to do that in CSS?). Happy to restore it if that causes unintended issues though.

Is the text still not copy-able for either of you? I can copy and paste it from a read-only tag.

Tag label that i copied

I don't believe that functionality is native to HTML or CSS; making text copy-able to the clipboard would need JavaScript. I don't think I'm mentioning that in the docs anywhere but should I? I would hope that consumers would look at the design guidelines instead of solely trusting our Storybook documentation, but I know sometimes we mention that a tooltip or other thing that's outside of the scope of our implementation should be available.

components/tag/stories/template.js

components/tag/stories/tag.test.js

marissahuysentruyt

Another PR, back from the dead! Thanks for the explanation on the padding/margin stuff with all of the new options- very helpful. I think overall the tokens are in a really good spot- there was only one I didn't see in the CSS (about the down state).

I found some style criss-crossing when I started to really mix up the controls, so I left some ideas on how to expand the test file so we covered more of those.

But I had just one extra thing to add, that I couldn't add to the code...

There's a note about the tooltip on hover to reveal the full text in the docs for text overflow. Is it helpful at all the mention that our component doesn't do that? 😆

components/tag/stories/tag.stories.js

components/tag/stories/template.js

marissahuysentruyt · 2025-04-25T16:28:14Z

components/tag/index.css

+		color: var(--spectrum-tag-content-color-active);
+
+		/* stylelint-disable-next-line spectrum-tools/no-unknown-custom-properties -- height and width are set by implementations */
+		transform: perspective(max(var(--spectrum-downstate-height), var(--spectrum-downstate-width) * var(--spectrum-component-size-width-ratio-down))) translateZ(var(--spectrum-component-size-difference-down));


I didn't see component-size-minimum-perspective-down token in the CSS. Is that supposed to be added to the smallest size tag, sort of like the extra small and small action buttons use that token?

My understanding based on how this is documented on our Foundations page is that we use minimum perspective for little components, and calculated perspective (which needs the down state decorator) for anything with a width larger than 24px. It's true that it's possible for tag-minimum-width-small to be 21px for desktop, but I believe in that case the height for the small tag would be 24px and so, using the max formula to determine whether the height or width is larger, we'd get the same perspective value as if we'd used the --component-size-minimum-perspective-down token (which is 24px).

My guess is that --component-size-minimum-perspective-down might be included in the spec as sort of a catchall that they add to all components that receive the down state treatment?

components/tag/index.css

marissahuysentruyt · 2025-04-25T17:07:54Z

components/tag/index.css


 	border-radius: var(--mod-tag-corner-radius, var(--spectrum-tag-corner-radius));
 	border-width: var(--mod-tag-border-width, var(--spectrum-tag-border-width));
 	border-style: solid;

-	padding-inline-start: calc(var(--mod-tag-spacing-inline-start, var(--spectrum-tag-spacing-inline-start)) - var(--mod-tag-border-width, var(--spectrum-tag-border-width)));
+	padding-inline-start: calc(var(--mod-tag-label-spacing-inline, var(--spectrum-tag-label-spacing-inline)) - var(--mod-tag-border-width, var(--spectrum-tag-border-width)));


Thanks for this explanation- it's pretty wild to get these spacing values correct.

I did notice an inconsistency with the label's margin-inline-end when it's next to the clear button. I think maybe we don't need to subtract the border width? The medium and large spacing between the label text and clear button is off by one if I compare to Figma. Technically, the small spacing checks out correctly, but that's because the token is set to 9px instead of 8px like Figma says.

Unfortnately, I don't really have a solution to offer. I can spend some time and see what I come up with.

jawinn · 2025-04-28T18:23:00Z

components/tag/stories/tag.stories.js

+/**
+ * Tags have a read-only option for when content in the disabled state still needs to be shown.
+ * Read-only tags cannot be interacted with or changed.
+ */
+export const ReadOnly = TagsDefaultOptions.bind({});
+ReadOnly.storyName = "Read only";
+ReadOnly.tags = ["!dev"];
+ReadOnly.args = {
+	isReadOnly: true,
+};
+ReadOnly.parameters = {
+	chromatic: { disableSnapshot: true },
+};
+


The guidelines also show the label text as being selectable and copyable, but currently it can't be selected and copied. The intended behavior needs some clarification; maybe something else to note for the follow-up.

jawinn · 2025-04-28T18:56:45Z

components/tag/stories/tag.test.js

+			isSelected: true,
+		},
+		{
+			testHeading: "Emphasized, selected",


Good callout on this one Marissa. I was wondering the same thing when looking this over. Looking at the desktop file and the playground, Figma only allows turning on emphasized in combination with selected. We don't have updated guidelines but I think you may be right that is only affects the selected state.

In that case, the emphasized Docs entry could be "Default - selected + emphasized" with "The emphasized option uses emphasized colors only when the tag is selected". And tests would need a few updates. Along with styles to make sure just emphasized or emphasized + removable doesn't show emphasized colors.

jawinn · 2025-04-28T19:01:31Z

components/tag/stories/tag.test.js

+		{
+			testHeading: "Emphasized, read-only",
+			isReadOnly: true,
+			isEmphasized: true,
+		},


This one was showing emphasized colors but I would expect it to show read-only colors.

Because we expect emphasized colors to take effect only when it's selected? I think I have this updated properly now:

jawinn · 2025-04-28T19:29:35Z

components/tag/stories/template.js

+			@focusin=${function() {
+				if (isReadOnly || isDisabled) return;
+				updateArgs({ isFocused: true });
+			}}
+			@focusout=${function() {
+				updateArgs({ isFocused: false });
+			}}


I'd recommend removing this, as the focus events are not the same behavior as :focus-visible. So currently if you click on the tag in the Default story, the focus indicator will appear when it shouldn't.

Ok! I'm using the keyboard focused instead of focused control now as well. I think that I'd thought it would be nice to see the control toggling when you keyboard focus, but that does have unintended side effects.

jawinn · 2025-04-28T19:34:05Z

components/tag/stories/tag.stories.js

Docs addition suggestion: I think it's worth mentioning in the Truncated story description that tags have a maximum width set. Since that might not be expected until it was encountered.

This shouldn't have had a maximum width set on the custom styles, since tag already has a max inline size set. I updated this story to show the truncated, max-width tag for all 3 sizes.

jawinn · 2025-04-28T19:39:07Z

components/tag/index.css

-		background-color: var(--highcontrast-tag-background-color-focus, var(--mod-tag-background-color-focus, var(--spectrum-tag-background-color-focus)));
-		color: var(--highcontrast-tag-content-color-focus, var(--mod-tag-content-color-focus, var(--spectrum-tag-content-color-focus)));
+	&:not(.is-disabled, .is-readOnly):focus-visible,
+	&:not(.is-disabled, .is-readOnly).is-focused {


Do we still need .is-focused for S2 if it's already handled by the :focus-visible? Is this a legacy thing? If needed for the implementation to apply, it should probably be documented.

I don't think there's a reason that it needs .is-focused, it's now using :focus-visible only and keyboard focused in the controls.

jawinn · 2025-04-28T19:55:05Z

components/tag/index.css

+	--spectrum-tag-background-color-selected: var(--highcontrast-tag-background-color-selected, var(--mod-tag-background-color-selected, var(--spectrum-neutral-background-color-default)));
+	--spectrum-tag-background-color-selected-hover: var(--highcontrast-tag-background-color-selected-hover, var(--mod-tag-background-color-selected-hover, var(--spectrum-neutral-background-color-hover)));
+	--spectrum-tag-background-color-selected-active: var(--highcontrast-tag-background-color-selected-active, var(--mod-tag-background-color-selected-active, var(--spectrum-neutral-background-color-down)));
+	--spectrum-tag-background-color-selected-focus: var(--highcontrast-tag-background-color-selected-focus, var(--mod-tag-background-color-selected-focus, var(--spectrum-neutral-background-color-key-focus)));


Could "selected" tokens be used for the value of existing custom properties in .spectrum-Tag.is-selected, instead of creating new custom properties? For example:

&.is-selected { --spectrum-tag-background-color: var(--highcontrast-tag-background-color-selected, var(--mod-tag-background-color-selected, var(--spectrum-neutral-background-color-default))); --spectrum-tag-content-color: var(--highcontrast-tag-content-color-selected, var(--mod-tag-content-color-selected, var(--spectrum-gray-25))); --spectrum-tag-border-color: var(--highcontrast-tag-border-color-selected, var(--mod-tag-border-color-selected, transparent)); }

I'll have to come back to this one, I made an attempt but started getting tangled when it came to hover/focus/active states. Is button a good component to reference to see where else it's done like this?

Done in 9624755

rise-erpelding · 2025-04-30T20:59:15Z

components/tag/index.css

 	outline: none;
-	user-select: none;


Is there a reason to keep this?

@rise-erpelding I think it's worth keeping it. user-select: none prevents selection or highlighting of text elements. If you double click it you'll see the text element is highlighted by the browser. If you add user-select: none the highlight disappears.

@rise-erpelding I agree with the removal, and don't see a reason to keep user-select: none. If we want to be able to copy and highlight that text, like the guidelines say for read-only, and I think you and Josh already we talking about, removing user-select: none should give us that ability.

The only "behavior" I'm unsure of is if the disabled tag is copy-able. Maybe user-select: none has to be on a disabled tag?

I think slapping a user-select: none; next to the pointer-events: none; for disabled is a good call. Made that addition.

rise-erpelding · 2025-04-30T21:02:56Z

components/tag/index.css

+
+	.spectrum-Thumbnail {
+		/* prevent thumbnail shrinking when label is longer */
+		flex-shrink: 0;


Unclear why only thumbnail seems to shrink when the label is long.

I did think about adding thumbnail/avatar/icon to the truncated section of tests and didn't do it because it would involve moving several things around the test file and the testing grid is already starting to feel a little bloated to me, I'm open to the idea though.

aramos-adobe · 2025-05-07T21:41:16Z

.changeset/khaki-icons-hammer.md

+
+Tag now uses Spectrum 2 tokens and specifications:
+
+- The invalid variant has been removed.


I would also add it's been removed because there are no individual invalid tags and errors are displayed in tag groups.

marissahuysentruyt

Thanks- a bunch of my initial feedback is complete and addressed!

I think my biggest curiosity, that I haven't quite worked out yet, is the truncation and close button issue I mentioned. Maybe we need

position: absolute;
inset-inline-end: calc(that-you-already-have); // margin-inline-end changes to inset-inline-end

And then maybe some max-inline-size control for the label itself, so it doesn't "go past" the clear button? Sort of like the wacky padding we have for inputs to accommodate icons within fields?

components/tag/stories/tag.stories.js

marissahuysentruyt · 2025-05-07T20:55:19Z

components/tag/stories/tag.test.js

+			isSelected: true,
+		},
+		{
+			testHeading: "Emphasized, selected",


I could see some refactoring of the testing grid. Just to lump more of the states & variants together, sort of like how checkbox or action button have a BUNCH of states.

(this is what I can see for this test grid, but obviously with correct headings! 😆 )

marissahuysentruyt · 2025-05-07T21:04:43Z

components/tag/stories/tag.stories.js

+/**
+ * Tags have a read-only option for when content in the disabled state still needs to be shown.
+ * Read-only tags cannot be interacted with or changed.
+ */
+export const ReadOnly = TagsDefaultOptions.bind({});
+ReadOnly.storyName = "Read only";
+ReadOnly.tags = ["!dev"];
+ReadOnly.args = {
+	isReadOnly: true,
+};
+ReadOnly.parameters = {
+	chromatic: { disableSnapshot: true },
+};
+


Is the text still not copy-able for either of you? I can copy and paste it from a read-only tag.

Tag label that i copied

marissahuysentruyt · 2025-05-07T21:35:05Z

components/tag/index.css


 	border-radius: var(--mod-tag-corner-radius, var(--spectrum-tag-corner-radius));
 	border-width: var(--mod-tag-border-width, var(--spectrum-tag-border-width));
 	border-style: solid;

-	padding-inline-start: calc(var(--mod-tag-spacing-inline-start, var(--spectrum-tag-spacing-inline-start)) - var(--mod-tag-border-width, var(--spectrum-tag-border-width)));
+	padding-inline-start: calc(var(--mod-tag-label-spacing-inline, var(--spectrum-tag-label-spacing-inline)) - var(--mod-tag-border-width, var(--spectrum-tag-border-width)));


@rise-erpelding I'm not certain where the discrepancy is, so don't take the fact that I'm commenting here to heart too much! This felt sort of like a similar conversation...

I'm wondering about the truncation and the clear button now. I can really smoosh that clear button to the edge of the tag with a long label. How do we avoid that?

aramos-adobe · 2025-05-07T22:03:36Z

components/tag/index.css

+	.spectrum-Thumbnail {
+		/* set visual inline-start spacing, but take away label spacing, this will result in negative margin - this calc does not need border width */
+		margin-inline-start: calc(var(--mod-tag-visual-spacing-inline-start, var(--spectrum-tag-visual-spacing-inline-start)) - var(--mod-tag-label-spacing-inline, var(--spectrum-tag-label-spacing-inline)));
+		margin-inline-end: var(--mod-tag-visual-spacing-inline-end, var(--spectrum-tag-visual-spacing-inline-end));
 	}

 	/* clear button */
 	.spectrum-Tag-clearButton {


@rise-erpelding, I also spotted what @marissahuysentruyt discovered when the longer the string in truncation mode the clear button magically slides over to the edge. One thing I tried to add:

.spectrum-Tag-clearButton { inset-inline-end: var(--spectrum-tag-edge-to-clear-icon); position: relative; padding-inline-start: var(--spectrum-tag-label-to-clear-icon); }

The longer the text it doesn't move over.

- updates to support visual content in storybook template: - icon and avatar controls moved to content section - thumbnail added - add visual control menu to choose visual -remove invalid variant references and options - support active, focus, hover - add documentation - minor refactoring and cleanup - support readonly

- remove selected template - add removable tag to grouped template - prevent showing of duplicate removable tag for in removable story - use grouped template for sizing

ADJUST FOCUS BEHAVIOR: - remove .is-focused states from CSS to use native pseudo-classes - use keyboard focused global state instead of focused global state in Storybook - change of arg from isFocused to isKeyboardFocused in template and tests - removal of focus event listeners to update args ADJUST EMPHASIZED STYLING - .is-emphasized is now .spectrum-Tag--emphasized in CSS and template - emphasized styles only take effect if also selected - update tests and test headings to better display the emphasized variant - small adjustments to story names on docs page to put selected stories next to each other and show emphasized and selected rather than just emphasized ADJUST READ ONLY STYLING - Remove user-select: none to make tag text selectable per guidelines - prevent selected styles from having effect if read-only or disabled - adjust emphasized & read-only tests to include selected state MISC OTHER FIXES - hasClearButton control has been changed to isRemovable - Update of Truncated story to remove max width and rely on component's max width

- change avatar size to 75 for medium - use flex-shrink to prevent thumbnail width shrinking when the label is long - use size map in the template for avatar and thumbnail sizing - fix removable tag functionality - remove focusability on clear button (it doesn't remove on delete but is that functionality that's needed here?), revert margin on clear button back to padding where it's not negative - add default sizing info on sizing story - combine inset focus indicator properties to use shorthand

This commit adjusts the spacing of the clear button to match the spec. Between the label and clear button the spacing should be as follows: S: 8px (Desktop) M: 12px (Desktop) L: 15px (Desktop) The label subtracts border width from the spacing, but when clear button exists, the label doesn't need to subtract the border width, so we'll add border width to the clear button spacing to cancel this out. The spacing between the label and clear button is the sum of the padding at the end of the label and the start of the clear button. I think the best way to test this is to add a mod for the border width, and verify that the spacing at the end of the label and start of the clear button continues to match the spec as mentioned above.

- puts clear button spacing tokens into calc for clear button size in order to expand target area - removes older clear button calcs, modified calcs serve to cancel out unneeded spacing from item label

rise-erpelding · 2025-05-09T14:01:53Z

New changes! ✨

test(tag): refactor testing template
I'm hoping I didn't lose any test cases in doing this. Selected states are now combined with unselected states. Since we have flex-shrink problems sometimes, I'm also adding visuals (icon, avatar, thumbnail) and visuals + clear button to the truncated section to test those with longer text.
fix(tag): keep clear button from shrinking
The clear button is a flex item! It just needed a flex-shrink: 0; to keep it from shrinking with longer text, I think.
chore(tag): add migrated tag
Adds the correct taggage.
chore(tag): update changeset
Explains the removal of the invalid variant a little better.

rise-erpelding · 2025-05-09T14:39:59Z

Forgot a couple of notes in my haste to reopen this for review!

I've noticed that sometimes the truncated text doesn't go to the edge of the itemLabel (like inside of the blue in the screenshot here), making it look like there's extra space. I think this is ok and just dependent on whatever is in the label text.

This is an older screenshot of the testing grid but the emphasized + selected colors in WHCM does bug me, but it's not different from what we have for emphasized in Foundations. I'd like to clarify WHCM specs at some point and we do have a ticket for that.

aramos-adobe · 2025-05-09T16:08:37Z

components/tag/stories/tag.stories.js

+	withHeading: false,
+	withBorder: false,
+	...args,
+}, context);
 TextOverflow.tags = ["!dev"];
 TextOverflow.args = {
 	hasIcon: true,
 	iconName: "CheckmarkCircle",
 	label: "An example of text overflow behavior. When the button text is too long for the horizontal space available, it will truncate and stay on one line.",


Super minor, but you mentioned "when the tag text" in the JS Doc but says "button text" here in the label prop - should be "tag text" too

aramos-adobe · 2025-05-09T16:09:16Z

.changeset/khaki-icons-hammer.md

+- "--mod-tag-avatar-spacing-inline-end" --> "--mod-tag-visual-spacing-inline-end",
+- "--mod-tag-icon-spacing-inline-end" --> "--mod-tag-visual-spacing-inline-end",
+- "--mod-tag-spacing-inline-start" --> "--mod-tag-label-spacing-inline",
+- "--mod-tag-label-spacing-inline-end" --> "--mod-tag-label-spacing-inline"


Add a comma at the end here

aramos-adobe

Really nifty fix using the flex-shrink on the clear button there! Overall this PR is so well done and I like the test cases you added.

marissahuysentruyt

Wonderful work on this, Rise! I'm going to approve, but I do suggest making the change on the location of the status object for the migrated badge before we merge! Looks like there's a few from Aziz from Friday, but overall, the migration looks great to me, and all of my concerns from the clear button are ffffiiiixxxed! Thanks for all your hard work!

marissahuysentruyt · 2025-05-12T14:03:04Z

components/tag/stories/tag.stories.js

+	status: {
+		type: "migrated",
 	},


This status object should go in parameters (at line 109ish) in order to get picked up properly- sorry I didn't clarify in my last comment.

I definitely should have been checking to see if this was doing anything 😂

marissahuysentruyt · 2025-05-12T14:36:01Z

components/tag/index.css

 	outline: none;
-	user-select: none;


@rise-erpelding I agree with the removal, and don't see a reason to keep user-select: none. If we want to be able to copy and highlight that text, like the guidelines say for read-only, and I think you and Josh already we talking about, removing user-select: none should give us that ability.

The only "behavior" I'm unsure of is if the disabled tag is copy-able. Maybe user-select: none has to be on a disabled tag?

marissahuysentruyt · 2025-05-12T14:44:06Z

components/tag/stories/template.js

+		${args.isRemovable ? "" : Template({
+			...args,
+			isRemovable: true,
+			}, context)}


If I haven't said it already, this is solution to showing the removable tags in this template is great!

rise-erpelding added wip This is a work in progress, don't judge. S2 Spectrum 2 labels Apr 22, 2025

rise-erpelding force-pushed the rise-erpelding/tag-s2-pt2 branch 2 times, most recently from 937827f to f5cf2f3 Compare April 23, 2025 16:12

rise-erpelding marked this pull request as ready for review April 23, 2025 16:13

rise-erpelding added ready-for-review and removed wip This is a work in progress, don't judge. labels Apr 23, 2025

rise-erpelding commented Apr 23, 2025

View reviewed changes

rise-erpelding mentioned this pull request Apr 23, 2025

feat(tag): migrate to Spectrum 2 #2649

Closed

17 tasks

rise-erpelding commented Apr 24, 2025

View reviewed changes

components/tag/stories/tag.test.js Show resolved Hide resolved

rise-erpelding self-assigned this Apr 24, 2025

marissahuysentruyt requested changes Apr 25, 2025

View reviewed changes

jawinn previously requested changes Apr 28, 2025

View reviewed changes

rise-erpelding force-pushed the rise-erpelding/tag-s2-pt2 branch 2 times, most recently from 51c77f3 to eec2584 Compare April 30, 2025 00:58

rise-erpelding commented Apr 30, 2025

View reviewed changes

castastrophe force-pushed the spectrum-two branch 3 times, most recently from 7e8269c to 4ce8d19 Compare May 5, 2025 21:18

rise-erpelding force-pushed the rise-erpelding/tag-s2-pt2 branch 2 times, most recently from 6534c35 to 7094320 Compare May 7, 2025 15:52

rise-erpelding requested review from marissahuysentruyt and aramos-adobe May 7, 2025 20:50

aramos-adobe reviewed May 7, 2025

View reviewed changes

marissahuysentruyt requested changes May 7, 2025

View reviewed changes

aramos-adobe reviewed May 7, 2025

View reviewed changes

rise-erpelding added 15 commits May 9, 2025 08:45

test(tag): set up testing grid

daefae4

chore(tag): metadata

7cca8b0

chore(tag): changeset

271c120

docs(tag): template adjustments

e2ea24b

- remove selected template - add removable tag to grouped template - prevent showing of duplicate removable tag for in removable story - use grouped template for sizing

fix(tag): make small tag's default size

a35f237

fix(tag): expand clear button target area

bc0e940

- puts clear button spacing tokens into calc for clear button size in order to expand target area - removes older clear button calcs, modified calcs serve to cancel out unneeded spacing from item label

refactor(tag): redefine custom props for variants

0d11ea0

chore(tag): update changeset

3fe3d56

test(tag): refactor testing template

36f6506

fix(tag): keep clear button from shrinking

d8732cd

chore(tag): add migrated tag

d02e832

rise-erpelding force-pushed the rise-erpelding/tag-s2-pt2 branch from cd7ce1c to d02e832 Compare May 9, 2025 13:46

chore(tag): update changeset

895417b

rise-erpelding requested review from marissahuysentruyt and aramos-adobe May 9, 2025 14:01

aramos-adobe reviewed May 9, 2025

View reviewed changes

aramos-adobe approved these changes May 9, 2025

View reviewed changes

marissahuysentruyt approved these changes May 12, 2025

View reviewed changes

fix(tag): move migrated tag and update disabled state actions

67a2f8d

rise-erpelding merged commit 6d53893 into spectrum-two May 12, 2025
12 checks passed

rise-erpelding deleted the rise-erpelding/tag-s2-pt2 branch May 12, 2025 18:21

This was referenced May 16, 2025

chore(swatch,menu,swatchgroup): fix reverted metadata assets #3752

Merged

chore: merge latest updates from main #3777

Merged


		Tag now uses Spectrum 2 tokens and specifications:

		- The invalid variant has been removed.

feat(tag): migrate to S2 #3682

feat(tag): migrate to S2 #3682

Conversation

rise-erpelding commented Apr 22, 2025 • edited by marissahuysentruyt Loading

Description

How and where has this been tested?

Validation steps

Regression testing

Screenshots

To-do list

changeset-bot bot commented Apr 22, 2025 • edited Loading

🦋 Changeset detected

github-actions bot commented Apr 22, 2025 • edited Loading

File metrics

Summary

tag

github-actions bot commented Apr 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rise-erpelding May 9, 2025 • edited Loading

Choose a reason for hiding this comment

marissahuysentruyt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jawinn Apr 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marissahuysentruyt May 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aramos-adobe May 7, 2025 • edited Loading

Choose a reason for hiding this comment

marissahuysentruyt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rise-erpelding commented May 9, 2025

rise-erpelding commented May 9, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aramos-adobe left a comment

Choose a reason for hiding this comment

marissahuysentruyt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marissahuysentruyt May 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rise-erpelding commented Apr 22, 2025 •

edited by marissahuysentruyt

Loading

changeset-bot bot commented Apr 22, 2025 •

edited

Loading

github-actions bot commented Apr 22, 2025 •

edited

Loading

github-actions bot commented Apr 22, 2025 •

edited

Loading

rise-erpelding May 9, 2025 •

edited

Loading

jawinn Apr 28, 2025 •

edited

Loading

marissahuysentruyt May 12, 2025 •

edited

Loading

aramos-adobe May 7, 2025 •

edited

Loading

marissahuysentruyt May 12, 2025 •

edited

Loading