Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Side by Side Diff: README.md

Issue 29441585: No Issue - SCSS Media Query Style Proposal (Closed)
Patch Set: Created May 18, 2017, 10:55 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « .hgignore ('k') | gulpfile.js » ('j') | gulpfile.js » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
(Empty)
1 # SCSS Media Query Style Proposal
2
3 ## Background
4
5 While working on Issue 5135, I wrote the responsive styles in this way:
6
7 ```
8 .h1
9 {
10 margin: $xl 0 $lg 0;
11 font-size: $font-size-h1;
12
13 @media (max-width: $tablet-breakpoint)
juliandoucette 2017/05/22 11:20:09 Note: CSS3 does support nested media queries. CSS2
ire 2017/05/23 00:03:49 Noted. I wasn't actually aware that CSS3 supported
juliandoucette 2017/05/24 11:44:18 Acknowledged.
14 {
15 font-size: $font-size-h1 - 10px;
16 margin: $md 0 $lg 0;
17 }
18 }
19 ```
20
21 Julian asked me to make my case for this method, as opposed to the current metho d of one media query per file and having the responsive styles for each selector defined within. So, here it is.
22
23
24 ## Pros
25
26 - It's, IMO, easier to read and understand. I think it's better to have all the styles for a class defined in one place, close together, rather than being scatt ered across different files or in different places of the same file. This also h elps to make sure you don't accidentally have duplicate styles, because you can see all the styles related to this one element in one place.
27
28 - It's easier to debug. Following on from my previous point, it's easier to debu g styles related to a selector if they are all defined in one place.
29
30 - It's consistent with the concept of "encapsulated modules". Instead of groupin g styles by breakpoints, we group styles by component. This makes is easier to s hare components across projects.
31
32 - It outputs cleaner CSS (IMO anyway). Particularly if we follow a "mobile-first " method for defining our styles, i.e. using `min-width` based media queries ins tead of `max-width` (and define them in the correct order). Styles will be laid out in a nice logical order, from mobile styles to styles for progressively larg er screen sizes.
33
34
35 ## Cons & Workarounds
36
37 - The major issue with this method is that, by default, it ends up outputting mo re CSS because each media query is duplicated. For example, see the `css/default .css` file/. However, this can be resolved with a postcss plugin I found, [css-m qpacker](https://github.com/hail2u/node-css-mqpacker). This plugin consolidates media queries so we don't get duplicates, see the `css/packed.css` file. Somethi ng to note with this is that declarations will be reordered a bit since they are packed together. In my experience, this shouldn't be a problem since styles are still roughly in the same order as defined.
juliandoucette 2017/05/22 11:20:09 Good find. It looks like this also works with sour
ire 2017/05/23 00:03:49 Thanks!
Thomas Greiner 2017/05/23 12:17:49 I can imagine that reordering CSS rules could caus
saroyanm 2017/05/23 14:55:24 You mean that compression fix duplication ? Or th
ire 2017/05/23 17:20:11 Yes there could be issues here, but since we know
Thomas Greiner 2017/05/23 18:30:35 Since compression usually works quite well with re
juliandoucette 2017/05/24 11:44:18 I created a proof of concept where the order of me
ire 2017/05/24 13:06:27 I think we may be talking about different things.
ire 2017/05/24 13:06:27 Yes I totally agree that there are cases where ord
juliandoucette 2017/05/30 10:10:47 I would like to discuss mqpacker in a separate iss
38
39 - Another potential con with this method is potentially ending up with a larger variety of media queries. This shouldn't be much of a problem for us, as I see w e already use standard breakpoints abstracted into SCSS variables to keep things consistent.
juliandoucette 2017/05/22 11:20:09 Another point: See https://helloanselm.com/2014/w
ire 2017/05/23 00:03:49 I haven't been able to find anything about this. M
juliandoucette 2017/05/24 11:44:17 Acknowledged.
OLDNEW
« no previous file with comments | « .hgignore ('k') | gulpfile.js » ('j') | gulpfile.js » ('J')

Powered by Google App Engine
This is Rietveld