fix(checkbox): corrects invalid+checked+hover background color #3617

marissahuysentruyt · 2025-03-12T14:33:31Z

Description

This PR addresses the regression we noticed in the invalid checkboxes, where the default hover color was "winning" out over the invalid hover color. To avoid this bug in the future, #3625 adds several tests and more Storybook support for hovers through the shared type control.

This PR includes adding a ::before selector in the is-invalid hover state.

It should be noted that there is more opportunity to refactor here. I noticed that the emphasized+invalid+checked+hovered state was behaving as expected. If that selector block (marked with a TODO in commit ea8741a) gets consolidated or moved into the default .spectrum-Checkbox, it seemed like it could safely be removed and all the styles would cascade as expected. The first commit in this PR reorganizes the emphasized invalid+checked+hovered css to be included with the default invalid+checked+hovered css, but that felt too repetitive. So instead, adding a ::before pseudo element appropriately targeted the invalid hovered checkboxes. Both approaches unfortunately led to selector changes.

Jira/Specs

CSS-1139

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

Pull down the branch to run locally or visit the deploy preview. [@cdransf]
Visit the checkbox docs page. [@cdransf]
Inspect the invalid set of checkboxes in the default story canvas. [@cdransf]
Verify that when a .spectrum-Checkbox element is hovered, the .spectrum-Checkbox-box::before pseudo element has a border color of --spectrum-checkbox-invalid-color-hover: var(--spectrum-negative-color-1000);. This applies to all 3 states (unchecked, checked, indeterminate). [@cdransf]
Repeat the steps above with the invalid set of checkboxes in the emphasized styling to verify no regressions have occurred. [@cdransf]
Confirm that the changes seen in the S2 foundations theme have been corrected in S1 and Express themes. [@cdransf]

Regression testing

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.

Screenshots

Before 🚫

After ✅

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 other components, I have tested to make sure they don't break.
✨ This pull request is ready to merge. ✨

changeset-bot · 2025-03-12T14:33:35Z

🦋 Changeset detected

Latest commit: b39b168

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/checkbox	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-03-12T14:39:38Z

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

github-actions · 2025-03-12T14:44:38Z

File metrics

Summary

Total size: 2.25 MB^*

Package	Size	Minified	Gzipped
checkbox	22.43 KB	21.36 KB	2.56 KB

checkbox

Filename	Head	Minified	Gzipped	Compared to base
index-base.css	22.04 KB	20.98 KB	2.51 KB	🔴 ⬆ 0.01 KB
index-theme.css	1.14 KB	1.12 KB	0.51 KB	-
index.css	22.43 KB	21.36 KB	2.56 KB	🔴 ⬆ 0.01 KB
metadata.json	12.70 KB	-	-	🔴 ⬆ 0.01 KB
themes/express.css	1.04 KB	1.01 KB	0.51 KB	-
themes/spectrum-two.css	1.01 KB	1.01 KB	0.52 KB	-
themes/spectrum.css	1.04 KB	1.01 KB	0.52 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.

marissahuysentruyt · 2025-03-12T16:43:18Z

components/checkbox/index.css

@@ -280,6 +282,7 @@
 		}
 	}

+	/* TODO: Because this selector was moved to the default variant's styles, this selector block can be deleted when it is safe to make changes to selectors. */


This is a note that we could refactor in the future. I experimented with copying this selector into the default .spectrum-Checkbox block, around line 168, and commented out this selector at line 287 nested within the --emphasized variant. The styles seemed to cascade correctly in both variants.

I thought that was too much repetition WITH new selectors, so I opted instead for just reorganizing the selector in the default block.

marissahuysentruyt · 2025-03-12T17:08:40Z

components/checkbox/index.css

@@ -158,7 +158,7 @@
 		}

 		&:hover {
-			.spectrum-Checkbox-input:checked + .spectrum-Checkbox-box,
+			.spectrum-Checkbox-input:checked + .spectrum-Checkbox-box::before,


I think the reason these styles weren't working is because we need the .spectrum-Checkbox-input:checked + .spectrum-Checkbox-box::before selector.

As I was looking through the other hover styles, trying to understand why they were working but this one wasn't, it seems like every other hover instance is referencing the entire .spectrum-Checkbox-input:checked + .spectrum-Checkbox-box::before selector. Am I missing anything?

I'm not sure I can correct this bug without it, but I'm very open to other ideas and happy to brainstorm!

marissahuysentruyt · 2025-03-12T17:27:51Z

TODO: run VRTs

jawinn · 2025-03-14T15:38:04Z

What do you think about separating the bug fixes out into their own PR(s)? This seems to be combining a couple distinct changes into one. Would adding/merging just the test coverage additions first and getting a VRT baseline help with validating the bug fixes afterwards?

components/checkbox/dist/metadata.json

cdransf

Ran through all the validation steps and everything looks good! ✨

- add missing ::before pseudo to target the before element in the invalid/checked/hover state - update metadata.json - create changeset

* feat(actionbutton): use s2 colors in spectrum-two theme (#3606) * feat(actionbutton): use closer to s2 colors in spectrum-two theme Requested colors update for action button, aligning the colors closer to the S2 design, post-foundations. In the foundations spectrum-two theme: - Removes the border - Changes the default background colors to match s2 specs - Updates the background colors used for static black and static white SWC-497 * fix(actionbutton): fix high contrast styles for selected disabled The selected + disabled button was not showing up as the disabled colors in high contrast mode. Fixed by adjusting the source order slightly in the high contrast media query so disabled is after selected and takes precedence. * fix(search): update disabled state in spectrum two (#3593) Co-authored-by: rise-erpelding <[email protected]> Co-authored-by: [ Cassondra ] <[email protected]> * chore(deps): bump the npm_and_yarn group with 2 updates (#3618) Bumps the npm_and_yarn group with 2 updates: [@babel/helpers](https://github.com/babel/babel/tree/HEAD/packages/babel-helpers) and [@babel/runtime](https://github.com/babel/babel/tree/HEAD/packages/babel-runtime). Updates `@babel/helpers` from 7.26.0 to 7.26.10 - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.26.10/packages/babel-helpers) Updates `@babel/runtime` from 7.24.4 to 7.26.10 - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.26.10/packages/babel-runtime) --- updated-dependencies: - dependency-name: "@babel/helpers" dependency-type: indirect dependency-group: npm_and_yarn - dependency-name: "@babel/runtime" dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: update release script install settings * fix(button): adjust s2 static colors [SWC-496] (#3613) * chore: release (#3619) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix(slider): corrects contrast bug caused by template default arg (#3614) * fix(stepper): fast follows for focus/focus+hover/keyboardFocus borders (#3621) * fix(stepper): focus/focus+hover/keyboardFocus border colors * chore(stepper): add changeset * fix(slider): offset variant border radius bug fix (#3611) * feat(slider): offset variant border radius bug fix * feat(slider): fix range slider * fix(combobox): border color fast follows swc-582 (#3609) * fix(combobox): correct s1/legacy container variable * fix(combobox): fast follow border color remapping - add mods for s2 foundations disabled picker button BG/border colors - correct disabled+read-only border color - add read-only border custom property - fixes express read-only border from gray-500 to gray-400 - update metadata.json * chore(combobox): create changeset * chore: release (#3623) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * chore: patch tj-actions vulnerability (#3626) * fix(alertbanner): change system variable from spectrum to legacy (#3624) * fix(alertbanner): change system: spectrum to legacy * chore(alertbanner): create changeset * test(checkbox): add more coverage for checkbox (#3625) * chore(checkbox): add isHovered state to checkbox - adds isHovered shared type and control to checkbox stories - adds several isHovered test cases - updates checkbox template to include isHovered arg * chore(form): fix the fieldgroup component input and labels * chore: release (#3631) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix(checkbox): add invalid+checked+hover checkbox styles (#3617) - add missing ::before pseudo to target the before element in the invalid/checked/hover state - update metadata.json - create changeset * chore: release (#3632) * chore: release --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix: undefined and duplicated variable after merging main fix(slider): remove duplicated values Remove a large number of duplicate values causing stylelint "Unexpected duplicate" warnings. It looks like things got doubled up somehow in a previous rebase or merge. This included duplicate t-shirt size classes. Also moves root styles block under the custom property definitions to be consistent with other components. fix(combobox): fixes undefined and duplicated values --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: TaraT <[email protected]> Co-authored-by: rise-erpelding <[email protected]> Co-authored-by: [ Cassondra ] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Cory Dransfeldt <[email protected]> Co-authored-by: Marissa Huysentruyt <[email protected]> Co-authored-by: aramos-adobe <[email protected]>

marissahuysentruyt added wip This is a work in progress, don't judge. do not merge A flag for a branch indicating it should not be merged. labels Mar 12, 2025

marissahuysentruyt self-assigned this Mar 12, 2025

marissahuysentruyt force-pushed the marissahuysentruyt/css-1139-checkbox-invalid-hover branch from f25d38b to 6c75a63 Compare March 12, 2025 14:51

marissahuysentruyt changed the title ~~fix(checkbox): wip~~ fix(checkbox): add hover states Mar 12, 2025

marissahuysentruyt force-pushed the marissahuysentruyt/css-1139-checkbox-invalid-hover branch from a5ab9df to b7c348a Compare March 12, 2025 16:39

marissahuysentruyt commented Mar 12, 2025

View reviewed changes

marissahuysentruyt force-pushed the marissahuysentruyt/css-1139-checkbox-invalid-hover branch from b7c348a to 94640f4 Compare March 12, 2025 17:03

marissahuysentruyt commented Mar 12, 2025

View reviewed changes

marissahuysentruyt removed wip This is a work in progress, don't judge. do not merge A flag for a branch indicating it should not be merged. labels Mar 12, 2025

marissahuysentruyt changed the title ~~fix(checkbox): add hover states~~ fix(checkbox): add hover state test coverage Mar 12, 2025

marissahuysentruyt marked this pull request as ready for review March 12, 2025 17:27

castastrophe force-pushed the main branch 10 times, most recently from c68f4e3 to d2272ea Compare March 12, 2025 21:56

marissahuysentruyt added bug Results from a bug in the CSS implementation ready-for-review labels Mar 13, 2025

marissahuysentruyt requested a review from cdransf March 14, 2025 12:56

marissahuysentruyt requested review from jawinn, castastrophe, 5t3ph, rise-erpelding and aramos-adobe March 14, 2025 12:56

marissahuysentruyt force-pushed the marissahuysentruyt/css-1139-checkbox-invalid-hover branch from 94640f4 to 4acee00 Compare March 14, 2025 18:53

marissahuysentruyt added wip This is a work in progress, don't judge. and removed ready-for-review labels Mar 14, 2025

marissahuysentruyt force-pushed the marissahuysentruyt/css-1139-checkbox-invalid-hover branch from 4acee00 to cfa9c01 Compare March 17, 2025 19:18

marissahuysentruyt added ready-for-review and removed wip This is a work in progress, don't judge. labels Mar 17, 2025

marissahuysentruyt commented Mar 17, 2025

View reviewed changes

components/checkbox/dist/metadata.json Show resolved Hide resolved

cdransf approved these changes Mar 17, 2025

View reviewed changes

castastrophe force-pushed the marissahuysentruyt/css-1139-checkbox-invalid-hover branch from cfa9c01 to 1dade0e Compare March 19, 2025 16:37

fix(checkbox): add invalid+checked+hover checkbox styles

b39b168

- add missing ::before pseudo to target the before element in the invalid/checked/hover state - update metadata.json - create changeset

marissahuysentruyt force-pushed the marissahuysentruyt/css-1139-checkbox-invalid-hover branch from 1dade0e to b39b168 Compare March 19, 2025 17:19

marissahuysentruyt changed the title ~~fix(checkbox): add hover state test coverage~~ fix(checkbox): corrects invalid+checked+hover background color Mar 19, 2025

castastrophe approved these changes Mar 19, 2025

View reviewed changes

castastrophe merged commit a02f1d1 into main Mar 19, 2025
12 checks passed

castastrophe deleted the marissahuysentruyt/css-1139-checkbox-invalid-hover branch March 19, 2025 17:51

github-actions bot mentioned this pull request Mar 19, 2025

chore: release #3632

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(checkbox): corrects invalid+checked+hover background color #3617

fix(checkbox): corrects invalid+checked+hover background color #3617

Uh oh!

marissahuysentruyt commented Mar 12, 2025 •

edited by cdransf

Loading

Uh oh!

changeset-bot bot commented Mar 12, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Mar 12, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Mar 12, 2025 •

edited

Loading

Uh oh!

marissahuysentruyt Mar 12, 2025

Uh oh!

marissahuysentruyt Mar 12, 2025

Uh oh!

marissahuysentruyt commented Mar 12, 2025

Uh oh!

jawinn commented Mar 14, 2025

Uh oh!

Uh oh!

cdransf left a comment

Uh oh!

Uh oh!

Uh oh!

fix(checkbox): corrects invalid+checked+hover background color #3617

fix(checkbox): corrects invalid+checked+hover background color #3617

Uh oh!

Conversation

marissahuysentruyt commented Mar 12, 2025 • edited by cdransf Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Jira/Specs

How and where has this been tested?

Validation steps

Regression testing

Screenshots

To-do list

Uh oh!

changeset-bot bot commented Mar 12, 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 Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

File metrics

Summary

checkbox

Uh oh!

marissahuysentruyt Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

marissahuysentruyt Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

marissahuysentruyt commented Mar 12, 2025

Uh oh!

jawinn commented Mar 14, 2025

Uh oh!

Uh oh!

cdransf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

marissahuysentruyt commented Mar 12, 2025 •

edited by cdransf

Loading

changeset-bot bot commented Mar 12, 2025 •

edited

Loading

github-actions bot commented Mar 12, 2025 •

edited

Loading

github-actions bot commented Mar 12, 2025 •

edited

Loading