postcss / autoprefixer

Parse CSS and add vendor prefixes to rules by Can I Use
https://twitter.com/autoprefixer
MIT License
21.63k stars 1.26k forks source link

[css-grid] move media query cell placements to come after cell styles #1048

Closed Dan503 closed 6 years ago

Dan503 commented 6 years ago

Currently if I write this:

/* How I would like to structure my scss file */

.grid {
  display: grid;
  grid-template-columns: repeat(2, 1fr);
  grid-template-areas:
    "a   b"
    "c   d";

  // Placing media query close to the other .grid code
  @media (min-width: 600px){
    grid-template-areas:
      "a   b   c   d";
    grid-template-columns: repeat(4, 1fr);
  }

  &__cell {
    &--a {
      grid-area: a;
    }
    &--d {
      grid-area: d;
    }
  }
}

auto-prefixer outputs this:

/* How Autoprefixer outputs that code */

.grid {
 display: -ms-grid;
 display: grid;
     -ms-grid-columns: (1fr)[2];
     grid-template-columns: repeat(2, 1fr);
     grid-template-areas: "a   b"
"c   d";
}

@media (min-width: 600px) {
 .grid {
       grid-template-areas: "a   b   c   d";
   -ms-grid-columns: (1fr)[4];
   grid-template-columns: repeat(4, 1fr);
 }
 .grid__cell--a {
   -ms-grid-row: 1;
   -ms-grid-column: 1;
 }
 .grid__cell--d {
   -ms-grid-row: 1;
   -ms-grid-column: 4;
 }
}

.grid__cell--a {
 -ms-grid-row: 1;
 -ms-grid-column: 1;
 grid-area: a;
}
.grid__cell--d {
 -ms-grid-row: 2;
 -ms-grid-column: 2;
 grid-area: d;
}

The issue is that the media query code is being overwritten by the default styles so the media query ends up having no effect in IE.

You can write the input code like this instead which fixes the issue:

/* How I have to structure my code to not cause the issue */

.grid {
  display: grid;
  grid-template-columns: repeat(2, 1fr);
  grid-template-areas:
    "a   b"
    "c   d";

  &__cell {
    &--a {
      grid-area: a;
    }
    &--d {
      grid-area: d;
    }
  }

  // Placing media query AFTER the cell styles
  @media (min-width: 600px){
    grid-template-areas:
      "a   b   c   d";
    grid-template-columns: repeat(4, 1fr);
  }
}

However it means you have to write half of your .grid styles at the top of the scss file and write the other half of the styles at the bottom. It is much nicer having them all centralised in one location.

I'd prefer to have the autoprefixer output for the first example to be this:

/* How I would like Autoprefixer to output the code from the 1st example */

.grid {
 display: -ms-grid;
 display: grid;
     -ms-grid-columns: (1fr)[2];
     grid-template-columns: repeat(2, 1fr);
     grid-template-areas: "a   b"
"c   d";
}

@media (min-width: 600px) {
 .grid {
       grid-template-areas: "a   b   c   d";
   -ms-grid-columns: (1fr)[4];
   grid-template-columns: repeat(4, 1fr);
 }
}

.grid__cell--a {
 -ms-grid-row: 1;
 -ms-grid-column: 1;
 grid-area: a;
}
.grid__cell--d {
 -ms-grid-row: 2;
 -ms-grid-column: 2;
 grid-area: d;
}

@media (min-width: 600px) {
 .grid__cell--a {
   -ms-grid-row: 1;
   -ms-grid-column: 1;
 }
 .grid__cell--d {
   -ms-grid-row: 1;
   -ms-grid-column: 4;
 }
}
ai commented 6 years ago

cc @yepninja

yepninja commented 6 years ago

@Dan503 We can fix it by adding a media query to each selector with grid-area. Then you will need to use https://github.com/hail2u/node-css-mqpacker to merge media queries.

Dan503 commented 6 years ago

I've used media query merging software before, they do far more harm than good :(

Is it not possible to save them into an array then output them at the end into one media query block?

I don't really like the idea of each one getting wrapped in a separate media query :(

yepninja commented 6 years ago

It's possible too. To my mind, we have 3 solutions for this case:

  1. Duplicate query for each area
  2. Duplicate query after the last area
  3. Append everything to parent query, then move this query after last area (or to the end of all CSS)
Dan503 commented 6 years ago

Duplicate query for each area

Creates way too much extra junk in the css file.

Duplicate query after the last area

That is the option I think works best. I think that this is what I requested in the issue isn't it?

Append everything to parent query, then move this query after last area (or to the end of all CSS)

Too much risk of specificity breaking due to drastic changes in the source order.

yepninja commented 6 years ago

I think, if we want insert media after the last area, the third solution would be the best.

Dan503 commented 6 years ago

.... maybe.

There is a bit of risk in that the media query is going to be holding more than just grid styles in it.

Moving site styles away from where the author wrote them is a bit dangerous.

Moving an extra media query that was generated through autoprefixer that only holds IE prefixes is a lot less risky in my mind.

Dan503 commented 6 years ago

Yeah it's not as neat and tidy but shifting an authors styles around scares me a bit.

yepninja commented 6 years ago

Agreed with you. It's not right to change authors styles. Let's try to implement the second solution.

Dan503 commented 6 years ago

@yepninja this is a high priority issue that should get fixed before the article is published.

ai commented 6 years ago

@yepninja do you need help with it from Cult of Martians?

yepninja commented 6 years ago

@ai Let's try

ai commented 6 years ago

@yepninja I will create a task tomorrow. Can you write on Russian current plan?

yepninja commented 6 years ago

Yep, I'll write plan today

yepninja commented 6 years ago
  1. Ознакомиться с обсуждением задачи
  2. Форкнуть Autoprefixer
  3. Поправить или добавить тесты в файлах:
  4. Поменять логику переопределения позиций областей в функции insertAreas
ai commented 6 years ago

@yepninja а что именно сделать надо?

yepninja commented 6 years ago

Нужно поменять логику переопределения областей для media. Сейчас новые позиции областей добавляются к медиа-выражению, где было переопредлено grid-template-areas, соответсвенно если это медиа-выражение указаны вышо каких-то областей, то переопределние не стработает. Поэтому нужно дублировать медиа-выражение с переопределениями, и вставлять его всего после последней области.

ai commented 6 years ago

https://cultofmartians.com/tasks/autoprefixer-grid-media.html

yepninja commented 6 years ago

Released in 8.6.1