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

Combined Media Queries #16345

Closed
wants to merge 1 commit into from
Closed

Combined Media Queries #16345

wants to merge 1 commit into from

Conversation

gotbahn
Copy link

@gotbahn gotbahn commented Apr 23, 2015

Bootstrap uses a lot of repeated media queries, this is proposal to combine them.

I found in issue tracker quite similar, approved pull request "Media query mixins" #13014. But it only changes hardcoded media queries to mixins (in my opinion not in better way to write code inside) and what is more important not combining media queries in output CSS.

So inside proposal changed all media queries to media mixins declarations, that called in media-queries.less file collector.

Media mixins naming composed of parts from media queries:
@media screen and (min-width: @screen-sm-min) {...} -> .media-screen-min-sm() {...}

Benefits:

  • combined media queries;
  • reduced little bit size of bootstrap.css from 3kb, bootstrap.min.css from 2.5kb
  • mechanism to scale media queries for LESS Bootstrap users without adding post processors

Concerns:

  • non standard way of use mixins possible only in LESS
  • CSS way to write media queries, media mixins should always wrap content
  • I founded that bootstrap not supported LESS extends, so didn't find way better than add .navbar-form to .form-inline. Of course form.less should not know anything about navbar.less.

- Change media queries to mixins
- Add media queries collector file
@cvrebert cvrebert added the css label Apr 23, 2015
@cvrebert
Copy link
Collaborator

I don't think we're interested in making a significant change like this to Bootstrap v3 at this point. We're trying to nudge v3 towards stable maintenance mode.
FWIW, we have implemented something similar to #13014 in our draft of Bootstrap v4.
With regard to combining media queries, I'm pretty sure we rely on declaration order in some places to resolve specificity issues, so I'd be concerned about potential breakage related to the reordering caused by the combining.

@mdo
Copy link
Member

mdo commented Apr 23, 2015

Yup, few things about this kind of stuff:

  • It's a big change to make in v3.
  • It's a task and approach better served via post-processing as the closer these things are to their original styles, the better.
  • File size doesn't matter much here because of gzipping—those repeated strings get sampled down to one string.
  • Performance wise, I've seen no benefit for combined vs separate media queries (not that you raised this concern, but it's worth noting.

So, given all that, we'll punt for now. Come v4 we'll have mixins for the media queries for easier syntax, but they'll still be close to the components that they modify. Way easier to build stuff that way.

@mdo mdo closed this Apr 23, 2015
@gotbahn
Copy link
Author

gotbahn commented Apr 23, 2015

@cvrebert, @mdo I see, make sense. Didn't expect this for v3, more for future, just live working example.
Thank you for detailed responses.

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

Successfully merging this pull request may close these issues.

3 participants