Skip to content

refactor(colorslider)!: core tokens migration #1924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 39 commits into from
Aug 25, 2023

Conversation

jawinn
Copy link
Collaborator

@jawinn jawinn commented Jun 5, 2023

Description

ColorSlider core tokens migration:
Updates the colorslider component to use core tokens as defined in the design.

Other notable changes:

  • Updates default border-color in forced-colors (high contrast) mode to use CanvasText, so border remains visible when using a dark high contrast theme (the default).
  • Updates Storybook to include an option for the gradient color stops, and creates another story using the "Alpha" variant from the example docs, which shows the checkerboard background. This should be useful for future tracking of visual regressions.
  • Handles flipping the gradient when using RTL base direction. A similar change was made on the SWC side and that transform: scaleX style rule can likely be removed once this merges in there.
  • Disabled no longer shows as 2px larger than enabled

ColorHandle:
Includes minor adjustment to styles to add new mod --mod-colorhandle-hitarea-border-radius.
ColorHandle change has been cherry-picked into its own PR #2065 . This update to ColorSlider will require a minimum version of ColorHandle that includes that update when it merges.

OpacityCheckerboard:
The OpacityCheckerboard story template is modified, so it can accept content as an arg and exclude the wrapper, so that it can be used in the ColorSlider story, and other components' stories. Also makes some other minor adjustments and adds a story.
These changes were also cherry-picked and included in an update that has merged into main ( #2056 ).

CSS-188

How and where has this been tested?

Chrome, Firefox, and Safari on Mac. And WHCM Edge through AssistivLabs.

To-do list

  • 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.
  • I have read the CONTRIBUTING document.
  • I have tested these changes in Windows High Contrast mode.
  • I have updated any relevant storybook stories and templates.
  • If my change(s) include visual change(s), a designer has reviewed and approved those changes.
  • This pull request is ready to merge.

jawinn added 9 commits May 19, 2023 12:34
Migrate dependencies and other build related files to use the new
@spectrum-css/tokens package.
- Add new tokens defined in design.
- Use tokens in existing CSS.
- Merge skin.css into index.css.
Add story that shows the existing variant from the docs, with a
transparent color used in the gradient and checkerboard appearing behind
it. Adds a hidden control to Storybook. This variant needed to be
testable and compared against in Storybook.
Removes unnecessary placeholder variable (%) that is only used once.
Clarifies what these "hidden" styles are doing.
- Finalize CSS to use new core tokens.
- Integrates skin.css into index.css.
- Remove "canvas" variant as was done with colorwheel component.
- Replaces some properties with their logical properties.
- Generate mods.
Run prettier with automatic changes to colorslider files, to update
formatting to match new changes to linting.

And manually make a few changes after stylelint check.
Updates colors for high contrast to fix disabled border color being off.
Refactors those colors and uses a local variable for the box-shadow
color.

Change: Uses CanvasText border color in high contrast mode, so
component still shows a border in high contrast mode with a dark
background theme. Border was previously barely visible.
@jawinn jawinn added breaking change Results in a breaking API change wip This is a work in progress, don't judge. labels Jun 5, 2023
@jawinn jawinn marked this pull request as draft June 5, 2023 16:48
@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2023

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

@github-actions github-actions bot temporarily deployed to pull request June 5, 2023 16:57 Inactive
These styles for the nested ColorHandle appear to be no longer
necessary.

- hit area size (width and height) is available through colorhandle's
  --mod property. The global-dimension-size-300 was equal to the same
  pixel value already set by colorhandle using
  --spectrum-color-control-track-width

- border-radius is set to 100% in colorhandle and there does not appear
  to be any option to support changing this through a mod or SWC options
@github-actions github-actions bot temporarily deployed to pull request June 9, 2023 21:17 Inactive
jawinn added 2 commits June 14, 2023 15:30
Per discussion with design, the label on the design was meant to serve
more as guidance, and does not need to be a part of the component.
@github-actions github-actions bot temporarily deployed to pull request June 14, 2023 19:46 Inactive
@jawinn jawinn marked this pull request as ready for review June 15, 2023 13:57
jawinn added 2 commits June 15, 2023 10:55
Add --mod custom property for the border radius of the hit area, so that
other components that use colorhandle can easily customize it
(colorslider sets it to square, not rounded).
Keep the square hit area (::after) on the nested colorhandle, which was
defaulting to rounded, using the newly added passthrough for
--mod-colorhandle-hitarea-border-radius
@github-actions github-actions bot temporarily deployed to pull request June 15, 2023 16:00 Inactive
Copy link
Contributor

@jenndiaz jenndiaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 🎨

Had a couple of questions/comments

jawinn added 3 commits June 16, 2023 11:50
Use same technique as SWC for reversing the gradient when using a RTL
base direction. The handles were flipping to the other side using the
new logical properties but the gradient was not.

Also updates documentation around this and improves overall docs
explanations. Changes first usage docs bullet point specifying
translateX, as this does not appear to be correct (SWC changes the
inset-inline-start and inset-block-end of the position absolute handle).

Fixes capitalization of component name in a few places to match the
sentence case specified in the Spectrum capitalization guidelines.
Update gradient used and use the matching colorhandle background color
for the new Alpha story.

Add new story for the "With Image" example from the docs. This example
also includes the 50% positioned handle, for testing/VRT purposes.
@github-actions github-actions bot temporarily deployed to pull request June 16, 2023 20:36 Inactive
@github-actions github-actions bot temporarily deployed to pull request June 20, 2023 12:40 Inactive
@pfulton
Copy link
Collaborator

pfulton commented Aug 1, 2023

Beta version released: @spectrum-css/colorslider@5.0.0-beta.0

@github-actions github-actions bot temporarily deployed to pull request August 1, 2023 21:10 Inactive
The 'inset' of the box-shadow border had been lost from the skin.css, as
noticed in VRTs.
@github-actions github-actions bot temporarily deployed to pull request August 2, 2023 21:23 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 3, 2023 14:31 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 3, 2023 20:40 Inactive
@pfulton
Copy link
Collaborator

pfulton commented Aug 3, 2023

New beta published: @spectrum-css/colorslider@5.0.0-beta.1

@github-actions github-actions bot temporarily deployed to pull request August 3, 2023 20:51 Inactive
jawinn added 3 commits August 7, 2023 10:49
Increase minimum version of colorhandle to be the latest release that
includes the addition of --mod-colorhandle-hitarea-border-radius
Update devDependencies to use latest version of tokens package.
@github-actions github-actions bot temporarily deployed to pull request August 7, 2023 15:02 Inactive
@github-actions github-actions bot temporarily deployed to pull request August 8, 2023 20:35 Inactive
@pfulton
Copy link
Collaborator

pfulton commented Aug 9, 2023

New beta released: @spectrum-css/colorslider@5.0.0-beta.2

@github-actions github-actions bot temporarily deployed to pull request August 9, 2023 13:54 Inactive
@pfulton pfulton added the run_vrt For use on PRs looking to kick off VRT label Aug 25, 2023
@github-actions github-actions bot temporarily deployed to pull request August 25, 2023 14:58 Inactive
@github-actions github-actions bot removed the run_vrt For use on PRs looking to kick off VRT label Aug 25, 2023
@pfulton pfulton merged commit da345bd into main Aug 25, 2023
@pfulton pfulton deleted the jawinn/css-188-color-slider-token-migration branch August 25, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Results in a breaking API change wip This is a work in progress, don't judge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants