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

Remove only screen and from @media inside reflex order helpers #65

Closed
Yoann-M opened this issue Feb 27, 2021 · 3 comments · Fixed by #67
Closed

Remove only screen and from @media inside reflex order helpers #65

Yoann-M opened this issue Feb 27, 2021 · 3 comments · Fixed by #67

Comments

@Yoann-M
Copy link
Contributor

Yoann-M commented Feb 27, 2021

Hello 👋

First of all, many thanks for your work ... I ❤️ this CSS library.
I used it a lot and still today in many projects in production 🚀

I have a question regarding media queries:

Is there a specific reason to use @media only screen and (min-width: $reflex-xs) for "reflex order helpers generation" instead of @media (min-width: $reflex-xs).

@media only screen and (min-width: $reflex-xs) {
@include loop-order-helpers($reflex-columns, '-xs');
}

❔ I'm asking you this question because when we/you use a plugin like gulp-group-css-media-queries we have declacrations redundant @media only screen and (min-width... and @media (min-width... instead of letting the tool merge all into@media (min-width... due to the only screen and keywords.

💡 The idea behind (if there is no specific reason), it would be to homogenize @media by removing only screen and (or addingonly screen and to all others) in order to have a lighter built CSS .

Best Regards
Yoann

@leejordan
Copy link
Owner

I think my intention here was not to apply the grid to any other media type such as print for example. Is there any risk involved in not specifying screen ?

@Yoann-M
Copy link
Contributor Author

Yoann-M commented Mar 4, 2021

Personally, I don't think there is a risk of not specifying screen.
Using print is only for very specific cases that can be taken into account in the CSS of the project in which Reflex is imported.

I took a look at

  • Foundation (for sites) : @media print, screen and (min-width: 40em) { -> print, screen are always declared
  • Bootstrap : @media (min-width: 1200px) { -> nothing specific is declared
  • UiKit : @media (min-width: 1200px) { -> nothing specific is declared
  • MaterialiseCSS : @media only screen and (min-width: 1200px) { -> only screen and are always declared

After some research W3C Media Types and W3C Media Queries

media (max-width:1200px) -> Is for a window with a max-width of 1200px.

@media screen and (max-width:1200px)-> Is for a device with a screen and a window with a max-width of 1200px.

@media only screen and (max-width:1200px) -> For only, there is a quote from W3C :

The keyword ‘only’ can also be used to hide style sheets from older user agents. User agents must process media queries starting with ‘only’ as if the ‘only’ keyword was not present. (W3C Recommendation 19 June 2012)

The idea behind my proposal is to homogenize all the @media.
So we can use for all @media (min-width: 1200px) { or @media screen and (max-width:1200px)

Tell me what you think about.
Best Regards
Yoann

@leejordan
Copy link
Owner

Thanks for explaining it. I agree with your proposal and your reasons. I think we can follow the majority and use @media (min-width: 1200px)

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

Successfully merging a pull request may close this issue.

2 participants