feat(badge): remove modifiers from the API [SWC-1264]#4283
feat(badge): remove modifiers from the API [SWC-1264]#4283castastrophe merged 1 commit intospectrum-twofrom
Conversation
|
| Name | Type |
|---|---|
| @spectrum-css/alertdialog | Major |
| @spectrum-css/asset | Major |
| @spectrum-css/assetcard | Major |
| @spectrum-css/assetlist | Major |
| @spectrum-css/avatar | Major |
| @spectrum-css/badge | Major |
| @spectrum-css/miller | Major |
| @spectrum-css/well | Major |
| @spectrum-css/bundle | Patch |
| @spectrum-css/preview | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
castastrophe
added
size-2 S
📚 Branch previewPR #4283 has been deployed to Azure Blob Storage: https://spectrumcss.z13.web.core.windows.net/pr-4283/index.html. |
File metricsSummaryTotal size: 1.41 MB*
badge
* An ASCII character in UTF-8 is 8 bits or 1 byte. |
castastrophe
force-pushed
the
castastrophe/feat-badge-modifier-removal
branch
from
867caf9 to
bf4974a
Compare
castastrophe
force-pushed
the
castastrophe/feat-badge-modifier-removal
branch
from
bf4974a to
8237249
Compare
| ".spectrum-Badge--turquoise", | ||
| ".spectrum-Badge--yellow", | ||
| ".spectrum-Badge--subtle", | ||
| ".spectrum-Badge--subtle:where(.spectrum-Badge--accent)", |
There was a problem hiding this comment.
FYI in the SWC next-gen web component we updated this class to remove the -style from the class name.
rise-erpelding
left a comment
rise-erpelding
left a comment
There was a problem hiding this comment.
Looks good! Left some questions for building my own knowledge on this work.
| --- | ||
|
|
||
| This update removes `--mod-*` custom property hooks per SWC-1264, see also the RFC for extensible styling. | ||
| This update removes `--mod-*` custom property hooks per SWC-1264, see also the RFC for extensible styling. In addition, this update cleans up any component-level custom properties that did not rely on the CSS cascade to define the styles; this was done to reduce the number of custom properties that are defined at the component level and trim down the size of the CSS we are shipping to consumers. |
There was a problem hiding this comment.
Love this update! ❤️
| /* background color variants */ | ||
| &.spectrum-Badge--neutral { | ||
| background: var(--mod-badge-subtle-background-color-default, var(--spectrum-badge-subtle-background-color-default)); | ||
| &:where(.spectrum-Badge--neutral) { |
There was a problem hiding this comment.
Tell me more about :where() in this context, it seems like the benefit over trying to match both classes is that it doesn't add specificity, right? I'm especially curious as we carry out similar CSS updates in other components. Will we want to consider this pattern in similar use cases in other components? Any pitfalls to be aware of? Any other things to note? (I believe it was :where in another component in that action menu PR I reviewed where we discovered that we actually did want the added specificity, for instance.)
There was a problem hiding this comment.
Yes, great questions. Badge is a good use-case for :where in that there are only a few variants active at any given time. I kept specificity on outline and subtle but where the colors showed up, I let the order they show up in the file be the deciding factor. For more complex components this gets ultra tricky because there are cases where we want that specificity. This seemed like a great opportunity to reduce it though so I added it here.
| /* Style = subtle */ | ||
| .spectrum-Badge--style-subtle { | ||
| color: var(--mod-badge-subtle-label-icon-color, var(--spectrum-badge-subtle-label-icon-color)); | ||
| .spectrum-Badge--subtle { |
There was a problem hiding this comment.
I'm on board with this change (and I see it's in barebones already 🎉 ), and given that our changeset will reflect alll the other component changes, I think I can see the rationale for not documenting this change in the changeset and I'm on board with that too... but I still want to highlight it for consideration in case someone else has a strong opinion or we have second thoughts.
There was a problem hiding this comment.
Yes, I did check our changesets to see if there were any references to the new class name and since it wasn't there, I opted to leave it out of the new docs. The concept of what was added is in the previous changeset but not the class name specifically.
There was a problem hiding this comment.
Are there any plans to carry over these changes to barebones? I'm assuming we'd want this version of the CSS to land in SWC at some point.
2 participants
Description
This update removes
--mod-badge-*custom property hooks per SWC-1264, see also the RFC for extensible styling. Class selectors and variants remain unchanged; stories were refreshed to match the current API.--mod-badge-*custom property hooks.Breaking change: the
--mod-badge-*override layer is removed. Consumers should set--spectrum-badge-*variables directly where customization as needed.Related issue(s)
Author's checklist
Reviewer's checklist
patch,minor, ormajorfeaturesValidation steps
spectrum-twobaseline.Regression testing
Validate:
The documentation pages for at least two other components are still loading, including:
If components have been modified, VRTs have been run on this branch: