feat(colorslider): S2 migration

[cdransf]

Description

CSS-1028

S2 colorslider migration

This migrates the colorslider component to S2. Custom properties have been remapped per the design spec.

Before	After
--spectrum-gray-900-rgb	--spectrum-gray-1000-rgb

Validation steps

1. Open the URL for the PR.
2. Navigate to the colorslider component and verify no regressions have occurred.

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.

2. 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

• 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 other components, I have tested to make sure they don't break.
• If my change impacts documentation, I have updated the documentation accordingly.
• ✨ This pull request is ready to merge. ✨

[changeset-bot]

🦋 Changeset detected

Latest commit: 4fd06b6

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/colorslider	Minor

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]

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

[github-actions]

File metrics

Summary

Total size: 2.23 MB^*
Total change (Δ): 🔴 ⬆ 0.28 KB (0.01%)

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

Package	Size	Δ
colorslider	4.08 KB	🔴 ⬆ 0.18 KB

Details

colorslider

Filename	Head	Compared to base
index.css	4.08 KB	🔴 ⬆ 0.18 KB (4.53%)
metadata.json	2.12 KB	🔴 ⬆ 0.10 KB (4.99%)

* 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.

[cdransf]

Per design we can ignore the spacing tokens as they're not required for the implementation (alignment of the handle within the color slide is handled by this rule:

.spectrum-ColorSlider-handle {
  inset-inline-start: 0;
  inset-block-start: 50%;
}

[rise-erpelding]

Migration-wise, this looks fine, since not a lot needed to change here! I have a few other recommendations though:

CSS
I have a few recommendations for the CSS that would make this file more consistent with some of the other migration work that's been done:

• Generally, we've been moving forced-colors media query blocks to the bottom in recent migrations (see feat(colorarea): S2 migration #3388, feat(tooltip): S2 migration #2743, or feat(statuslight): s2 migration #2818 for instance), and I see that Color Slider's is still further up in the CSS file, that could probably be moved down.
• We have this line:

  block-size: var(--mod-color-slider-control-track-width, var(--spectrum-color-control-track-width));

  I'm really not sure why that color-control-track-width token has that name, but it seems like a good idea to create an alias custom property for this, especially given that its token name doesn't match its mod name, something like this, we can even take the word "control" out and change the mod name completely, up to you:

   /* Up at the topmost .spectrum-ColorSlider block */
   --spectrum-color-slider-control-track-width: var(--spectrum-color-control-track-width);
  
  /* further down in the code, same place it is now */
  block-size: var(--mod-color-slider-control-track-width, var(--spectrum-color-slider-control-track-width));

• In that same vein, we could also alias the global token for the disabled background color that we have here, similar to how it's done in Color Wheel, Picker, or Search, for instance:

  --spectrum-color-slider-border-color-local: var(--highcontrast-color-slider-border-color-disabled, var(--mod-color-slider-border-color-disabled, var(--spectrum-disabled-background-color)));
  background: var(--highcontrast-color-slider-background-color-disabled, var(--mod-color-slider-background-color-disabled, var(--spectrum-disabled-background-color)));

• There's also an unused token for the spacing between the field label and the color slider, but this isn't new. I don't know if there's some specific reason we're not adding the label to this implementation (Slider has a label that we display), but it feels like something we ought to be doing. Feel free to get more opinions on this one.

Storybook
Otherwise, this component could probably benefit from a few Storybook enhancements similar to what we added for several of our other components back when we were doing docs to storybook migrations (this component got migrated early on, before we decided that we would be doing that kind of thing):

• Being able to change the colors in the slider from the Storybook controls or add an image (Swatch on main added this with the docs to storybook migration docs(swatch,swatchgroup,table,tabs): docs migrations to storybook #2925, so we might be able to copy that over)
• A control to be able to view the vertical variant from the default story in Storybook. In this branch, all the variants are showing, but on main, on the default variant is visible and the others are used just for display purposes on the Docs page, that's probably where we'd like to end up one day when spectrum-two has some of those docs changes from main.

Last thing! I can't use the keyboard to focus on the slider handle (I can in main, but not on this branch or spectrum-two), this may not be a big deal, but I'm curious about why it's happening.

[jawinn]

I'm still seeing a few issues with the Alpha and Vertical stories being displayed on the Docs.

This'll also need some rebasing after the rebase in spectrum-two that recently happened, which is causing the large number of changed files to be shown. It may be easier to cherry-pick your commits than to handle the conflicts (at least that's what I found in my other branch).

[jawinn]

Alpha

It looks like the Alpha story is still using the gradientStops arg that renamed, so it's not displaying correctly:

Prod:

Vertical

The Vertical story also still needs an update for its story in our docs, and the VRT tests file config. The gradient should be in the to bottom direction for these instances where we set the arg value, though we talked about not needing any complicated logic if someone is manually flipping the vertical control on the Default story:

[cdransf]

Got it! I renamed and updated that arg and updated the gradient direction in the tests you highlighted. ✨

[rise-erpelding]

A couple of other details on the docs page that look like they still need some attention:

• This Alpha story doesn't look the same as in Foundations or main, it looks like the gradient in the args is using the same rainbow gradient rather than the previous one that included rgba values.
  
• The Vertical variant is improved! But it looks like it needs its handle adjusted, the color doesn't match the color next to it in the gradient and it's off-center. Previous versions showed the handle at the top which probably isn't strictly necessary, but it seems like maybe that's where the red color is coming from. Feels like a bit of a silly ask since it's not a working implementation, but would help it look the part anyway!
  

[cdransf]

A couple of other details on the docs page that look like they still need some attention:

• This Alpha story doesn't look the same as in Foundations or main, it looks like the gradient in the args is using the same rainbow gradient rather than the previous one that included rgba values.

Totally missed that! Thank you! That should be all fixed (updated with the value from the redux branch). ✨

• The Vertical variant is improved! But it looks like it needs its handle adjusted, the color doesn't match the color next to it in the gradient and it's off-center. Previous versions showed the handle at the top which probably isn't strictly necessary, but it seems like maybe that's where the red color is coming from. Feels like a bit of a silly ask since it's not a working implementation, but would help it look the part anyway!

I adjusted the inset-block-start for the .spectrum-ColorSlider-handle when the slider is oriented vertically. The default class hasn't been updated in ~2-5 years.

.spectrum-ColorSlider-handle {
  inset-inline-start: 0;
  inset-block-start: 50%;
}

Which is why we're seeing the handle 50% of the way through the slider. I added:

.spectrum-ColorSlider--vertical {
  ...
  .spectrum-ColorSlider-handle {
    inset-block-start: 0;
  }
}

Which I'm not sure is the right path — would love some feedback on that. 😄