feat(color-slider): migrate to core tokens #3507

jawinn · 2023-08-02T19:45:48Z

Description

This changes the color-slider component to use the new version in spectrum CSS that has migrated to core tokens.
Applies SWC opacity checkerboard tools class to the component.

Related issue(s)

CSS PR

How has this been tested?

Confirm any differences in VRTs
Confirm that examples function and look as intended, given expected VRT differences
Confirm that it looks correct in RTL and that the gradient is reversed

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

I have signed the Adobe Open Source CLA.
My code follows the code style of this project.
If my change required a change to the documentation, I have updated the documentation in this pull request.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

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.

github-actions · 2023-08-02T19:50:46Z

Branch Preview

Visual regression test results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

github-actions · 2023-08-02T20:01:46Z

Tachometer results

Chrome

color-area permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	445 kB	219.29ms - 220.51ms	-	slower ❌ 1% - 2% 2.17ms - 3.82ms
branch	440 kB	216.35ms - 217.46ms	faster ✔ 1% - 2% 2.17ms - 3.82ms	-

color-handle permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	363 kB	71.70ms - 72.45ms	-	slower ❌ 3% - 4% 2.01ms - 3.09ms
branch	359 kB	69.14ms - 69.91ms	faster ✔ 3% - 4% 2.01ms - 3.09ms	-

color-slider permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	446 kB	182.78ms - 186.58ms	-	unsure 🔍 -0% - +2% -0.70ms - +3.24ms
branch	442 kB	182.90ms - 183.92ms	unsure 🔍 -2% - +0% -3.24ms - +0.70ms	-

color-wheel permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	448 kB	185.99ms - 187.90ms	-	slower ❌ 1% - 2% 1.58ms - 3.78ms
branch	443 kB	183.73ms - 184.81ms	faster ✔ 1% - 2% 1.58ms - 3.78ms	-

Firefox

color-area permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	445 kB	374.47ms - 396.81ms	-	unsure 🔍 -5% - +2% -20.59ms - +7.31ms
branch	440 kB	383.93ms - 400.63ms	unsure 🔍 -2% - +5% -7.31ms - +20.59ms	-

color-handle permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	363 kB	165.09ms - 177.99ms	-	faster ✔ 3% - 11% 4.83ms - 20.77ms
branch	359 kB	179.66ms - 189.02ms	slower ❌ 3% - 12% 4.83ms - 20.77ms	-

color-slider permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	446 kB	341.83ms - 364.45ms	-	faster ✔ 4% - 12% 15.73ms - 47.11ms
branch	442 kB	373.69ms - 395.43ms	slower ❌ 4% - 14% 15.73ms - 47.11ms	-

color-wheel permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	448 kB	345.17ms - 367.99ms	-	unsure 🔍 -5% - +2% -18.80ms - +8.28ms
branch	443 kB	354.56ms - 369.12ms	unsure 🔍 -2% - +5% -8.28ms - +18.80ms	-

jawinn · 2023-08-04T16:37:49Z

I've looked through the VRTs and am seeing these expected changes:

Fix/Upgrade: WHCM now shows a border around the track.
Fix: The track no longer grows in size by 2 pixels when in the disabled state.
Token Change: Disabled background color change - it's now using the --spectrum-disabled-background-color token.
Token Change: The border-radius at Large platform scale stays at 4px instead of changing to 5px. It is now using the token --spectrum-color-slider-border-rounding which does not currently change at large.
Token Change: Express track height change - The token color-control-track-width is used for the height of the track, and there is no different value specified for Express. I am double-checking in the verification channel since this is a significant visual change (edit: response was that this looks correct according to the token spec).

One other weird one shows in some of the diffs but I don't see any visual difference when scrubbing. I don't see an actual issue here:

jawinn · 2023-08-07T14:41:25Z

@Westbrook After looking over the VRTs, this is looking ready for CSS release to me. Does everything look good to you?

jawinn · 2023-08-08T19:22:50Z

It's worth noting that the new OpacityCheckerboard is not integrated yet on the SWC side. I'd recommend adding a story that shows a gradient with transparency (like the "Alpha" variant in Spectrum CSS), so that this is represented in VRTs.

Westbrook · 2023-08-09T11:49:07Z

Fix/Upgrade: WHCM now shows a border around the track.

For this, do you know if design plans add this border to other color controls? For instance, before I read this I saw this VRT and was struck by the way the two controls don't seem "related". By no means an implementation issue, so not blocking, but a good piece of planning info to have on our end.

I'd recommend adding a story that shows a gradient with transparency (like the "Alpha" variant in Spectrum CSS), so that this is represented in VRTs.

Great call, right after we support opacity selection 😬. Wanna contribute some new functionality in a follow up PR? 😉

LGTM. Seems ready for a full release!

jawinn · 2023-08-09T21:38:38Z

Fix/Upgrade: WHCM now shows a border around the track.

For this, do you know if design plans add this border to other color controls? For instance, before I read this I saw this VRT and was struck by the way the two controls don't seem "related". By no means an implementation issue, so not blocking, but a good piece of planning info to have on our end.

I agree that there should be consistency between these two components. It appears to be this same way (as changes) in the React component as well. I think that ColorArea also should probably have a similar WHCM border to ensure the boundaries of the element are clear. Otherwise the gradient can disappear into the background depending on the gradient and background used. @majornista Do you have any thoughts on this? I see that you've opened some recent WHCM issues.

najikahalsema

Looks good! Rebase, update the ci hash, etc... and it's good to go!

New version of Spectrum CSS colorslider that has migrated to using core tokens.

Remove recently added CSS for reversing the gradient in RTL mode that is now included in the new spectrum CSS colorslider (same rule now exists in generated spectrum-color-slider.css).

Use the new opacity checkerboard tools class.

Update version of spectrum CSS color slider to the latest 5.0 release, just out of beta.

Westbrook

Took a team effort, but I think we're ready to land this.

I added this comment on the CSS side to try and get to the root of what's going on in the CSS here that needed the local override.

Thanks, everyone!

* build(color-slider): use beta of 5.0 spectrum css component New version of Spectrum CSS colorslider that has migrated to using core tokens. * fix(color-slider): remove css that is now a duplicate Remove recently added CSS for reversing the gradient in RTL mode that is now included in the new spectrum CSS colorslider (same rule now exists in generated spectrum-color-slider.css). * build(color-slider): update beta version of CSS * feat(color-slider): use opacity-checkerboard Use the new opacity checkerboard tools class. * build(color-slider): update to 5.0 spectrum css release Update version of spectrum CSS color slider to the latest 5.0 release, just out of beta. * test: correct tests for layout updates * ci: update golden images cache --------- Co-authored-by: Westbrook Johnson <[email protected]>

* build(color-slider): use beta of 5.0 spectrum css component New version of Spectrum CSS colorslider that has migrated to using core tokens. * fix(color-slider): remove css that is now a duplicate Remove recently added CSS for reversing the gradient in RTL mode that is now included in the new spectrum CSS colorslider (same rule now exists in generated spectrum-color-slider.css). * build(color-slider): update beta version of CSS * feat(color-slider): use opacity-checkerboard Use the new opacity checkerboard tools class. * build(color-slider): update to 5.0 spectrum css release Update version of spectrum CSS color slider to the latest 5.0 release, just out of beta. * test: correct tests for layout updates * ci: update golden images cache --------- Co-authored-by: Westbrook Johnson <[email protected]> fix(number-field): update value on pressing enter chore(number-field): update value on pressing enter

fix(number-field): updated value update logic to call call renderUpdate in numberfield fix(number-field): blur the element on pressing enter fix(slider): updated test chore: fixed merge conflicts feat(color-slider): migrate to core tokens (#3507) * build(color-slider): use beta of 5.0 spectrum css component New version of Spectrum CSS colorslider that has migrated to using core tokens. * fix(color-slider): remove css that is now a duplicate Remove recently added CSS for reversing the gradient in RTL mode that is now included in the new spectrum CSS colorslider (same rule now exists in generated spectrum-color-slider.css). * build(color-slider): update beta version of CSS * feat(color-slider): use opacity-checkerboard Use the new opacity checkerboard tools class. * build(color-slider): update to 5.0 spectrum css release Update version of spectrum CSS color slider to the latest 5.0 release, just out of beta. * test: correct tests for layout updates * ci: update golden images cache --------- Co-authored-by: Westbrook Johnson <[email protected]> fix(number-field): update value on pressing enter chore(number-field): update value on pressing enter fix(number-field): request update on return

fix(number-field): update value on pressing enter fix(number-field): request update on return fix(number-field): updated value update logic to call call renderUpdate in numberfield fix(number-field): blur the element on pressing enter fix(slider): updated test chore: fixed merge conflicts feat(color-slider): migrate to core tokens (#3507) * build(color-slider): use beta of 5.0 spectrum css component New version of Spectrum CSS colorslider that has migrated to using core tokens. * fix(color-slider): remove css that is now a duplicate Remove recently added CSS for reversing the gradient in RTL mode that is now included in the new spectrum CSS colorslider (same rule now exists in generated spectrum-color-slider.css). * build(color-slider): update beta version of CSS * feat(color-slider): use opacity-checkerboard Use the new opacity checkerboard tools class. * build(color-slider): update to 5.0 spectrum css release Update version of spectrum CSS color slider to the latest 5.0 release, just out of beta. * test: correct tests for layout updates * ci: update golden images cache --------- Co-authored-by: Westbrook Johnson <[email protected]> fix(number-field): update value on pressing enter chore(number-field): update value on pressing enter fix(number-field): request update on return fix(number-field): update inputValue on clicking return

Westbrook mentioned this pull request Aug 9, 2023

feat(opacity-checkerboard): adding new component #3416

Merged

13 tasks

jawinn marked this pull request as ready for review August 28, 2023 19:57

jawinn added the ready-for-review label Sep 1, 2023

najikahalsema previously approved these changes Sep 1, 2023

View reviewed changes

Westbrook mentioned this pull request Sep 6, 2023

fix(color-handle,color-loupe,swatch,thumbnail): use the Opacity Checkerboard package #3625

Merged

10 tasks

Westbrook force-pushed the colorslider-core-tokens branch from 8f49e06 to c4ada35 Compare September 8, 2023 11:58

Westbrook added the Component: Color slider label Sep 8, 2023

Westbrook force-pushed the colorslider-core-tokens branch from c4ada35 to 0a2f032 Compare September 8, 2023 12:23

Westbrook dismissed najikahalsema’s stale review via c34a67c September 8, 2023 14:24

jawinn and others added 6 commits September 8, 2023 17:00

build(color-slider): use beta of 5.0 spectrum css component

471c621

New version of Spectrum CSS colorslider that has migrated to using core tokens.

fix(color-slider): remove css that is now a duplicate

1cbc35e

Remove recently added CSS for reversing the gradient in RTL mode that is now included in the new spectrum CSS colorslider (same rule now exists in generated spectrum-color-slider.css).

build(color-slider): update beta version of CSS

1818618

feat(color-slider): use opacity-checkerboard

38eed76

Use the new opacity checkerboard tools class.

build(color-slider): update to 5.0 spectrum css release

824a2f5

Update version of spectrum CSS color slider to the latest 5.0 release, just out of beta.

test: correct tests for layout updates

6872683

Westbrook force-pushed the colorslider-core-tokens branch from c34a67c to 6872683 Compare September 8, 2023 21:17

ci: update golden images cache

1afde5b

Westbrook approved these changes Sep 8, 2023

View reviewed changes

Westbrook merged commit 96d0d40 into main Sep 8, 2023

Westbrook deleted the colorslider-core-tokens branch September 8, 2023 21:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(color-slider): migrate to core tokens #3507

feat(color-slider): migrate to core tokens #3507

jawinn commented Aug 2, 2023 •

edited

Loading

github-actions bot commented Aug 2, 2023

github-actions bot commented Aug 2, 2023 •

edited

Loading

color-area permalink

color-handle permalink

color-slider permalink

color-wheel permalink

color-area permalink

color-handle permalink

color-slider permalink

color-wheel permalink

jawinn commented Aug 4, 2023 •

edited

Loading

jawinn commented Aug 7, 2023

jawinn commented Aug 8, 2023

Westbrook commented Aug 9, 2023

jawinn commented Aug 9, 2023

najikahalsema left a comment

Westbrook left a comment

feat(color-slider): migrate to core tokens #3507

feat(color-slider): migrate to core tokens #3507

Conversation

jawinn commented Aug 2, 2023 • edited Loading

Description

Related issue(s)

How has this been tested?

Types of changes

Checklist

Best practices

github-actions bot commented Aug 2, 2023

Branch Preview

github-actions bot commented Aug 2, 2023 • edited Loading

Tachometer results

color-area permalink

color-handle permalink

color-slider permalink

color-wheel permalink

color-area permalink

color-handle permalink

color-slider permalink

color-wheel permalink

jawinn commented Aug 4, 2023 • edited Loading

jawinn commented Aug 7, 2023

jawinn commented Aug 8, 2023

Westbrook commented Aug 9, 2023

jawinn commented Aug 9, 2023

najikahalsema left a comment

Choose a reason for hiding this comment

Westbrook left a comment

Choose a reason for hiding this comment

jawinn commented Aug 2, 2023 •

edited

Loading

github-actions bot commented Aug 2, 2023 •

edited

Loading

jawinn commented Aug 4, 2023 •

edited

Loading