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.

Sign up for GitHub

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

Jump to bottom

fix(button): primary, secondary color fast follows #3648

Merged
merged 2 commits into from
Apr 3, 2025
Merged

fix(button): primary, secondary color fast follows #3648

merged 2 commits into from
Apr 3, 2025

Conversation

rise-erpelding
Copy link
Collaborator

@rise-erpelding rise-erpelding commented Mar 31, 2025 •
edited by aramos-adobe
Loading

Description

S2 Foundations fixes to adjust:

  • primary outline background color for hover, active, and focus states (gray-100)
  • secondary fill background color for active state (gray-200)
  • secondary outline background color for hover, active, and focus states (gray-100)
  • secondary outline border color for default (gray-300) and active states (gray-400)

Given that previous fixes were made to requests for changes in static-color variants in #3613 and are now being reflected in the corresponding SWC PR, this should bring button colors fully up to date with the S2 button spec.

SWC-496
SDS-14605

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

Open the testing preview for the button component and confirm that the colors for primary and secondary button variants match the S2 spec. Doing so should cover the feedback and requests made in Jira. You are welcome to do this on your own or use the checklist below!

Confirming that changes made here are taking effect is the priority (the specific requests from the Jira ticket are noted in bold below), but to reduce chances of additional changes needing to be made to this component, I would appreciate verification of all border and background colors for the following variants, not just changed colors (I'm including the rgb values that these colors should compute to in light mode only to make this easier):

  1. Primary fill
    • Default background should be neutral-background-color-default/gray-800 (41, 41, 41 in light mode) (@aramos-adobe)
    • Hover, focus, down/active background should be neutral-background-color/hover/neutral-background-color-key-focus/neutral-background-color-down/gray-900 (19, 19, 19 in light mode) (@aramos-adobe )
  2. Primary outline
    • Default background should be transparent (@aramos-adobe)
    • Hover, focus, down/active background should be gray-100 (233, 233, 233 in light mode) - this was specifically noted in the design feedback and changed here (@aramos-adobe)
    • Default border should be gray-800 (41, 41, 41 in light mode) (@aramos-adobe )
    • Hover, focus, down/active border should be gray-900 (19, 19, 19 in light mode) (@aramos-adobe )
  3. Secondary fill
    • Default background should be gray-100 (233, 233, 233 in light mode) - this was specifically noted in design feedback, but appeared to already be taking effect in CSS implementation (@aramos-adobe )
    • Hover, focus, down/active background should be gray-200 (225, 225, 225 in light mode) - this was specifically noted in design feedback and changed here (@aramos-adobe )
  4. Secondary outline
    • Default background should be transparent (@aramos-adobe )
    • Hover, focus, down/active background should be gray-100 (233, 233, 233 in light mode) - this was specifically noted in design feedback and changed here (@aramos-adobe )
    • Default border should be gray-300 (218, 218, 218 in light mode) - in the spec, [confirmed](https://adobedesign.slack.com/archives/C06Q3G5HJUX/p1743519542859989? thread_ts=1741610886.735469&cid=C06Q3G5HJUX) to be the correct value by the designer, changed here (@aramos-adobe )
    • Hover, focus, down/active border should be gray-400 (198, 198, 198 in light mode) - this was specifically noted in design feedback and changed here (@aramos-adobe )
  5. Confirm that there are no regressions introduced
    • Compare S1 to main to confirm that there are no changes, especially with active state (@aramos-adobe )

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. (@rise-erpelding)
  • 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 tested these changes in Windows High Contrast mode.
  • I have run VRTs
  • ✨ This pull request is ready to merge. ✨

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Mar 31, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: db38a56

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/button Patch

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 GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 31, 2025 •
edited
Loading

File metrics

Summary

Total size: 2.24 MB*

Package Size Minified Gzipped
button 38.43 KB 36.57 KB 4.19 KB

button

Filename Head Minified Gzipped Compared to base
index-base.css 30.26 KB 28.75 KB 3.75 KB 🟢 ⬇ 0.06 KB
index-theme.css 12.39 KB 12.01 KB 1.14 KB 🔴 ⬆ 0.09 KB
index.css 38.43 KB 36.57 KB 4.19 KB 🔴 ⬆ < 0.01 KB
metadata.json 20.23 KB - - 🔴 ⬆ 0.06 KB
themes/express.css 9.91 KB 9.61 KB 1.07 KB 🔴 ⬆ 0.08 KB
themes/spectrum-two.css 9.98 KB 9.68 KB 1.07 KB 🔴 ⬆ 0.08 KB
themes/spectrum.css 9.99 KB 9.69 KB 1.09 KB 🔴 ⬆ 0.08 KB
* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 31, 2025 •
edited
Loading

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

@rise-erpelding rise-erpelding added ask design Questions or topics for the design team wip This is a work in progress, don't judge. skip_vrt Add to a PR to skip running VRT (but still pass the action) fast-follows An issue or PR that quickly follows a release size-2 S ~6-18hrs; not hard or time consuming, one or two work days to complete. ready-for-review and removed ask design Questions or topics for the design team labels Mar 31, 2025
@rise-erpelding rise-erpelding marked this pull request as ready for review April 1, 2025 16:23
@rise-erpelding rise-erpelding removed the wip This is a work in progress, don't judge. label Apr 1, 2025
castastrophe
castastrophe requested changes Apr 1, 2025
View reviewed changes
Copy link
Collaborator

@castastrophe castastrophe left a comment

Choose a reason for hiding this comment

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

One question about the S1 fallback for the new border down color/state but otherwise looks good.

@rise-erpelding
fix(button): primary, secondary color fast follows
5d7925c 
Makes adjustments for:
primary outline bg color hover down focus
secondary fill bg color active
secondary outline bg color hover, focus, down
secondary outline border color default, down
@rise-erpelding rise-erpelding force-pushed the rise-erpelding/button-ff-pt3 branch from 0f2a6cd to 8bca288 Compare April 1, 2025 18:16
@rise-erpelding rise-erpelding force-pushed the rise-erpelding/button-ff-pt3 branch from 8bca288 to db38a56 Compare April 1, 2025 18:42
@castastrophe castastrophe added run_vrt For use on PRs looking to kick off VRT and removed skip_vrt Add to a PR to skip running VRT (but still pass the action) labels Apr 2, 2025
@aramos-adobe aramos-adobe self-requested a review April 2, 2025 21:42
@rise-erpelding rise-erpelding merged commit 8e52975 into main Apr 3, 2025
34 checks passed
@rise-erpelding rise-erpelding deleted the rise-erpelding/button-ff-pt3 branch April 3, 2025 00:48
@github-actions github-actions bot mentioned this pull request Apr 3, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@castastrophe castastrophe castastrophe approved these changes

@aramos-adobe aramos-adobe aramos-adobe approved these changes

Assignees
No one assigned
Labels
fast-follows An issue or PR that quickly follows a release ready-for-review run_vrt For use on PRs looking to kick off VRT size-2 S ~6-18hrs; not hard or time consuming, one or two work days to complete.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rise-erpelding @castastrophe @aramos-adobe