Skip to content
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

fix: minimize css custom property reassignment pt 1 #3043

Open
wants to merge 8 commits into
base: next
Choose a base branch
from

Conversation

eirikbacker
Copy link
Contributor

@eirikbacker eirikbacker commented Jan 22, 2025

  • Minimizing CSS custom property reassignment
  • Started convention --ds-internal- for internal CSS custom properties - should we do this? 🤔
  • Only button has re-assignments due to data-variant - should we have more CSS props for this then?
  • The components where data-color is defined in compontent CSS file, still have re-assignments
    • This applies to: alert, badge, button, card, details, link, popover, validation
    • This will probably change when surface exists as white, so lets deal with these re-assignments after surface is done

@eirikbacker eirikbacker self-assigned this Jan 22, 2025
Copy link

changeset-bot bot commented Jan 22, 2025

⚠️ No Changeset found

Latest commit: 99062c8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

Copy link
Contributor

github-actions bot commented Jan 22, 2025

Preview deployments for this pull request:

Storybook - 24. Jan 2025 - 14:27

Copy link
Contributor

github-actions bot commented Jan 22, 2025

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 53.48% 2958 / 5531
🔵 Statements 53.48% 2958 / 5531
🔵 Functions 85.71% 180 / 210
🔵 Branches 76.79% 523 / 681
File CoverageNo changed files found.
Generated in workflow #1791 for commit 99062c8 by the Vitest Coverage Report Action

@Barsnes
Copy link
Member

Barsnes commented Jan 24, 2025

Input, radio, checkbox still works fine with forced colors

@Barsnes
Copy link
Member

Barsnes commented Jan 24, 2025

on second look, the check is the wrong color.
This is your PR:
image

This is in next:
image

It's only for the checkbox, though the radio has a smaller border as well

@eirikbacker eirikbacker assigned Barsnes and unassigned eirikbacker Jan 24, 2025
@eirikbacker
Copy link
Contributor Author

Thanks for good testing @Barsnes! ☺️ Super that you can follow up this as you have the best test device 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 👀 Ready for review
Development

Successfully merging this pull request may close these issues.

3 participants