-
Notifications
You must be signed in to change notification settings - Fork 40
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
feat: css modules #2318
feat: css modules #2318
Changes from 1 commit
9e008e2
e2871bf
df6eaba
4aa5bb7
cbb1a8f
092ecce
014641a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,13 +2,16 @@ | |
|
||
/* https://wiki.csswg.org/ideas/mistakes */ | ||
|
||
/* TODO: Can this be moved to the related components? */ | ||
[class^='ds-'], | ||
[class^='ds-']::before, | ||
[class^='ds-']::after { | ||
box-sizing: border-box; | ||
} | ||
|
||
/* Inherit fonts for inputs and buttons */ | ||
|
||
/* TODO: Can this be moved to the related form element CSS files? */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think this is a better candididate for moving to to their relevant components. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See comment above regarding targeting start of classnames :) |
||
input[class^='ds-'], | ||
button[class^='ds-'], | ||
textarea[class^='ds-'], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
/* TODO: Could these styles be placed in css root like the other files? */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed |
||
.ds-paragraph { | ||
--dsc-bottom-spacing: var(--ds-spacing-5); | ||
|
||
|
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.
This is relevant for all components, it's the same as doing
So I think this is fine to have in this file 🤔
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.
Not if moving to CSS modules or allowing people to extend our classes like:
Also, this targets only if elements class names starts with
ds-
soclass="ds-button my-button"
works, butclass="my-button ds-button"
does not. I think it is safer and more explicit to just addbox-sizing: border-box
into the relevant classes in the relevant files :)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.
Ah yeah, but this should be
[class~="ds-"]
😄