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 using the "not" keyword should not be merged #1831

Closed
1 of 2 tasks
jaydenseric opened this issue Sep 9, 2015 · 7 comments
Closed
1 of 2 tasks
Labels
CSS compatibility Support the CSS spec enhancement New feature or request

Comments

@jaydenseric
Copy link

jaydenseric commented Sep 9, 2015

Edited by @nex3

Specs: sass/sass-spec#1267


Nested media queries using the not keyword should not be merged. According to the spec, not flips the result for the entire query and does not apply to specific chunks.

For example:

@media (min-width: 500px) {
  @media not all and (min-width: 800px) {
    background: red;
  }
 }

Should compile to the same:

@media (min-width: 500px) {
  @media not all and (min-width: 800px) {
    background: red;
  }
 }

At 400px the background is expectedly not red.

Instead it compiles to:

@media not all and (min-width: 500px) and (min-width: 800px) {
  background: red;
}

At 400px the background is unexpectedly red.

@nex3 nex3 added bug Something isn't working help wanted Extra attention is needed labels Sep 11, 2015
@chriseppstein
Copy link

I had the thought "Since CSS allows nested media queries now, I wonder whether it's better to just remove the media merging feature from Sass." Then I discovered IE11 still doesn't support nested media queries. (IE9-11 is 8% of global users)

Noting that here in case someone else thinks it.

@jaydenseric
Copy link
Author

Wow! Didn't know that about IE11 either.

@andrew-skybound
Copy link

Since browser support for nested @media queries is not ubiquitous, how about requiring the "not" keyword to appear on either all media queries in a rule (if the parent has it) or on none of them (if the parent does not). That would make the intent explicit, and future-proof the style sheet for the day when manually nesting @media rules is no longer necessary.

Not nesting media queries sometimes does not seem like a good idea. At the very least it will lead to unexpected failures on IE.

So the example above would be an error. This would be required:

@media not all and (min-width: 500px) {
  @media not all and (min-width: 800px) {
    background: red;
  }
}

@nex3
Copy link
Contributor

nex3 commented Apr 6, 2018

Note that this question came up again in sass/libsass#2425. The current cross-implementation behavior is to omit nested queries, but as more browsers natively support nested media queries this becomes a CSS compatibility question. I think going forward we should probably move to leaving nested queries as-is when they can't be successfully merged.

@nex3 nex3 added enhancement New feature or request CSS compatibility Support the CSS spec and removed bug Something isn't working labels Apr 6, 2018
@nex3
Copy link
Contributor

nex3 commented Jul 13, 2018

This is a straightforward enough change that I don't think it needs a full proposal. @xzyfer agreed?

@xzyfer
Copy link

xzyfer commented Jul 13, 2018 via email

nex3 added a commit to sass/sass-spec that referenced this issue Jul 18, 2018
This also adds some additional media query merging specs in places
where they were lacking.

See sass/sass#1831
nex3 added a commit to sass/dart-sass that referenced this issue Jul 18, 2018
nex3 added a commit to sass/dart-sass that referenced this issue Jul 18, 2018
nex3 added a commit to sass/sass-spec that referenced this issue Jul 18, 2018
)

This also adds some additional media query merging specs in places
where they were lacking.

See sass/sass#1831
@nex3 nex3 removed the help wanted Extra attention is needed label Oct 12, 2018
@nex3
Copy link
Contributor

nex3 commented Nov 5, 2020

Closing this out since LibSass is deprecated.

@nex3 nex3 closed this as completed Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS compatibility Support the CSS spec enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants