Media Query scoping

[b-m-f]

Hi,

I wanted to use the functionality described in https://github.com/postcss/postcss-custom-properties/issues/9 .

I came up with this POC.

Works for me with importFrom. Only for CSS files for now.

If you like the general idea I can keep working on this.

Basically the mediaquery specific rules will be saved along with the customProperties. If any of var()s parents matches any of those mediaqueries the customProperties will be merged and the transformations applied.

[b-m-f]

I have added the test and improved the code.

Hope the docs are fine as well.

[b-m-f]

Also, I was thinking to kill the extra mediaquery tests and just add a few new ones to the basic ones that check correct overwriting of values.

Good idea?

[jonathantneal]

It looks much better. I looked it over to follow along and infer the new syntax. I’ll look again later today.

[b-m-f]

Hi @jonathantneal .

I have merge your changes and updated the tests. All green.

[jonathantneal]

Thanks, @b-m-f. I came down with a bad flu, so I may not respond to this for a while. Please ping me at the end of the week if you haven’t heard back from me.

[b-m-f]

@jonathantneal . Oh, no probs. Get well soon 👍🏻

[b-m-f]

Hi @jonathantneal,

just merged in your latest changes with the z-index.

[endel]

Hi guys! This is exactly what I'm looking for. Any updates here?

[endel]

Does this pull-request work along with postcss-calc? I've just tried it out and, apparently, it doesn't. I had to duplicate my CSS rules inside @media in order to have it recalculated for my media queries. Cheers

[b-m-f]

Hi @endel,

I can take a look at that.

Do you have some code to replicate it?

[silvenon]

Hey @b-m-f, I'm really stoked about this PR and thanks so much for adding this advanced and much needed feature!

I really want to try it out and see if I can break it (also to confirm that it does work with postcss-calc), but I'm having trouble linking this. I cloned this repo, checked out your PR, ran yarn pretest to generate index.cjs.js file and ran yarn link.

In my demo repository I ran yarn link postcss-custom-properties and set up a really minimal imports.css to try out the media query, but it doesn't work, it compiles the old way, ignoring the media query. (I set up yarn start, a helper script which compiles and input.css and watches for changes.)

I feel like I made a silly mistake somewhere, but I just can't figure out what it is.

[b-m-f]

Hi @silvenon ,

try adding a Media Query rule in your input.css. Sth like

@media (min-width: 100px) { 
  .foo {
    border-radius: var(--border-radius);
    margin: var(--margin);
  }
}

If the variable is not used in the code than it wont be included. The matching of the rules is done by checking if code with a similar Media Query exists.

[silvenon]

Ah, that's how it works, thanks! 😅

It works well with postcss-calc, but it's important to list postcss-calc after postcss-custom-properties in the configuration. Otherwise calc won't be able to do its thing because var() didn't compile yet.

What about automatically generating a media query for every selector that uses a variable using instead of having to write one yourself in the input code. Is that not a good idea or too complicated?

[silvenon]

For example, this:

:root {
  --margin-bottom: 1rem;
}

@media (min-width: 40rem) {
  :root {
    --margin-bottom: 2rem;
  }
}

.header {
  margin-bottom: var(--margin-bottom);
}

could compile to this:

.header {
  margin-bottom: 1rem;
  margin-bottom: var(--margin-bottom);
}
@media (min-width: 40rem) {
  .header {
    margin-bottom: 2rem;
    margin-bottom: var(--margin-bottom);
  }
}

I could try sending a PR for this after yours gets merged.

[b-m-f]

Hm, the only problem I see is if you would not like to use the MediaQuery every time.

What about this:

:root {
  --margin-bottom: 1rem;
}

@media (min-width: 40rem) {
  :root {
    --margin-bottom: 2rem;
  }
}

.header {
  margin-bottom: var(--margin-bottom);
}

.footer {
  margin-bottom: var(--margin-bottom);
}

This would create the following:

.header {
  margin-bottom: 1rem;
  margin-bottom: var(--margin-bottom);
}

.footer {
  margin-bottom: 1rem;
  margin-bottom: var(--margin-bottom);
}

@media (min-width: 40rem) {
  .header {
    margin-bottom: 2rem;
    margin-bottom: var(--margin-bottom);
  }

.footer {
    margin-bottom: 2rem;
    margin-bottom: var(--margin-bottom);
  }
}

The problem would be if Id not like to change the margin-bottom of either the footer or header.

[b-m-f]

@jonathantneal Just another ping :) Also maybe you see @silvenon `s suggestion from another perspective?

[silvenon]

I see, I overlooked that specificity issue, never mind.

[silvenon]

Huh, I just checked and media queries do not increase specificity. If you always insert a media query right after every selector containing var(), overriding should be fine.

@media (min-width: 40rem) {
  .header {
    margin-bottom: 2rem;
  }
}
.header {
  margin-bottom: 1rem; /* this wins, simply based on order */
}

I just learned something new.

Was specificity the problem you were worried about?

[b-m-f]

Hm, I dont see the benefit yet tbh. Does this not make the rule obsolete then?

I just wouldnt want to have unintended rules created automatically so that the resulting codebase would increase without developer control.

The benefit of automatically creating it would be that you only define the vars in media queries and this would propagate throughout the code wherever that var is used, correct?

[silvenon]

Yeah, it's not within the scope of this PR, but I wanted to hear opinions.

But it would probably be too much hassle over only partial gain. PostCSS can only operate on a per-file basis, so it couldn't be a complete polyfill.

[jonathantneal]

Hi all, I was excited for this feature, but I hadn’t had a chance to dive into the code to understand how the fallbacks were being written. I expect the feature would work like this:

:root {
  --font-size-1: 16px;
}

@media (min-width: 960px) {
  :root {
    --font-size-1: 22px;
  }
}

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

/* becomes */

:root {
  --font-size-1: 16px;
}

@media (min-width: 960px) {
  :root {
    --font-size-1: 22px;
  }
}

body {
  font-size: 16px;
  font-size: var(--font-size-1);
}

@media (min-width: 960px) {
  body {
    font-size: 22px;
    font-size: var(--font-size-1);
  }
}

First, I’m not certain this PR handled the case I’ve listed above as I expected. Did it, @b-m-f?

But now that I know how sensitive custom properties are to media query ordering, we would need to introduce logic to allow the destruction of custom properties by media query when a new top-level :root potentially overwrites it. This also affects how we should guide the custom property JSON markup.

[b-m-f]

Hi @jonathantneal,

I could rework this, to make it work with the example given. Will need some time though.

I am not quite sure I understand what you mean with

But now that I know how sensitive custom properties are to media query ordering, we would need to introduce logic to allow the destruction of custom properties by media query when a new top-level :root potentially overwrites it. This also affects how we should guide the custom property JSON markup.

though.

[jonathantneal]

Basically, if a Custom Property inside of a Media Query is followed by a Custom Property outside of a Media Query, it makes the previous Media Query fully obsolete.

This also affects how we should guide the custom property JSON markup.

We allow properties to be passed in via importFrom, and they appear like so:

:root {
  --brand-color: #1138aa;
}

{
  "custom-properties": {
    "--brand-color": "#1138aa"
  }
}

How would these look when supporting Custom Properties within Media Queries?

:root {
  --brand-color: #1138aa;
}

@media (min-width: 960px) {
  :root {
    --brand-color: #1337bb;
  }
}

Without thinking deeply about it, I would imagine they would look like this:

{
  "custom-properties": {
    "--foo": [
      "#1138aa",
      {
        "media": "(min-width: 960px)",
        "value": "#1337bb"
      }
    ]
  }
}

[b-m-f]

@silvenon @jonathantneal This will take quite a bit of rewriting I think. I am starting a new Job this week and dont think Ill be able to manage working on this at the same time.

Maybe you want to take over @silvenon ? If so I am happy to answer any questions. Otherwise I cant estimate when Ill get around to finishing this. Probably not before december.

[silvenon]

I'm also a bit swamped, but I'd like to take over, yes. It will just take me a bit of time.

[jonathantneal]

This aspirational work has become stale so I am closing this PR. I’m sorry to do it, and I appreciate the work that was put into it.

[dangelion]

Any news on this? It would be very very useful