-
Notifications
You must be signed in to change notification settings - Fork 2
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
styled bad order #72
Comments
As far as I remember, media-rules are added to the end, but depending on whether the style is static or dynamic. The order is: static style + static media + dynamic style + dynamic media. Try workaround: @media (prefers-reduced-motion: reduce) {
& ul li a {
transition: ${() => 'none'}; // fn
}
} |
The order is unchanged. Besides, I do not think it is a good solution: I really have to think about the way styled works to sort my attributes based on that. I think I defined the media query at the end and it should be rendered at the end, according to the order I defined it. |
Let's say we have this code and media-query is inserted at the end when mounting the component. Everything will work. But what happens when a new component with a different color appears? The style will be calculated and inserted at the end of the style tag and it will no longer be correct. What to do in this case, provided that we cannot touch already inserted styles. Otherwise, we would have to parse the already inserted style blocks from the entire application every time: something has changed and we parse the entire style tag again, rearrange the styles and rewrite the tag again. This looks like a second parsing job and it will most likely be slow. What to do in this case? const Box = styled.div`
& a {
color: ${props => props.$color};
}
@media (max-width: 800px) {
& a {
color: red;
}
}
`; Maybe it is good case for const Box = styled.div`
& a {
color: ${props => props.$color};
}
@media (max-width: 800px) {
& a[href] {
color: red;
}
}
`; |
Shouldn't styled create a new class for the different color and insert it afterwards? const Box = styled.div`
& a {
color: ${props => props.$color};
}
@media (max-width: 800px) {
& a {
color: red;
}
}
`; .class-one a {
color: blue;
}
@media (max-width: 800px) {
.class-one a {
color: red;
}
}
.class-two a {
color: green;
}
@media (max-width: 800px) {
.class-two a {
color: red;
}
} |
Then we need to support one more nesting, because I don't know how easy it is to add this to the current architecture. We need to see if this can be done in the original styled-components. const Box = styled.div`
& a {
color: ${props => props.$color};
@media (max-width: 800px) {
color: red;
}
}
`; |
Outdated react and styled component versions because I just cloned the first random github repository to test this const Box = styled.div`
& a {
font-weight: bold;
color: ${props => `${props.$color}`};
}
@media (max-width: 800px) {
& a {
color: red;
}
}
`;
class App extends Component {
render() {
return (
<>
<Box $color='blue'>
<a>blue</a>
</Box>
<Box $color='green'>
<a>green</a>
</Box>
</>
);
}
} <style data-styled="active" data-styled-version="5.3.3">
.gLrqxF a{font-weight:bold;color:blue;}@media (max-width:800px){.gLrqxF a{color:red;}}
.eKiNWE a{font-weight:bold;color:green;}@media (max-width:800px){.eKiNWE a{color:red;}}
</style> I think this is the behavior I expected This also seems to work, this syntax looks more like LESS (what I am used to) but the previous one is more similar to standard CSS, which I think is most important const Box = styled.div`
& a {
font-weight: bold;
color: ${props => `${props.$color}`};
@media (max-width: 800px) {
color: red;
}
`; <style data-styled="active" data-styled-version="5.3.3">
.kscQJB a{font-weight:bold;color:blue;}@media (max-width:800px){.kscQJB a{color:red;}}
.kSMeAQ a{font-weight:bold;color:green;}@media (max-width:800px){.kSMeAQ a{color:red;}}
</style> |
If we change the code a little, we will see this in styled-components const Box = styled.div`
background-color: blue; // added
color: black; // added
& a {
font-weight: bold;
color: ${(props) => `${props.$color}`};
}
@media (max-width: 800px) {
& a {
color: red;
}
}
`; .iLKKBR {
background-color: blue;
color: black;
}
.iLKKBR a {
font-weight: bold;
color: blue;
}
@media (max-width: 800px) {
.iLKKBR a {
color: red;
}
}
.duYaUQ {
background-color: blue;
color: black;
}
.duYaUQ a {
font-weight: bold;
color: green;
}
@media (max-width: 800px) {
.duYaUQ a {
color: red;
}
} It looks like it just stupidly duplicates all the styles for the new props. I had a better opinion of them. This just generates a bunch of duplicate code. And the css of the component can be quite large, apparently it will repeat all this. Dark generates this (without empty classes, i will fix it later) .dk-caccjf {
background-color: blue;
color: black;
}
@media (max-width: 800px) {
.dk-caccjf a {
color: red !important; // for now
}
}
.dk-dbegje a {
font-weight: bold;
color: blue;
}
.dk-iaijad a {
font-weight: bold;
color: green;
} Anyway, I will think about it. |
You're right, in styled-components 5.3.3 const Box = styled.div`
background-color: lightgray;
color: black;
& a {
font-weight: bold;
color: ${props => `${props.$color}`};
@media (max-width: 800px) {
color: red;
}
`;
class App extends Component {
render() {
return (
<>
<Box $color='blue'>
<a>blue</a>
</Box>
<Box $color='green'>
<a>green</a>
</Box>
</>
);
}
} results in .biclYi {
background-color: lightgray;
color: black;
}
.biclYi a {
font-weight: bold;
color: blue;
}
@media (max-width:800px) {
.biclYi a {
color: red;
}
}
.bLMHlz {
background-color: lightgray;
color: black;
}
.bLMHlz a {
font-weight: bold;
color: green;
}
@media (max-width:800px) {
.bLMHlz a {
color: red;
}
}
|
"dependencies": {
"react": "18.3.1",
"react-dom": "18.3.1",
"styled-components": "6.1.11"
}, gzip can certainly help, but we stand for beautiful solutions, and repeating styles that do not require repetition is not beautiful. |
I've been trained to be afraid of const Box = styled.div`
& a {
font-weight: bold;
color: ${props => `${props.$color}`};
}
@media (min-width: 500px) {
& a {
color: red;
}
@media (min-width: 900px) {
& a {
color: teal;
}
}
`;
I absolutely agree, as long as it works. |
@media (min-width: 500px) {
& a[data-spec] {
color: red;
}
} In this case, these expressions will be added in the order they are written, so here you set the priority yourself. @media (min-width: 500px) {
& a {
color: red;
}
}
@media (min-width: 900px) {
& a {
color: teal;
}
} |
Based on this logic, in your case it would be ideal to use CSS variables rather than generate styles dynamically. This would remove this problem and work faster because there is no js calculations. At the same time, the theme could also be switched, simply changing the variables in |
I do that already, but the original issue was about the wrong position of media queries generated with
In this situation I guess Again, I do not want to think in which situation I should use one technique and in which situation I should use another. Maintaining such a codebase would be awful: some media queries here some there, some use weird selectors some don't. |
These solutions have been proposed as hacks until I can make fixes to the codebase. But I also have to say that I won't be able to contribute them as often as I'd like, because open-source is not where I work. |
Of course. |
In the new version this code will generate these styles const Box = styled.div`
background-color: pink;
color: black;
& a {
font-weight: bold;
color: ${p => `${p.$color}`};
@media (max-width: 800px) {
color: red;
}
}
`;
createRoot(document.getElementById('root')).render(
<>
<Box $color='blue'>
<a>blue</a>
</Box>
<Box $color='green'>
<a>green</a>
</Box>
</>,
); .dk-bjjdjh {
background-color: pink;
color: black;
}
.dk-ggedee a {
font-weight: bold;
color: blue;
}
@media (max-width: 800px) {
.dk-ggedee a {
color: red;
}
}
.dk-bjajid a {
font-weight: bold;
color: green;
}
@media (max-width: 800px) {
.dk-bjajid a {
color: red;
}
} |
I just realized that if we had written it like this, we wouldn’t have had any problems in the first place. const Box = styled.div`
background-color: pink;
color: black;
${p => css`
& a {
font-weight: bold;
color: ${p.$color};
}
@media (max-width: 800px) {
& a {
color: red;
}
}
`}
`; This example brought me to this idea. It will break everything again, so nested expressions are not a solution const Box = styled.div`
background-color: pink;
color: ${p => p.$color};
@media (max-width: 800px) {
color: red;
}
`; In this case, it would be correct to write like this const Box = styled.div`
background-color: pink;
${p => css`
color: ${p.$color};
@media (max-width: 800px) {
color: red;
}
`}
`; Sorry that I did not immediately come to this decision and suggested options with !important and others. This is the only correct approach given the architecture based on avoiding duplicates of CSS. Your example const LinksRow = styled.div`
display: flex;
justify-content: space-between;
gap: 2.5vw;
& ul {
list-style-type: none;
padding: 0;
margin: 0;
width: 100%;
}
& ul li {
list-style: none;
border-bottom: ${props => props.theme.borderStyle};
}
${props => css`
& ul li a {
display: flex;
justify-content: space-between;
align-items: center;
text-decoration: none;
color: ${props.theme.black};
padding: 0.5vw 0;
transition: color 0.2s, background-color 0.2s, padding 0.3s;
}
& ul li a:hover {
color: ${props.theme.white};
background-color: ${props.theme.black};
padding: 0.5vw;
}
@media (prefers-reduced-motion: reduce) {
& ul li a {
transition: none;
}
}
`}
`; I understand that when writing you have to think about how to do it. But it seems to me that this is a rather specific case, when we write styles not only for the root tag itself, but also for nested ones, and at the same time we want to interrupt the specificity of selectors in the media expression. |
I am trying to understand how I should be thinking to obtain the expected behavior in the future: |
To be honest, I’m still thinking about some kind of single rule. I also came to this variant - writing the problematic property separately so that it does not end up in the dynamic class if it does not depend on props. If it has such a dependence, then write all its uses inside the function. const LinksRow = styled.div`
display: flex;
justify-content: space-between;
gap: 2.5vw;
& ul {
list-style-type: none;
padding: 0;
margin: 0;
width: 100%;
}
& ul li {
list-style: none;
border-bottom: ${props => props.theme.borderStyle};
}
& ul li a {
display: flex;
justify-content: space-between;
align-items: center;
text-decoration: none;
color: ${props => props.theme.black};
padding: 0.5vw 0;
}
& ul li a:hover {
color: ${props => props.theme.white};
background-color: ${props => props.theme.black};
padding: 0.5vw;
}
& ul li a {
transition: color 0.2s, background-color 0.2s, padding 0.3s; // write this prop separately
}
@media (prefers-reduced-motion: reduce) {
& ul li a {
transition: none;
}
}
`; Or we should regenerate the entire CSS of the component, as is done in styled-components. |
We should probably abandon the approach of reusing styles if it causes such problems. I'll think about it. |
the media query style is outputted before the
& ul li a
style.Because they have the same specificity, the browser ignores the media query styles. I believe the media query should be put after the styles defined before it.
The text was updated successfully, but these errors were encountered: