Skip to content
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

feat(progressbar): s2 migration #2659

Merged
merged 1 commit into from May 8, 2024
Merged

Conversation

mdt2
Copy link
Collaborator

@mdt2 mdt2 commented Apr 12, 2024

Description

Migrates Progress Bar to Spectrum 2 (design spec). Includes:

  • Updated font sizes
  • Updated colors
  • WHCM improvement for Static White variant
  • Increased Chromatic coverage
  • Consolidation of Storybook stories (only Default now and users can use controls to see other variants/states)
  • No mods were changed

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

@jawinn

  • Variants are accessible by controls in Storybook (especially check Static White)
  • Check Chromatic testing preview in Storybook
  • design review @mdt2 (Slack approval)

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

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 documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

@@ -53,7 +53,7 @@ export const withContextWrapper = makeDecorator({
// Change background color when demonstrating static color options.
if (args.staticColor === "black") {
container.style.background = "var(--spectrum-examples-gradient-static-black)";
} else if (args.staticColor === "white") {
} else if (args.staticColor === "white" || args.isStaticWhite === "true") {
Copy link
Collaborator Author

@mdt2 mdt2 Apr 12, 2024

Choose a reason for hiding this comment

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

In the case of Progress Bar, only Static White is supported. I didn't want to make folks use a dropdown in the Storybook controls when there's only one option, so I expanded this to accommodate for isStaticWhite as a boolean option.

Copy link
Contributor

github-actions bot commented Apr 12, 2024

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

Copy link
Contributor

github-actions bot commented Apr 12, 2024

File metrics

Summary

Total size: 4.32 MB*
Total change (Δ): ⬇ 0.07 KB (-0.00%)

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

Package Size Δ
progressbar 11.19 KB ⬇ 0.05 KB

Details

progressbar

File Head Base Δ
index-base.css 11.19 KB 11.24 KB ⬇ 0.05 KB (-0.45%)
index-vars.css 11.19 KB 11.24 KB ⬇ 0.05 KB (-0.45%)
index.css 11.19 KB 11.24 KB ⬇ 0.05 KB (-0.45%)
metadata.json 5.95 KB 5.86 KB ⬆ 0.08 KB (1.40%)
* 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.

Copy link
Collaborator

@jawinn jawinn left a comment

Choose a reason for hiding this comment

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

I tried out all the various options, static white, and high contrast mode, and this is looking great. I see you brought up the side label question with design already.

components/progressbar/stories/progressbar.stories.js Outdated Show resolved Hide resolved
components/progressbar/index.css Outdated Show resolved Hide resolved
Copy link
Collaborator

@rise-erpelding rise-erpelding left a comment

Choose a reason for hiding this comment

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

This is looking good! The testing preview shows all the variants (including Static White), which match the design spec, which I can also access through the Storybook controls, and WHCM looks good as well!

@mdt2 mdt2 force-pushed the mdt2/css-711-s2-progress-bar branch from 6b74834 to 55fb881 Compare April 16, 2024 14:31
@pfulton pfulton force-pushed the spectrum-two branch 2 times, most recently from f3dd9ff to 9931a8e Compare April 19, 2024 18:22
@castastrophe castastrophe force-pushed the spectrum-two branch 5 times, most recently from 6dbc850 to ae7aedd Compare April 26, 2024 20:37
@mdt2 mdt2 force-pushed the mdt2/css-711-s2-progress-bar branch from b037ee9 to 92b9f11 Compare April 30, 2024 14:32
Copy link

changeset-bot bot commented Apr 30, 2024

🦋 Changeset detected

Latest commit: 5f582df

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/progressbar Major

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

@mdt2 mdt2 force-pushed the mdt2/css-711-s2-progress-bar branch 2 times, most recently from 7d5bcfb to 53917df Compare May 2, 2024 20:10
@mdt2 mdt2 force-pushed the mdt2/css-711-s2-progress-bar branch from 53917df to 5f582df Compare May 7, 2024 15:28
@pfulton pfulton added the run_vrt For use on PRs looking to kick off VRT label May 8, 2024
@pfulton pfulton merged commit bd38eaa into spectrum-two May 8, 2024
22 checks passed
@pfulton pfulton deleted the mdt2/css-711-s2-progress-bar branch May 8, 2024 20:43
@github-actions github-actions bot mentioned this pull request May 8, 2024
pfulton pushed a commit that referenced this pull request May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run_vrt For use on PRs looking to kick off VRT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants