spectrum-css icon indicating copy to clipboard operation
spectrum-css copied to clipboard

feat(tag): migrate to Spectrum 2

Open rise-erpelding opened this issue 1 year ago • 4 comments

Description

Migrates the Tag component to Spectrum 2, using new and updated tokens, removing borders, and adding the ability to embed the Thumbnail component.

Express and Spectrum themes removed

Express and Spectrum themes in express.css and spectrum.css respectively have been removed, and relevant styles moved into the index.css file.

Medium is default size

All sizeM styles are now the default.

New tokens

New tokens for Spectrum 2, including

  • corner rounding (now scales with Tag size)
  • minimum and maximum width
  • updated spacing tokens

Color and border changes

  • Borders have been removed (they have become transparent in most cases; if Windows high-contrast mode is used, Tag still has borders)
  • Background and content colors have been adjusted slightly to match Spectrum 2 design spec

Thumbnail added

Tag with Thumbnail is now a visual that is supported (along with Avatar and Icon).

Visuals scale with size change

Small, medium, and large Tags each have appropriately sized icons, avatars, or thumbnails which scale as the component changes size.

Spacing Adjustments

Spacing has been adjusted to match design spec for S2: Tag with label only The tag (.spectrum-Tag) controls the inline-start spacing, while the label (.spectrum-itemLabel) controls the inline-end spacing. Border width is subtracted when setting the margin/padding. Note that .spectrum-Tag uses inline-start padding because it’s inline-flex.

Tag with label and clear button The correct clear button inline-start and inline-end spacing are applied to .spectrum-Tag-clearButton, but since the spacing from the label is still being applied (which has the border width subtracted), this is subtracted from the margin-inline-start being added. This results in 1px of margin being added to the start of the clear button for M and L variants, because the margin being applied to the label is 1px smaller to account for border width.

Tag with Visual (Icon, Avatar, Thumbnail) These all use the same spacing tokens (component-edge-to-visual, component-text-to-visual) but there are individual custom props for each visual type to try to avoid issues with migration. Inline-start margin on the visual is offset negatively to account for the difference between the tag padding and what the edge-to-visual value should be. Inline-end spacing remains the same as before.

CSS-618

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

  • [ ] Validate colors (emphasized, invalid, selected, default)
  • [ ] Validate disabled state (should not have hover state, should not have pointer cursor for clear button, avatars/thumbnails should be dimming)
  • [ ] View testing preview - are any variants missing that could be useful?

Validate Spacing at each size:

  • [ ] Label only
  • [ ] Label with clear button
  • [ ] Label with Icon
  • [ ] Label with Avatar
  • [ ] Label with Thumbnail
  • [ ] Label with Visual (Avatar, Thumbnail, or Icon) and clear button

Regression testing

Validate:

  1. The documentation pages for at least two other components are still loading, including:
  • [ ] The pages render correctly, are accessible, and are responsive.
  1. If components have been modified, VRTs have been run on this branch:
  • [ ] VRTs have been run and looked at.
  • [ ] Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

To-do list

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

rise-erpelding avatar Apr 08 '24 21:04 rise-erpelding

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

github-actions[bot] avatar Apr 08 '24 21:04 github-actions[bot]

File metrics

Summary

Total size: 4.52 MB* Total change (Δ): ⬇ 36.60 KB (-0.78%)

Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.

Package Size Δ
tag 27.69 KB ⬇ 8.70 KB

Details

tag

File Head Base Δ
index-base.css 27.69 KB 31.03 KB ⬇ 3.34 KB (-10.77%)
index-vars.css 27.69 KB 36.39 KB ⬇ 8.70 KB (-23.91%)
index.css 27.69 KB 36.39 KB ⬇ 8.70 KB (-23.91%)
metadata.json 13.68 KB 17.08 KB ⬇ 3.40 KB (-19.92%)
index-theme.css - 5.94 KB 🚨 deleted, moved, or renamed
themes/express.css - 3.25 KB 🚨 deleted, moved, or renamed
themes/spectrum.css - 3.27 KB 🚨 deleted, moved, or renamed
* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

github-actions[bot] avatar Apr 08 '24 21:04 github-actions[bot]

Thanks for the feedback! Here's a list of things that have been fixed since being reviewed and a list of outstanding questions:

Fixes made:

  • Unneeded sizes have been removed from the object mapping size abbreviations to size words
  • Description added for Visual select menu in Storybook
  • Font-weight updated to medium (was previously regular)
  • List of mods added
  • Added Thumbnail opacity when disabled, I can easily remove it if for some reason it’s not wanted though
  • Invalid variant color changes for WHCM

Questions for design:

  • Thumbnail opacity is decreased when tag is disabled, similar to Avatar—there was no design spec for this so wanted to confirm that’s wanted here.
  • Invalid disabled selected state - this looks just like Invalid selected and it seems like perhaps disabled styles ought to take precedence here.
  • Should we be using down state scale tokens?

rise-erpelding avatar Apr 12 '24 19:04 rise-erpelding

⚠️ No Changeset found

Latest commit: fb7d0e36d2ddcdacb50a6eaaec7fbbc395a591ae

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

changeset-bot[bot] avatar Apr 19 '24 19:04 changeset-bot[bot]

Closing in favor of #3682

rise-erpelding avatar Apr 23 '25 16:04 rise-erpelding