fix: remove redundant default on data attributes#4999
Conversation
🦋 Changeset detectedLatest commit: e1c0eb3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
|
Preview deployments for this pull request: storybook - |
There was a problem hiding this comment.
Pull request overview
This PR removes default variant prop values from several React components so they don’t always emit redundant data-variant="..." attributes in the rendered markup, aligning better with the CSS that typically treats “default” as the absence of a variant attribute.
Changes:
- Removed default
variantvalues inAvatar,Badge,Card,Details,Paragraph,Popover,Skeleton, andTagsodata-variantis omitted unless explicitly provided. - Updated
Avatartest to stop asserting the defaultdata-variantattribute. - Added a changeset documenting the patch change for
@digdir/designsystemet-react.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/components/tag/tag.tsx | Stop defaulting variant to avoid always rendering data-variant. |
| packages/react/src/components/skeleton/skeleton.tsx | Remove default variant so data-variant is only present when set. |
| packages/react/src/components/popover/popover.tsx | Remove default variant to avoid redundant data-variant="default". |
| packages/react/src/components/paragraph/paragraph.tsx | Remove default variant so default styling is represented by no attribute. |
| packages/react/src/components/details/details.tsx | Remove default variant to avoid redundant default data attribute output. |
| packages/react/src/components/card/card.tsx | Remove default variant to reduce redundant markup. |
| packages/react/src/components/badge/badge.tsx | Remove default variant so data-variant="base" isn’t emitted by default. |
| packages/react/src/components/avatar/avatar.tsx | Remove default variant so data-variant="circle" isn’t emitted by default. |
| packages/react/src/components/avatar/avatar.test.tsx | Adjust test expectations to match removal of default data-variant. |
| .changeset/tiny-rocks-sell.md | Changeset entry for the patch release. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
What do we tell people that have selectors based on these defaults? |
We haven't removed any selectors, but are you maybe thinking about documenting why some variants are not present in the data-att table or not able to override with selectors? |
Right. What I mean is people that have custom CSS that target these defaults |
Ah, ok now I get it, so if people have made their own data-attribute selectors that target the expected defaults? hmm, good question. Its not hard to fix downstream, we have it documented and how many edge-case scenarios are we going to provide backwards compability for? 🤔 They have been more in the way then helped. We have already removed some of them. |
Another thing that speaks in favor of doing this change is that consumers that need the default selectors can just pass these in as props, since we still allow them to be set |
resolves #4843
Removing redundant
data-variantdefault values in react components as they already have the following style by default in CSS.Doing to reduce markup clutter and easier to override style globally (#4955 , #4968).
Remove following
data-variantdefaults: