fix(actionbutton): fast follows by marissahuysentruyt · Pull Request #3540 · adobe/spectrum-css

marissahuysentruyt · 2025-02-06T17:40:13Z

Description

🐛
During foundations review (pre-merge), we noticed that action button had some possible bugs. Those possible bugs included:

Selected + Disabled possibly missing a background color token

the increased border-radius makes the corner triangle feel misplaced/too tight

the disabled variants are even lower contrast than before (not strictly required by a11y but still seems too low)

Through some investigation, it was found the the disabled background was referencing transparent-black-* tokens instead of disabled-background-color tokens for the disabled states. This PR also utilizes the disabled-static-*-background-color tokens for the static color action button's disabled state. Additionally, the corner rounding for this foundations action button component has been updated to use the S2 action button corner rounding specs to accommodate the hold/corner triangle icon better. (more thorough action button styling updates to match S2 specs will occur in the action button migration)

Jira/Specs/Notes

CSS-1118
S2 action button Figma tokens specs
S2 Foundations Review canvas (see the action button notes from Steph)

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

Regression testing

Additional validation

View the S1 context of action button.
Upon inspection, all action buttons in all sizes should retain their --spectrum-corner-radius-100 corner rounding.
View the Express context of action button.
Upon inspection, all action buttons in Disabled and Selected+Disabled states should retain --mod-actionbutton-background-color-default (which should be set to --spectrum-actionbutton-background-color-disabled that resolves to --spectrum-gray-200 as their background color.
Upon inspection, all action buttons in all sizes should retain their --spectrum-corner-radius-100 corner rounding as well.

Validate:

The documentation pages for at least two other components are still loading, including:

The pages render correctly, are accessible, and are responsive.

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. (action group, contextual help, date picker and pagination all had VRT diffs due to the new corner rounding for extra small or small action buttons in S2 context).

Screenshots

To-do list

I have read the contribution guidelines.
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.
✨ This pull request is ready to merge. ✨

changeset-bot · 2025-02-06T17:40:20Z

🦋 Changeset detected

Latest commit: 85df6e7

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/actionbutton	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 · 2025-02-06T17:43:17Z

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

github-actions · 2025-02-06T17:44:52Z

File metrics

Summary

Total size: 2.25 MB^*

Package	Size	Minified	Gzipped
actionbutton	25.07 KB	24.01 KB	3.02 KB

actionbutton

Filename	Head	Minified	Gzipped	Compared to base
index-base.css	20.30 KB	19.42 KB	2.71 KB	🔴 ⬆ 0.01 KB
index-theme.css	7.42 KB	7.21 KB	0.88 KB	🔴 ⬆ 1.31 KB
index.css	25.07 KB	24.01 KB	3.02 KB	🔴 ⬆ 1.14 KB
metadata.json	14.46 KB	-	-	🔴 ⬆ 1.07 KB
themes/express.css	5.51 KB	5.33 KB	0.86 KB	🔴 ⬆ 0.87 KB
themes/spectrum-two.css	5.90 KB	5.72 KB	0.90 KB	🔴 ⬆ 1.08 KB
themes/spectrum.css	5.67 KB	5.50 KB	0.88 KB	🔴 ⬆ 0.85 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.

5t3ph · 2025-02-07T17:01:22Z

Following up from our DM - since the disabled tokens match expectations, we can take further changes back to discuss with design re: contrast.

Border radius changes do seem to be coming through, and as you noted it helps particularly the Small and Extra Small variants.

Validated that a background now appears for Disabled + Selected, too.

rise-erpelding

Looks good overall! Let's just double-check the S1 standard/emphasized disabled background color.

rise-erpelding · 2025-02-10T19:49:49Z

components/actionbutton/themes/spectrum.css

 		--spectrum-actionbutton-background-color-down: var(--spectrum-gray-300);
 		--spectrum-actionbutton-background-color-focus: var(--spectrum-gray-200);
-		--spectrum-actionbutton-background-color-selected-disabled: var(--spectrum-transparent-black-200);
+		--spectrum-actionbutton-background-color-disabled: var(--spectrum-gray-75);


I'm wondering about this one... digging for the S1 spec and also looking at the last PR preview before Foundations was merged, it looks like we might want transparent for this instead? Unless there's a particular reason for this to be gray-75 instead?

It looks like the changes resolved the initial issue (missing background for "Disabled + Selected" in Dark theme), but maybe need a second pass to keep transparent the default for disabled, and the gray the default for selected-disabled and then only do variant overrides when it changes to simplify and keep the naming meaningful.

ohohoho good calls all around. I was thinking just to keep the S1 background color I saw on prod, but I see now that the S1 background color is actually incorrect in the first place. Sorry about the misunderstanding! I'll see if I can dig into these once more.

Ok I think I've the transparent background back for disabled S1 buttons. fb4cc7b Let me know if something still looks off!

components/actionbutton/themes/express.css

components/actionbutton/index.css

rise-erpelding

It looks good, just needs that comma back that you noted!

5t3ph

🎉

* fix(actionbutton): defines disabled background colors - defines --spectrum-actionbutton-background-color-disabled, --spectrum-actionbutton-background-color-selected-disabled custom properties to use disabled-background-color and disabled-static-*- background-color is those states - updates metadata.json - S1 actionbutton disabled background color should not have been altered - preserve express disabled `gray-200` background color, and updates background in selected-disabled dark mode (to use disabled-background- color) * fix(actionbutton): transparent background for S1 disabled state * chore(actionbutton): add changeset

marissahuysentruyt self-assigned this Feb 6, 2025

marissahuysentruyt added bug Results from a bug in the CSS implementation run_vrt For use on PRs looking to kick off VRT labels Feb 6, 2025

marissahuysentruyt force-pushed the marissahuysentruyt/fix-action-button-fast-follows branch 4 times, most recently from 075a25e to 397d8d2 Compare February 10, 2025 17:05

marissahuysentruyt requested review from castastrophe and rise-erpelding February 10, 2025 17:05

marissahuysentruyt marked this pull request as ready for review February 10, 2025 17:05

marissahuysentruyt requested a review from 5t3ph February 10, 2025 17:18

marissahuysentruyt added ready-for-review size-2 S ~6-18hrs; not hard or time consuming, one or two work days to complete. labels Feb 10, 2025

rise-erpelding reviewed Feb 10, 2025

View reviewed changes

marissahuysentruyt force-pushed the marissahuysentruyt/fix-action-button-fast-follows branch from 397d8d2 to fb4cc7b Compare February 11, 2025 22:02

marissahuysentruyt commented Feb 11, 2025

View reviewed changes

components/actionbutton/index.css Outdated Show resolved Hide resolved

marissahuysentruyt requested a review from rise-erpelding February 11, 2025 22:18

rise-erpelding approved these changes Feb 12, 2025

View reviewed changes

marissahuysentruyt force-pushed the marissahuysentruyt/fix-action-button-fast-follows branch from fb4cc7b to 0dde443 Compare February 12, 2025 14:52

5t3ph approved these changes Feb 12, 2025

View reviewed changes

marissahuysentruyt force-pushed the marissahuysentruyt/fix-action-button-fast-follows branch from 0dde443 to 19546f1 Compare February 12, 2025 15:18

marissahuysentruyt added ready-to-merge and removed run_vrt For use on PRs looking to kick off VRT ready-for-review labels Feb 12, 2025

marissahuysentruyt force-pushed the marissahuysentruyt/fix-action-button-fast-follows branch from 19546f1 to 85df6e7 Compare February 13, 2025 14:04

marissahuysentruyt merged commit a8bb0b8 into main Feb 13, 2025
11 checks passed

marissahuysentruyt deleted the marissahuysentruyt/fix-action-button-fast-follows branch February 13, 2025 14:30

github-actions bot mentioned this pull request Feb 13, 2025

chore: release #3552

Merged

marissahuysentruyt mentioned this pull request Mar 27, 2025

fix(actionbutton): reimplement support for border-radius mod #3644

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(actionbutton): fast follows#3540

fix(actionbutton): fast follows#3540
marissahuysentruyt merged 1 commit intomainfrom
marissahuysentruyt/fix-action-button-fast-follows

marissahuysentruyt commented Feb 6, 2025 •

edited by rise-erpelding

Loading

Uh oh!

changeset-bot bot commented Feb 6, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Feb 6, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Feb 6, 2025 •

edited

Loading

Uh oh!

5t3ph commented Feb 7, 2025

Uh oh!

rise-erpelding left a comment

Uh oh!

rise-erpelding Feb 10, 2025

Uh oh!

5t3ph Feb 10, 2025

Uh oh!

marissahuysentruyt Feb 11, 2025

Uh oh!

marissahuysentruyt Feb 11, 2025

Uh oh!

Uh oh!

Uh oh!

rise-erpelding left a comment

Uh oh!

5t3ph left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

marissahuysentruyt commented Feb 6, 2025 • edited by rise-erpelding Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Jira/Specs/Notes

How and where has this been tested?

Validation steps

Regression testing

Additional validation

Screenshots

To-do list

Uh oh!

changeset-bot bot commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

File metrics

Summary

actionbutton

Uh oh!

5t3ph commented Feb 7, 2025

Uh oh!

rise-erpelding left a comment

Choose a reason for hiding this comment

Uh oh!

rise-erpelding Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

5t3ph Feb 10, 2025

Choose a reason for hiding this comment

Uh oh!

marissahuysentruyt Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

marissahuysentruyt Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rise-erpelding left a comment

Choose a reason for hiding this comment

Uh oh!

5t3ph left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marissahuysentruyt commented Feb 6, 2025 •

edited by rise-erpelding

Loading

changeset-bot bot commented Feb 6, 2025 •

edited

Loading

github-actions bot commented Feb 6, 2025 •

edited

Loading

github-actions bot commented Feb 6, 2025 •

edited

Loading