Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested media queries problem #114

Closed
panec opened this issue Jan 7, 2016 · 6 comments
Closed

Nested media queries problem #114

panec opened this issue Jan 7, 2016 · 6 comments

Comments

@panec
Copy link

panec commented Jan 7, 2016

This is not an issue of the library, it is sass in general, but would be nice if tackling this problem would be possible. The following code:

// ----
// libsass (v3.3.2)
// ----
@import "include-media";

@include media(">tablet", "<=desktop") {
  width: 50%;
  @include media(">phone") {
  width: 60%;
  }
  @include media(">tablet") {
  width: 70%;
  }
}

Produces this:

@media (min-width: 769px) and (max-width: 1024px) {
  width: 50%;
}

@media (min-width: 769px) and (max-width: 1024px) and (min-width: 321px) {
  width: 60%;
}

@media (min-width: 769px) and (max-width: 1024px) and (min-width: 769px) {
  width: 70%;
}

When the expected outcome would be:

@media (min-width: 769px) and (max-width: 1024px) {
  width: 50%;
}

/* 'and (min-width: 321px)' was left out as in not in a scope of parent */
@media (min-width: 769px) and (max-width: 1024px) {
  width: 60%;
}

/* 'and (min-width: 769px)' was left out as it duplicates already existing query */
@media (min-width: 769px) and (max-width: 1024px) {
  width: 70%;
}

If that can be achieved with another library as a additional step of post processing I will be grateful for suggestions.

@KittyGiraudel
Copy link
Collaborator

I believe we could add further checking to make sure the generated media queries actually make sense but I am not eager to do it for simplicity reasons. I strongly believe it is not the responsibility of include-media to make sure the generated media queries actually match anything.

Maybe a plugin then.

@nirazul
Copy link

nirazul commented Jan 7, 2016

I've yet to come across a real case, where this sort of nested media queries really makes sense.
If you have full control over your stylesheet, this will never be needed, as you can abstract anything into a range between two breakpoints. On the contrary, the example is ambiguous as I would have thought that the inner statements overwrite the outer ones because that's how the cascade works.

@panec
Copy link
Author

panec commented Jan 7, 2016

@nirazul I can agree that this example is ambiguous, as I'm understanding it differently, as the browsers are computing it - in a way that all conditions separated by and need to me met.

Another good example of it is this:

@media (min-width: 769px) and (max-width: 1024px) and (max-width: 321px) {
  width: 60%;
}

That will never be applied as max-width: 321px + min-width: 769px exclude themselves. In this case that rule should be removed from the css.

For the case in which it might occur is quite simple. Imagine that you got a mixin that got some include-media call inside and nest it inside another include-media closure. You do not have the visibility what is happening until the end css will be generated.
You can parametrise the mixin to allow generating code for each media query inside it but it will be prone to error and hard to maintain.

I agree with @hugogiraudel as it should be a part of another plugin / post generation step thou.

@drazik
Copy link
Contributor

drazik commented Jan 8, 2016

I think that instead of "rewrite" your media query if it contains some parts that exclude themselves, it should just warn you about it. Because it you end with such excluding media query, I bet it's because you made a mistake. And in this case, it must be a plugin.

@panec
Copy link
Author

panec commented Jan 15, 2016

To solve the issue that was mentioned I created a separate plugin as @hugogiraudel suggested. It is in PostCSS https://github.com/panec/postcss-mq-optimize

@eduardoboucas
Copy link
Owner

That's pretty cool, thanks for that. I'll close this then! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants