-
Notifications
You must be signed in to change notification settings - Fork 201
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
🚀 Deployed on https://pr-1924--spectrum-css.netlify.app |
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
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.
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
jenndiaz
suggested changes
Jun 15, 2023
There was a problem hiding this 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
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.
jenndiaz
reviewed
Jun 16, 2023
Beta version released: |
14 tasks
The 'inset' of the box-shadow border had been lost from the skin.css, as noticed in VRTs.
15 tasks
New beta published: |
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.
New beta released: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
ColorSlider core tokens migration:
Updates the
colorslider
component to use core tokens as defined in the design.Other notable changes:
transform: scaleX
style rule can likely be removed once this merges in there.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