-
Notifications
You must be signed in to change notification settings - Fork 202
fix(switch): active down state specificity #3767
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
base: spectrum-two
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 52a70a4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
🚀 Deployed on https://pr-3767--spectrum-css.netlify.app |
File metricsSummaryTotal size: 1.41 MB*
switch
* An ASCII character in UTF-8 is 8 bits or 1 byte. |
29c083f
to
f23344a
Compare
2bfb96b
to
7465ca5
Compare
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.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
markdownlint-fix
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-menu-item-min-height", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-menu-item-spacing-multiplier", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-menu-item-top-edge-to-text", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-menu-item-value-color-default", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-menu-section-header-color", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-menu-section-header-min-width" |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-blue-800", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-border-width-200", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-component-top-to-text-100", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-corner-radius-100", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-focus-indicator-color", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-focus-indicator-thickness", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-gray-1000-rgb", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-navigational-indicator-top-to-back-icon-extra-large", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-spacing-100", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-text-to-visual-75", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-transparent-black-200-opacity", |
[markdownlint-fix] reported by reviewdog 🐶
spectrum-css/components/swatch/dist/metadata.json
Lines 7 to 8 in a51655a
".spectrum-Swatch--lightBorder .spectrum-Swatch-fill:before", | |
".spectrum-Swatch--noBorder .spectrum-Swatch-fill:before", |
[markdownlint-fix] reported by reviewdog 🐶
".spectrum-Swatch--sizeM", |
[markdownlint-fix] reported by reviewdog 🐶
".spectrum-Swatch-image", |
[markdownlint-fix] reported by reviewdog 🐶
".spectrum-Swatch-mixedValueIcon", |
[markdownlint-fix] reported by reviewdog 🐶
".spectrum-Swatch.is-mixedValue .spectrum-Swatch-fill", |
[markdownlint-fix] reported by reviewdog 🐶
spectrum-css/components/swatch/dist/metadata.json
Lines 38 to 41 in a51655a
".spectrum-Swatch.is-mixedValue .spectrum-Swatch-mixedValueIcon", | |
".spectrum-Swatch.is-nothing .spectrum-Swatch-fill", | |
".spectrum-Swatch.is-nothing .spectrum-Swatch-fill:after", | |
".spectrum-Swatch.is-nothing.spectrum-Swatch--rectangle .spectrum-Swatch-fill:after", |
[markdownlint-fix] reported by reviewdog 🐶
".spectrum-Swatch:after", |
[markdownlint-fix] reported by reviewdog 🐶
".spectrum-Swatch:focus-visible:after", |
[markdownlint-fix] reported by reviewdog 🐶
"--mod-animation-duration-100", |
[markdownlint-fix] reported by reviewdog 🐶
"--mod-swatch-border-color", |
[markdownlint-fix] reported by reviewdog 🐶
"--mod-swatch-border-color-light", |
[markdownlint-fix] reported by reviewdog 🐶
"--mod-swatch-border-radius", |
[markdownlint-fix] reported by reviewdog 🐶
"--mod-swatch-focus-indicator-border-radius", |
[markdownlint-fix] reported by reviewdog 🐶
"--mod-swatch-inner-border-color-selected", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-swatch-border-color", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-swatch-border-color-light", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-swatch-border-radius", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-swatch-dash-icon-color", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-swatch-focus-indicator-border-radius", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-swatch-inner-border-color-selected", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-swatch-size", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-animation-duration-100", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-focus-indicator-color", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-gray-25", |
[markdownlint-fix] reported by reviewdog 🐶
spectrum-css/components/swatch/dist/metadata.json
Lines 109 to 110 in a51655a
"--spectrum-gray-800", | |
"--spectrum-gray-900", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-red-900", |
[markdownlint-fix] reported by reviewdog 🐶
"--spectrum-workflow-icon-size-100", |
[markdownlint-fix] reported by reviewdog 🐶
"--highcontrast-swatch-focus-indicator-color" |
[markdownlint-fix] reported by reviewdog 🐶
".spectrum-SwatchGroup--spacious" |
[markdownlint-fix] reported by reviewdog 🐶
spectrum-css/components/swatchgroup/dist/metadata.json
Lines 8 to 11 in a51655a
"modifiers": [ | |
"--mod-swatchgroup-spacing-compact", | |
"--mod-swatchgroup-spacing-regular", | |
"--mod-swatchgroup-spacing-spacious" |
[markdownlint-fix] reported by reviewdog 🐶
"component": ["--spectrum-swatchgroup-spacing"], |
[markdownlint-fix] reported by reviewdog 🐶
"passthroughs": [], |
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.
I can see why we need the extra specificity but I'm not sure it will translate well when it gets to the web component (.spectrum-Switch.spectrum-Switch
converted likely to :host:host
). We can merge this in but would you mind adding some details about what selectors it's clashing with and why we added it?
.changeset/fluffy-hands-appear.md
Outdated
"@spectrum-css/switch": patch | ||
--- | ||
|
||
Fix active down state specificity |
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.
Could you expound a little on this in a long format? We use this information to populate our Changelogs so we try to include as much as possible to help customers validate this update on their end.
components/switch/index.css
Outdated
@@ -229,7 +229,7 @@ | |||
border-color: var(--highcontrast-switch-border-color-hover, var(--mod-switch-border-color-hover, var(--spectrum-switch-border-color-hover))); | |||
} | |||
|
|||
.spectrum-Switch:active & { | |||
.spectrum-Switch.spectrum-Switch:active & { |
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.
Can you add a comment about the specificity this helps with? This won't likely translate to the web component (:host:host) so we want to document that issue if we can.
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.
It looks like what it will convert to - :host(:active):host(:active)
- is accepted as a selector, and does bump specificity.
However, there is still a conflict when the switch is both checked and active:
Maybe we need to add a few more states to the testing grid for "Active + Unchecked" and "Active + Checked"
5dff2ea
to
52a70a4
Compare
Description
CSS-1176
RE
Note:We should consider conducting an audit to check for any additional hover specificity regressions introduced by this plugin.
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
Validate:
Screenshots
To-do list