-
Notifications
You must be signed in to change notification settings - Fork 61
Please clearly show in the README the limitation / bugs it can create #4
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
Comments
I dont see problem. http://jsbin.com/wewunusubu/2 .root {
--text-color: red;
}
.component {
--text-color: blue;
}
.component .header {
color: var(--text-color);
}
.component .text {
--text-color: green;
color: var(--text-color);
} this would be compiled to .component .header {
color: blue;
}
.component .text {
color: green;
} so |
Yes this is ONE problem. The output you provide is completely not enough to say "No limitation". And when I read again this quote, I see a something wrong:
There is a bug + limitation with the current implementation. |
This is trying to convey: we look through the explicit structure of the rules and selectors in your CSS and resolve the value from that. I understand that with real CSS variables, the true cascade structure would resolve the values you are expecting. To get what you expect with Input:
Output:
|
So I have to repeat some definition with more css ? |
@MoOx I think what you are wanting would be in the minority of what people actually want/expect. I realize the CSS custom properties spec would make it your way which does make sense, but definitely not in terms of this plugin because no nesting structure was provided for us to work from. If I was nesting a component over and over, I would expect it it look the same. Maybe a better solution to your problem would be to use
|
You choose the syntax from w3c spec and you don't want to follow the spec ? Weird.
So don't put link to w3c spec everywhere in the readme. You are harming people that are eager to use w3c spec compliant code.
color was just an example. If you put any other properties, this whould work as expected by the specs? |
@MoOx I think the opening part of the readme explains pretty well and reiterates multiple times, that this can not perform exactly as the spec. The spec is linked for reference so people can check out the syntax or compare to the actual functionality. Because CSS custom properties are not widely supported. The use-case you bring up is not widely used today and in my opinion, is not a very good way to construct styles. To get the output you expect now, you would have to nest/descendant-selector anyway. |
Since this plugin does not provide a future proof code (see MadLittleMods/postcss-css-variables#4) and documentation is still misleading and unclear about the differences, I think it does not belong into this section.
Since this plugin does not provide a future proof code (see MadLittleMods/postcss-css-variables#4) and documentation is still misleading and unclear about the differences, I think it does not belong into this section.
Since this plugin does not provide a future proof code (see MadLittleMods/postcss-css-variables#4) and documentation is still misleading and unclear about the differences, I think it does not belong into this section. [skip ci]
@MadLittleMods Mind noting this spec deviation on the readme? Unlike some, I'm not as concerned about intentional deviations from spec (even Babel has a several with ES2015), but I'd find it much more comforting knowing them. I think this stems from the fact this plugin makes some unsafe assumptions regarding the selectors, like above. As in with this CSS: .root {
--text-color: red;
}
.component {
--text-color: blue;
}
.component .header {
color: var(--text-color);
}
.component .text {
--text-color: green;
color: var(--text-color);
} you'll get this for each one: //- In Jade, for brevity
.component: Black text
.component: .header: Blue text
.component: .header: .text: Green text
.component: .header: .text: .header: Green text per spec, blue per this polyfill The reason this is the case is because of selector order, and this plugin substitutes the color when it's not necessarily safe to do so. The result will end up like this: .root {
/* --text-color: red; */
}
.component {
/* --text-color: blue; */
}
.component .header {
color: /* var(--text-color) */ blue;
}
.component .text {
/* --text-color: green; */
color: /* var(--text-color) */ green;
}
/* The above, reduced */
.component .header { color: blue; }
.component .text { color: green; } The browser knows that This plugin, on the other hand, does not. It has no ability to know anything about the HTML itself (especially if it's generated through JS), and thus it is prone to making unsafe assumptions unless it's ultra-conservative like postcss-custom-properties. It would be appreciated if you note this in the README, though. |
Hey @isiahmeadows, Per your investigation and this issue, the deviation is not knowing the actual DOM structure and instead we work off the explicit structure assumed from your CSS selectors. Feel free to make a PR to update the readme to make this point more clear 😀 Currently we have this:
and this comment which definitely needs to be rewritten as I meant to say something like "assumed DOM structure from your explicit CSS selectors."
|
Based on [this comment](MadLittleMods#4 (comment)). I've also added + fixed several other things, and I cleaned up the organization and presentation to be a little more cohesive.
Currently you use a title like
With the current html, this is completlly wrong
See native result on firefox http://jsbin.com/tireneluxo/1/edit?html,css,output
Your output give a completly different result
http://jsbin.com/zacakilamo/1/edit?html,css,output
It's unexpected according to what I can read in the readme https://github.com/MadLittleMods/postcss-css-variables#using-cascading-variables-the-var-notation
So it's totally unclear.
The text was updated successfully, but these errors were encountered: