-
Notifications
You must be signed in to change notification settings - Fork 11
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
CSS specificity #11
Comments
@rybon I found two issues with your Button functional component:
Maybe that already helps? |
No, that was just some code I quickly wrote to illustrate the issue. I fixed the syntax. The issue remains though. |
@rybon I can reproduce that issue now. I'm working on fixing it but it'll take some time. I'll keep you updated. |
Great! Thank you for picking this up. I appreciate all the work you put into this library. My idea for a possible fix: Say my root React component is |
I agree - Having a method of "netsting" classnames would allow for more specificity. As of right now, this generates an error, because "custombutton" is not one of the whitelisted pseudo elements that are allowed to be nested with objects: const stylesheet = cssInJS({
theme: {
// Some styles here for general theming
custombutton: {
// styles for button here
// these would theoretically be available with className={stylesheet.theme.custombutton}
// and generate '.src_themes_js-stylesheet-theme .src_themes_js-stylesheet-theme-custombutton'
}
}
}); I think the main issue to figure out is the following: Is the object (that isn't a pseudo element) styling for specific subelements (i.e. I think the following syntax would make sense to distinguish between the two to allow for this functionality: const stylesheet = cssInJS({
theme: {
// Some styles here for general theming
// Treat all nested objects that aren't pseudo elements as element styling within the class
// This generates the css rule .src_themes_js-stylesheet-theme div
div: {
},
// Treat all nested objects preceded by a '.' as a nested class for specificity purposes
// This generates the css rule '.src_themes_js-stylesheet-theme .src_themes_js-stylesheet-theme-custombutton'
'.custombutton': {
// styles for button here
// these would theoretically be available with className={stylesheet.theme.custombutton}
// and generate '.src_themes_js-stylesheet-theme .src_themes_js-stylesheet-theme-custombutton'
}
}
}); Thoughts? |
I think the problem is that the ordering of styles in the generated bundle CSS file isn't correct. See https://jsfiddle.net/11qmaeu4/ for an example on how order of styles matters (you are probably aware of that). The idea I have to fix this is to construct a dependency graph of components while the babel transformation happens (maybe something like that is already built-in into babel). Then I'll do a topological sort of those dependencies and write the components stylesheets in the resulting order into the bundle css. For example, given we have the following component dependencies:
Sorting that results in
which will be bundled as
Maybe that helps? WDYT? |
I'm not quite sure that will work for all project structures. For example, assume the following project structure:
In this case, how would you know where to stick the cssInJS generated classes into the bundle when the specificity of imported things might not be known? In other words, what if A, B, C, and D all depend on various classes from the imported stylesheet variables (from the themes directory)? How do those get ordered? |
I tested this approach and got the styles to be generated in a reverse order, but that didn't seem to help as far as I could determine. I feel the solution is to provide an API that will make certain selectors more specific than others. In my case, nesting my theme classes under an 'app' class is enough. I suppose we'd have to walk the component tree starting from the root, analyse every |
What about introducing a special css rule that acts as a directive to prepend something to the generated classname? Example: const PrimaryButton = () => (<Button className={styles.primary}>Click me!</Button>);
const styles = cssInJS({
primary: {
'@prepend': '.my-theme',
fontSize: 24
}
}); The above will be transformed to: const PrimaryButton = () => (<Button className={styles.primary}>Click me!</Button>);
const styles = {
primary: 'PrimaryButton_js-styles-primary'
}; And the generated CSS will be: .my-theme .PrimaryButton_js-styles-primary {
font-size: 24px;
} |
Sure, that could work. The only thing that feels a bit off with that approach is that we deviate from standard CSS syntax. But fine as a fix for now I suppose. |
Is the const styles = cssInJS({
mytheme: {
primary: {
fontSize: 24
}
}
}); To me, this feels more intuitive and is more expressive in the code for use (i.e. |
Oh I see what your main use case was. Is the idea that |
Here's a crazy idea that might solve all use cases. E.g.: const OuterComponent = () =>
<InnerComponent className={classNames(outerStyles.redBorder)} />;
const InnerComponent = ({className}) =>
<div className={classNames(className, innerStyles.blueBorder)} />; While parsing this, when we encounter the first instance of
When we encounter the second
Once parsing is done, this boils down to:
So we know to sort I think an approach like this is a more general and more robust solution than having to manually increase the specificity of the selectors. Thoughts? |
We could even take this one step further and consider the order of the styles inside the For example: const InnerComponent = ({className}) =>
<div className={classNames(innerStyles.defaultBorder, className, innerStyles.padding)} /> The intent here is that the border can be overridden, but the padding can’t. This can be achieved with a graph like this:
That would be transformed into:
Which results in:
Implementation could be tricky, but it seems doable. |
Great suggestions! Should be doable indeed. Maybe the PostCSS toolchain can help us out here. |
Isn't that what the classnames package already does? |
The JS API of the provided The preprocessor needs something to look for to detect when classes are applied to other components. I guess it could also just look for all occurrences of the |
All generated classes have the same specificity, but that complicates extending generic styles with custom styles for a specific theme.
Example:
When I render
CustomButton
, thecolor: 'blue'
still takes precedence overcolor: 'green'
, but I want my green theming to override the default styling. Is there a mechanism to do that without resorting to specificity hacks?The text was updated successfully, but these errors were encountered: