feat: encapsulate Typography styles.
#383
Conversation
| 'inverted', | ||
| ])} | ||
| tone={select('Text tone', { | ||
| none: null, |
There was a problem hiding this comment.
fixes storybook proptype warning
There was a problem hiding this comment.
Could it also be fixed by providing a default value to select?
There was a problem hiding this comment.
The problem was that when you selected "none", it set the proptype to none, which is not in the list.
| @@ -39,8 +30,3 @@ a { | |||
| text-decoration: none; | |||
| transition: color 0.2s ease-in; | |||
| } | |||
|
|
|||
| p { | |||
There was a problem hiding this comment.
making reset.mod.css a bit lighter 👍
rollup.config.js
Outdated
| @@ -49,6 +50,7 @@ const postcssPlugins = [ | |||
| preserve: false, | |||
| importFrom: { 'custom-properties': customProperties }, | |||
| }), | |||
| postcssNesting(), | |||
There was a problem hiding this comment.
Nice. This removes the need to compose even more.
|
Yay for Percy 🎉 |
There was a problem hiding this comment.
Thanks for starting this!
I wonder whether we can simplify the CSS and avoid adding postcss-nesting.
To me postcss-nesting and css-modules don't seem like they would work together very well conceptually. But I've never actually used postcss-nesting together with css-modules.
| 'inverted', | ||
| ])} | ||
| tone={select('Text tone', { | ||
| none: null, |
There was a problem hiding this comment.
Could it also be fixed by providing a default value to select?
| font-size: var(--font-h4); | ||
| font-weight: normal; | ||
| } | ||
|
|
||
| h5 { | ||
| h5.subheadline { |
There was a problem hiding this comment.
Could we remove those h1 prefixes from the selectors? Our JS code should ensure that they're applied to the right elements. I wouldn't consider it part of the CSS's job.
There was a problem hiding this comment.
We still need it for the font size though right? either that or we need two different classnames.
There was a problem hiding this comment.
Ah yeah, I missed that part. I'm okay with either approach then 👍
|
|
||
| &.negative { | ||
| color: var(--color-red); | ||
| } |
There was a problem hiding this comment.
Could we define separate top-level classes instead? Then we might not need the postcss-nesting plugin as well.
So something like
.bold {
font-weight bold;
}
.positive {
color: var(--color-green-25);
}
.subheadline {
margin: 0;
color: var(--font-color);
}We are using the same "modifier" values from what I can tell, so this should reduce the amount of CSS as well.
There was a problem hiding this comment.
done, I removed postcss-nesting and updated the PR to simplify the CSS. VRT still passing. I love VRT.
montezume
changed the title
Typograph styles.Typography styles.
Closes #382