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

refactor(tag)!: migrate to core tokens

Open lunarfusion opened this issue 3 years ago • 2 comments

Description

BREAKING CHANGE: This migrates the Tag component to core tokens.

  • Jira issue CSS-181 https://jira.corp.adobe.com/browse/CSS-181

How and where has this been tested?

  • Chrome 103 for macOS
  • Firefox 102 for macOS
  • Safari 15.5 for macOS
  • How this was tested: comparing local build of http://localhost:3000/docs/tag.html with XD redline for Tag and with https://spectrum.adobe.com/page/tag

Screenshots

Spectrum variants: Screen Shot 2022-09-20 at 5 28 33 PM

Specrum t-shirt sizes: Screen Shot 2022-09-20 at 5 29 58 PM

Spectrum focus indicator: Screen Shot 2022-09-21 at 11 05 31 AM

Express variants: Screen Shot 2022-09-20 at 5 31 11 PM ress variants:

Express t-shirt sizes: Screen Shot 2022-09-20 at 5 31 49 PM

Express focus indicator: Screen Shot 2022-09-21 at 11 05 19 AM

To-do list

  • [ ] If my change impacts other components, I have tested to make sure they don't break.
  • [x] If my change impacts documentation, I have updated the documentation accordingly.
  • [x] I have read the CONTRIBUTING document.
  • [x] I have tested these changes in Windows High Contrast mode.
  • [ ] This pull request is ready to merge.

lunarfusion avatar Sep 20 '22 21:09 lunarfusion

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

github-actions[bot] avatar Sep 20 '22 21:09 github-actions[bot]

The code here looks good, @lunarfusion! Thanks for working through this. It looks like this is the first one where there are a lot of differences between Spectrum & Express! Exciting.

I did want to mention that it looks like the avatar is getting awfully close to the edge of the tag, especially in Express Screen Shot 2022-09-29 at 11 04 05 AM

Would you mind taking a look at that?

I checked everything out and there was one mistake I made (needed to remove the tag border width from the avatar space to top). However, this did not correct the problem in spectrum-large, express theme due to an error when the tokens were created.

screen_shot_2022-09-29_at_4 37 39_pm screen_shot_2022-09-29_at_4 54 15_pm

Lynn Hao has created a ticket to resolve this: https://jira.corp.adobe.com/browse/DNA-1217

lunarfusion avatar Oct 03 '22 19:10 lunarfusion

Deploy shows the token issue in express causing avatar misalignment is resolved. Screen Shot 2022-10-20 at 3 05 27 PM

lunarfusion avatar Oct 20 '22 19:10 lunarfusion

New commit due to token names having been changed since the component was migrated.

--spectrum-focus-ring-thickness ➟ --spectrum-focus-indicator-thickness --spectrum-focus-ring-gap ➟ --spectrum-focus-indicator-gap --spectrum-focus-ring-color ➟ --spectrum-focus-indicator-color --spectrum-negative-content-color ➟ --spectrum-negative-content-color-default

This component should be ready for beta now if you approve, @pfulton. Thanks!

lunarfusion avatar Oct 21 '22 12:10 lunarfusion

Rebased against the latest code in main and released the beta: @spectrum-css/[email protected]

pfulton avatar Nov 01 '22 14:11 pfulton

New commit due to token names having been changed since the component was migrated.

--spectrum-focus-ring-thickness ➟ --spectrum-focus-indicator-thickness --spectrum-focus-ring-gap ➟ --spectrum-focus-indicator-gap --spectrum-focus-ring-color ➟ --spectrum-focus-indicator-color --spectrum-negative-content-color ➟ --spectrum-negative-content-color-default

This component should be ready for beta now if you approve, @pfulton. Thanks!

Yeah, had to do the same migration with the re-naming of the indicator.

bernhard-adobe avatar Nov 01 '22 17:11 bernhard-adobe