Investigate support for media-query scoped properties #9

Open
necolas opened this Issue Sep 28, 2014 · 13 comments

Projects

None yet

4 participants

@necolas
Member
necolas commented Sep 28, 2014

See: reworkcss/rework-vars#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:

  • inlined @import statements in media queries
  • 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)?)
  • generating large amounts of extra CSS
  • specificity/cascade issues - moving/generating rules is always going to hit the problem where earlier styles are unintentionally overridden.
<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
Member
MoOx commented Sep 29, 2014

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
Member
MoOx commented Sep 30, 2014

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
Member
necolas commented Sep 30, 2014

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
Member
MoOx commented Sep 30, 2014

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
Member
necolas commented Sep 30, 2014

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.

@necolas necolas closed this Sep 30, 2014
@MoOx MoOx added a commit that referenced this issue Oct 1, 2014
@MoOx MoOx Update README according to #1 & #9 6d4fad5
@hgl
Contributor
hgl commented Apr 10, 2015

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
Contributor
hgl commented Apr 10, 2015

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:

  • js defined custom properties only apply to the top level

  • js defined custom properties must be defined like this

    {
      "": { "width": "1em" },
      "(width >= 960px)": { "width": "2em" }
    }

    Media queries must match exactly for the custom properties to be applied, making things simple.

  • custom properties are extracted as above

@MoOx
Member
MoOx commented Apr 10, 2015

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
Contributor
hgl commented Apr 10, 2015

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
Member
MoOx commented Apr 26, 2015

@necolas what do you think about the solution below (injecting rules after the selector using the custom prop) ?
Someone opened an issue on cssnext #MoOx/postcss-cssnext#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; }
@MoOx MoOx reopened this Apr 30, 2015
@hgl
Contributor
hgl commented Apr 30, 2015

This is pretty smart. I think it will work.

@MoOx
Member
MoOx commented Aug 3, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment