Skip to content
This repository has been archived by the owner on Feb 18, 2022. It is now read-only.

remove :root limitation by injecting rules with new declarations that just contains modified properties. #1

Closed
MoOx opened this issue Aug 1, 2014 · 35 comments

Comments

@MoOx
Copy link
Contributor

MoOx commented Aug 1, 2014

ref reworkcss/rework-vars#30

@MoOx MoOx changed the title remote :root limitation by injecting rules with new declarations that just contains modified properties. remove :root limitation by injecting rules with new declarations that just contains modified properties. Aug 1, 2014
@iamvdo
Copy link

iamvdo commented Aug 27, 2014

You can't do that because of unknown DOM structure, check example:

<button class="btn btn--active"></button>
.btn {
   --bg: red;
   background: var(--bg);
}
.btn--active {
  --bg: green;
}

The button should be green. You can't fallback this case with post-processor.

@MoOx
Copy link
Contributor Author

MoOx commented Aug 27, 2014

.btn {
   background: red;
}

.btn .btn--active,
.btn--active.btn,
.btn--active .btn {
  background: green;
}

What about this ?

@MoOx
Copy link
Contributor Author

MoOx commented Aug 27, 2014

You don't care about the dom, you can just output all the possible selectors.
I've to admit it can seems stupid, but this should work.

@MoOx
Copy link
Contributor Author

MoOx commented Aug 27, 2014

(I've update the possible output above)

@iamvdo
Copy link

iamvdo commented Aug 27, 2014

How you make relation between .btn and .btn--active, this could also be h1 and .title ?

BTW, the only solution I think is using Phantom as uncss does. I checked a bit but it seems rather too complicated for me ;)

@MoOx
Copy link
Contributor Author

MoOx commented Aug 27, 2014

You know both selectors use the same custom property name.

We are actually doing 2 pass (one to read values, one to replace). We can just create a smarter map with not just name-> value, but selector/name/value (and/or name/selector/value).

@MoOx
Copy link
Contributor Author

MoOx commented Aug 27, 2014

selector/name/value & name/selector/declaration should do the trick (or something like that)
I've to think about that just a bit, but it's doable.

@iamvdo
Copy link

iamvdo commented Aug 27, 2014

You know both selectors use the same custom property name.

Nope.

.title {
   background: var(--bg);
}
h1 {
  --bg: green;
}
h2 {
  --bg: red;
}

@iamvdo
Copy link

iamvdo commented Aug 27, 2014

Knowing DOM structure is essential.

@MoOx
Copy link
Contributor Author

MoOx commented Aug 27, 2014

You know that .title use --bg.
You know that h1,h2 declarage --bg so you know you will have to make a link between .title & h1,h2.

You can inject the following rules

.title {
   background: var(--bg);
}

h1 {
  --bg: green;
}

/* injected code */
.title h1,
h1.title,
h1 .title {
  background: green;
}

h2 {
  --bg: red;
}

/* injected code */
.title h2,
h2.title,
h2 .title {
  background: red;
}

@iamvdo
Copy link

iamvdo commented Aug 27, 2014

you know you will have to make a link between .title & h1,h2.

Not necessarily. You can use same variable names many times.

@bloodyowl
Copy link
Contributor

@MoOx
Copy link
Contributor Author

MoOx commented Aug 27, 2014

That's why I'm talking about 2 tuples name/selector/declaration and selector/name/value.
If you have others interesting examples to share I'm eager to read those so I can make rock solid tests :)

@iamvdo
Copy link

iamvdo commented Aug 27, 2014

@MoOx Replace h1 h2 with div and p:

.block {
   background: var(--bg);
}
div {
  --bg: green;
}
p {
  --bg: red;
}

Structure can be:

<div class="block">
  <p>
    BLOCK
  </p>
</div>
<div>
  <p class="block">
    BLOCK
  </p>
</div>

Or:

<div class="block">
  <p class="block">
    BLOCK
  </p>
</div>
<div class="block">
  <p class="block">
    BLOCK
  </p>
</div>

Or anything else...

@bloodyowl
Copy link
Contributor

@iamvdo
Copy link

iamvdo commented Aug 27, 2014

@bloodyowl Tu voudrais pas juste être un peu plus constructif !

@bloodyowl
Copy link
Contributor

.block {
   background: var(--bg);
}
div {
  --bg: green;
}
p {
  --bg: red;
}

qui donne

div.block {
  background: green;
}
p.block {
  background: red;
}

c'est bien ce à quoi on s'attend, nan? la structure du DOM n'a pas d'impact sur les sélecteurs concernés. il suffit juste de réunir tous les sélecteurs définissant une variable n avec ceux qui l'utilisent.

@iamvdo
Copy link

iamvdo commented Aug 27, 2014

Non justement. Relis la spec des propriétés personnalisées CSS et tu verras que la cascade y joue un grand rôle. Impossible de connaitre à l'avance.
Une propriété perso définit la valeur pour l'ensemble de la cascade qui en découle. Peu importe si c'est des div, des p, des .block ou autre.

@MoOx
Copy link
Contributor Author

MoOx commented Aug 27, 2014

By creating all possibles (& potentially stupids) selectors, cascade can be respected.

For now you didn't show us an example that can be handled by a transformation.

@bloodyowl
Copy link
Contributor

ben il suffit de dupliquer :

div.block,
div.block .block {
  background: green;
}
p.block,
p.block .block {
  background: red;
}

@iamvdo
Copy link

iamvdo commented Aug 27, 2014

OK, sometimes you have to, sometimes not, but you don't know when, that's where the problem is.

@MoOx
Copy link
Contributor Author

MoOx commented Aug 27, 2014

This is not an issue, you just put all possibilities all the times & boom problem solved.

@iamvdo
Copy link

iamvdo commented Aug 27, 2014

Nope, but btw, try to code something and I'll be there to find issues ;)

@MoOx
Copy link
Contributor Author

MoOx commented Sep 24, 2014

What do you think about this test (try on firefox plz) http://jsbin.com/hokode/1/edit

@MoOx
Copy link
Contributor Author

MoOx commented Sep 24, 2014

An update on my current wip https://gist.github.com/MoOx/28808ebe3a9e4bd897ef

@MoOx
Copy link
Contributor Author

MoOx commented Sep 24, 2014

Here I think I got something for the nesting issue (cf gist comments) http://jsbin.com/hunuy/2/edit

@necolas
Copy link
Contributor

necolas commented Sep 27, 2014

This isn't possible/practical :) Not worth the time IMO.

@MoOx
Copy link
Contributor Author

MoOx commented Sep 28, 2014

I've implemented something locally that works except for the deep nesting issue (that will need some client side shit - or a clever idea I didn't get) using a tuple with custom prop/selector tree/value.

This handle (& that's a interesting part even if we still limit to :root) :root in media queries like
reworkcss/rework-vars#17
I can try to make something clean & still limited to :root for now to support :root in mq which could be really a nice add. Don't you think ?

@necolas
Copy link
Contributor

necolas commented Sep 28, 2014

Supporting :root in media queries is a different problem. Might generate a lot of extra CSS, but I think it's feasible. However, the topic of this issue is not.

@necolas
Copy link
Contributor

necolas commented Sep 30, 2014

Maybe close this for similar reasons to #9 (not feasible and staying focused on shipping)?

@MoOx
Copy link
Contributor Author

MoOx commented Oct 1, 2014

Yeah, I was so stubborn... We can't respect the cascade correctly.
My bad for everyone involved in this thread :(

I'll update the README then...

@kevinSuttle
Copy link

Oh man. This is a bummer, but thanks for all the work that went into figuring this out up until this point. Hopefully we'll be able to use scope-defined custom properties soon.

@kevinSuttle
Copy link

kevinSuttle commented Jun 6, 2016

This also seems to mean that you cannot import CSS files that use :root declarations, unless there's an implied PostCSS plugin that I should be using like https://github.com/postcss/postcss-import.

e.g.

/* Theme.css */
:root {
  --HighlightColor: #3acfb6;
}
/* fonts.css */
import "./Theme.css"; 

.cardTitle {
  color: var(--HighlightColor)
}

Outputs:

variable '--HighlightColor' is undefined and used without a fallback

@kristoferjoseph
Copy link

kristoferjoseph commented Jun 6, 2016

@kevinSuttle got around that by doing terrible things this in my webpack.config.
I am not proud of this: https://gist.github.com/kristoferjoseph/16ed0f0d368be2f4610da815d58410a6

UPDATE:

@MoOx approves

@MoOx
Copy link
Contributor Author

MoOx commented Jun 6, 2016

It's not terrible, it's the clean way to do the thing ;)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants