refactor(switch): use core tokens #2561
Conversation
|
From these VRTs: https://ccaa0f962774b405b6b634e0a9dc979c--spectrum-web-components.netlify.app/review/ this either is expecting a newer version of the Tokens package or some update HTML. Can you clarify @bernhard-adobe? The branch is also out of date and might not have the latest versions from |
Tachometer resultsChromeaction-bar permalink
action-menu permalink
card permalink
illustrated-message permalink
menu permalink
overlay permalink
picker permalink
popover permalink
slider permalink
split-button permalink
switch permalink
tooltip permalink
Firefoxaction-bar permalink
action-menu permalink
card permalink
illustrated-message permalink
menu permalink
overlay permalink
picker permalink
popover permalink
slider permalink
split-button permalink
switch permalink
tooltip permalink
|
Thanks @Westbrook . We made HTML changes and I will update the docs accordingly. We now offer t-shirt sizing for Switch and I think that requires the updates. |
|
Starting to look good here @bernhard-adobe! Can you add documentation for the t-shirt sizing, a la: https://github.com/adobe/spectrum-web-components/pull/2471/files#diff-8bae4c0fc7a00665c41e46959ef1459db31ef40dee44e9a6587a08ac118f6832 and update the stories to exhibit the t-shirt sizes singularly, a la: https://github.com/adobe/spectrum-web-components/pull/2471/files#diff-81d397bcd494c209576bbcaa66ee5243b133b827d13498a23e4ac52555bf74cd then we should be good to update the new Golden Images Cache and, baring a full release at the CSS level, ship this! |
Makes sense! I added the t-shirt sizing |
Westbrook
left a comment
Westbrook
left a comment
There was a problem hiding this comment.
Those two missed lines and the stable release and this is looking great! Thanks @bernhard-adobe
|
@Westbrook I committed your suggested changes. |
|
For the VRT updates, you can checkout https://vrt--spectrum-web-components.netlify.app/bschmidt/switch-core-tokens to ensure that things are looking as expected across all of the themes, etc. If so, grab the has in the third Side Nav Item there and update it following the instructions you linked and CI should start to pass. |
Wow that is awesome, you are doing very well in VRT things. |
pfulton
left a comment
pfulton
left a comment
There was a problem hiding this comment.
Looks good to me, @bernhard-adobe! Thanks for taking care of this and seeing it through.
packages/switch/package.json
Outdated
| }, | ||
| "devDependencies": { | ||
| "@spectrum-css/switch": "^1.0.23" | ||
| "@spectrum-css/switch": "^2.0.0-beta.0" |
There was a problem hiding this comment.
With this approved, does that mean you'll move to a stable release here? @pfulton
There was a problem hiding this comment.
Yes, that's correct @Westbrook. I just merged and released, and we're now at @spectrum-css/switch@2.0.0
Westbrook
force-pushed
the
bschmidt/switch-core-tokens
branch
from
317b849 to
522a9ed
Compare
Westbrook
force-pushed
the
bschmidt/switch-core-tokens
branch
from
522a9ed to
d79be7e
Compare
- adding switch t-shirt size support - documenting switch t-shirt sizing
Westbrook
force-pushed
the
bschmidt/switch-core-tokens
branch
from
d79be7e to
0272c09
Compare
Westbrook
force-pushed
the
bschmidt/switch-core-tokens
branch
from
ca4b99d to
d29b5eb
Compare
Westbrook
left a comment
Westbrook
left a comment
There was a problem hiding this comment.
LGTM! Caught one work around internally that obsolesced and bumped the golden images, so we're good to go.
@bernhard-adobe thanks for diving so deep on this. You really grabbed onto some lower level pieces bringing this together! 🙏🏼 When you have time, I'd love to hear if there were any places you saw where we could make the contribution process easier/clearer.
Adds a beta version of the new Switch component.
Uses beta release, hold for full release.
Description
Related issue(s)
CSS JIra. https://jira.corp.adobe.com/browse/CSS-100 Implement%20Switch%20Core%20Tokens
CSS-100) #1496
adobe/spectrum-css#1496 feat(switch)!: migrating switch to core-tokens (CSS-42, CSS-100) #1496
Motivation and context
Core Tokens migration in Spectrum CSS.
Also breaking change as we are adding t-shirt sizing as well.
How has this been tested?
Screenshots (if appropriate)
Types of changes
Checklist
Best practices
This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against
main.