SE7ENSKY / group-css-media-queries

CSS postprocessing: group media queries. Useful for postprocessing preprocessed CSS files.
MIT License
100 stars 19 forks source link

Wrong order of media queries #4

Closed Narretz closed 9 years ago

Narretz commented 10 years ago

I posted this to the grunt repo, but it actually belongs here.

Original: https://github.com/Se7enSky/grunt-group-css-media-queries/issues/3

I have the following css:

@media (min-width: 992px) {
  .padded {
    padding: 0 20px;
  }
}
@media (min-width: 480px) {
  .button-small {
    padding: 0 12px 0 15px;
  }
}
@media (min-width: 992px) {
  .button-small {
    padding: 0 25px;
  }
}

Grouped, this ends up as:

@media (min-width: 992px) {
  .padded {
    padding: 0 20px;
  }

  .button-small {
    padding: 0 25px;
  }
}

@media (min-width: 480px) {
  .button-small {
    padding: 0 12px 0 15px;
  }
}

min-width: 480px must come before min-width: 920px, otherwise it overwrites it.

ivankravchenko commented 10 years ago

group-css-media-queries is meant to be totally indifferent on CSS cascading feature.

On our projects, we're using BEM and properly isolate from any edge cases that postprocessors can introduce.

If we introduce some ordering logic into library, this logic should be clear. It is not clear to me at this moment.

I think that ordering is a point to discuss. Comments are welcome.

roman-vabishchevych commented 10 years ago

I suggest this as a more semantically-correct. This should work and eliminate from further errors.

@media (min-width: 992px) {
  .padded {
    padding: 0 20px;
  }
}
@media (min-width: 480px) and (max-width: 991px) {
  .button-small {
    padding: 0 12px 0 15px;
  }
}
@media (min-width: 992px) {
  .button-small {
    padding: 0 25px;
  }
}
Narretz commented 10 years ago

True, when I always use the max form it works.

The thing is that when grouping is not order aware, it breaks previously working css, i.e. my cascading was correct and the grouping ignored it. It should be sufficient to keep order the media queries i ascending order, regardless of min / max. I tested it by extending the module and so far it seems to works as expected.

Btw, what is BEM?

ivankravchenko commented 10 years ago

BEM refers to block-element-modifier methodology. Modular, scalable and efficient for production. http://bem.info/method/ for more info.

Generally, I meant that high-quality CSS is order-insensitive, and BEM is not directly related to this.

tpalmer75 commented 10 years ago

Hi, I'm also experiencing this issue. My CSS is overriding itself because the media queries are placed incorrectly.

How is it ordering them currently? It appears to order them according to which media query has the most content, but there are a few out of place.

I would greatly appreciate any sort of help that can be given to order them properly.

ivankravchenko commented 10 years ago

Hello, @tpalmer75 Basic and most perfect recommendation is to write order-insensitive CSS. Could you show your relevant CSS fragments, we'll try to get you be clear about this.

tpalmer75 commented 10 years ago

See attached image (Sorry for large picture, retina screen )

I am writing my media queries inline with my less styles. This makes it much easier to write responsive styles. However, less compiles the styles and writes them all out separately. I was exciting to find your grunt package and thought it might solve my problem. It is perfect, except that they are out of order when I process my css.

image

ivankravchenko commented 10 years ago

I am sorry, could you please provide compiled CSS fragment (as text) and conflicting CSS property? It is slightly hard to guess what exactly CSS will be compiled and what property is unexpectedly overwritten.

ivankravchenko commented 10 years ago

Yeah, my guys also coding like:

.logo
  width 128px
  @media phone
    width 64px

That was the reason why this simple utility born :)

LeusMaximus commented 9 years ago

I want to support this issue. I have the same problem with combine media-queries after preprocessors.

ivankravchenko commented 9 years ago

Guys, we're refactored ordering. Who interested, please review https://github.com/SE7ENSKY/group-css-media-queries#media-queries-order and unit tests. Comments are welcome.

tpalmer75 commented 9 years ago

Cool! Thanks for the update, I'll check it out.

Narretz commented 9 years ago

Forgot to tell you that it works very well now. Thanks! 😃

ivankravchenko commented 9 years ago

Thank you!