postcss / postcss-custom-properties

Use Custom Properties in CSS
https://postcss.github.io/postcss-custom-properties
MIT License
596 stars 77 forks source link

Investigate support for media-query scoped properties #9

Closed necolas closed 6 years ago

necolas commented 9 years ago

See: https://github.com/reworkcss/rework-vars/issues/17. I still think this is inherently problematic (like plugins that consolidate media queries), but worth exploring to get an answer.

Example input:

:root { --fontSize: 2em; }
.Section { font-size: var(--fontSize); }

@media (min-width: 64em) {
  :root { --font-size: 3em; }
}

Example output:

.Section { font-size: 2em; }

@media (min-width: 64em) {
  .Section { font-size: 3em; }
}

Complications could include:

<div class="One Two">Text</div>

Input:

:root { --fontSize: 2em; }
.One { font-size: var(--fontSize); }
.Two { font-size: 6em; }

@media (min-width: 64em) {
  :root { --font-size: 3em; }
}

Output (notice One now overrides Two, which it would not with a native solution):

.One { font-size: 2em; }
.Two { font-size: 6em; }

@media (min-width: 64em) {
  .One { font-size: 3em; }
}
MoOx commented 9 years ago

I've something in a stash that can do the following

@media screen and (min-width: 320px) {
  :root {
    --size: 1em;
  }

  @media (min-height: 320px) {
    :root {
      --size: 0.5em;
    }
  }
}

:root {
  --size: 2em;
}

body {
  font-size: var(--size);
}

@media screen and (min-width: 320px) {
  p {
    font-size: var(--size);
  }
}

body {
  font-size: 2em;
}

@media screen and (min-width: 320px) {body {font-size: 1em;}}

@media screen and (min-width: 320px) {@media (min-height: 320px) {body {font-size: 0.5em;}}}

@media screen and (min-width: 320px) {
  p {
    font-size: 2em;
  }

@media screen and (min-width: 320px) {p {font-size: 1em;}}

@media screen and (min-width: 320px) {@media (min-height: 320px) {p {font-size: 0.5em;}}}
}

Obviously this is producing a lot of code (don't look mq values, there are stupid), but I think it's doing what user can expect.

Note: we might want to optimize mq output after that.

MoOx commented 9 years ago

inlined @import statements in media queries

That's the point of this issue & why we should handle that. I'm not sure what the problem here.

not knowing the relationship between different media queries (might generate extra CSS rules that aren't needed, e.g., what if the MQ is @media screen (min-width:0)?)

Do we really need to handle that ? If user is writing weird css, he can only get weird output :) Maybe a second pass can be done to optimise a little bit ? In my example above, we can probably group some rules easily.

generating large amounts of extra CSS

With great powers came great responsabilities ?

specificity/cascade issues - moving/generating rules is always going to hit the problem where earlier styles are unintentionally overridden.

I think in the example above, the cascade is respected (& in this case of my local wip - sourcemap is referring to the custom prop definition).

necolas commented 9 years ago

Do we really need to handle that?

The point isn't about handling it, but about holes in the model because of incomplete information.

think in the example above, the cascade is respected

In my example, the resulting style resolution is different to what it would be with native custom properties. These kinds of divergences are unavoidable when you inject rules or move them around. All the media query packer plugins have the same caveat.

MoOx commented 9 years ago

Well I'm so focus on shipping something that I miss these kind of issues. It's a dangerous game indeed. So not sure if there is a way to make this right :/

necolas commented 9 years ago

Probably not. Anyway, I think we've done some investigation so this can be closed. It was always going to be a low-priority anyway.

hgl commented 9 years ago

Can we add an option to allow this type of media scoped properties?

Input

:root { --fontSize: 2em; }
.One { font-size: var(--fontSize); }
.Two { font-size: 6em; }

@media (min-width: 64em) {
  :root { --font-size: 3em; }
  .One { font-size: var(--fontSize); }
}

=>

.One { font-size: 2em; }
.Two { font-size: 6em; }

@media (min-width: 64em) {
  .One { font-size: 3em; }
}

In this case, the author manually add the rule to @media to promise cssnext that specificity won't be an issue.

And with this option enabled, if they give an input:

:root { --fontSize: 2em; }
.One { font-size: var(--fontSize); }
.Two { font-size: 6em; }

@media (min-width: 64em) {
  :root { --font-size: 3em; }
}

we only produce

:root { --fontSize: 2em; }
.One { font-size: var(--fontSize); }
.Two { font-size: 6em; }

This isn't spec compliant, but this is because the author gave an incorrect input (for this option). She should know how the option works before enabling it. Although the option itself isn't spec compliant, giving it the correct input, it will generate spec compliant code.

The reason I want this behavior is that if I put all variables in the top level, naming becomes very painful:

:root { --fontSize: 2em; --wideViewFontSize: 3em }
.One { font-size: var(--fontSize); }
.Two { font-size: 6em; }

@media (min-width: 64em) {
  .One { font-size: var(--wideViewFontSize); }
}

Tons of variables need to be prefixed with wideView. length isn't the only problem. Imagine a variable that only gets applied in a wide view (e.g., --navWidth: 960px, in a narrow vide, the nav is as wide as the viewport, so the custom property isn't used), now should you prefix the name with wideView or not? (What if a even wider view use the 960px value, should you create another --extraWideViewNavWidth with the same value?) With scoped properties, these problems all go away.

hgl commented 9 years ago

But there is probably one problem: passing js defined custom properties and extracting custom properties is not as simply as it current does.

I can think of a few solutions:

MoOx commented 9 years ago

Both solutions are dangerous. I don't think we should implement partial & weird solutions.

Maybe you can "just" use a shorter convention for size like I do

:root {
  --fontSize: 2em;
  --fontSize--XL: 3em; /* i use xs, s, m, l, xl */
}
.One { font-size: var(--fontSize); }
.Two { font-size: 6em; }

@media (min-width: 64em) {
  .One { font-size: var(--fontSize--XL); }
}
hgl commented 9 years ago

A bit cryptic IMHO. But I agree the option is dangerous. I'm looking at shadow dom now to see if it can help.

MoOx commented 9 years ago

@necolas what do you think about the solution below (injecting rules after the selector using the custom prop) ? Someone opened an issue on cssnext #https://github.com/cssnext/cssnext/issues/99 with this solution and with a quick test it seems it is working as native custom props.

<div class="One Two">Text</div>

Input:

:root { --fontSize: 2em; }
.One { font-size: var(--fontSize); }
.Two { font-size: 6em; }

@media (min-width: 64em) {
  :root { --font-size: 3em; }
}

Output:

.One { font-size: 2em; }
@media (min-width: 64em) { /* this do not override selector below */
  .One { font-size: 3em; }
}

.Two { font-size: 6em; }
hgl commented 9 years ago

This is pretty smart. I think it will work.

kevinSuttle commented 8 years ago

Check out this thread @MoOx @necolas

https://github.com/reworkcss/rework-vars/issues/15#issuecomment-30803384

MoOx commented 8 years ago

As you can see, a solution has been proposed, it's just nobody implemented it. Feel free to make a PR.

sanchan commented 7 years ago

is there any way to set variables for a given media-query from the variables options? Like:

variables: {
   '--color': 'blue',
   '@media only screen and (min-width : 600px)': {
     '--color': 'green'
   }
}
jonathantneal commented 6 years ago

I understand the proposed solution, so I’ll accept a PR that accomplishes this. Otherwise, I’ll be closing out this issue next week.