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

Unified Diff: README.md

Issue 29441585: No Issue - SCSS Media Query Style Proposal (Closed)
Patch Set: Created May 18, 2017, 10:55 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « .hgignore ('k') | gulpfile.js » ('j') | gulpfile.js » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: README.md
===================================================================
new file mode 100644
--- /dev/null
+++ b/README.md
@@ -0,0 +1,39 @@
+# SCSS Media Query Style Proposal
+
+## Background
+
+While working on Issue 5135, I wrote the responsive styles in this way:
+
+```
+.h1
+{
+ margin: $xl 0 $lg 0;
+ font-size: $font-size-h1;
+
+ @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.
+ {
+ font-size: $font-size-h1 - 10px;
+ margin: $md 0 $lg 0;
+ }
+}
+```
+
+Julian asked me to make my case for this method, as opposed to the current method of one media query per file and having the responsive styles for each selector defined within. So, here it is.
+
+
+## Pros
+
+- 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 scattered across different files or in different places of the same file. This also helps 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.
+
+- It's easier to debug. Following on from my previous point, it's easier to debug styles related to a selector if they are all defined in one place.
+
+- It's consistent with the concept of "encapsulated modules". Instead of grouping styles by breakpoints, we group styles by component. This makes is easier to share components across projects.
+
+- 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 instead 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 larger screen sizes.
+
+
+## Cons & Workarounds
+
+- The major issue with this method is that, by default, it ends up outputting more 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-mqpacker](https://github.com/hail2u/node-css-mqpacker). This plugin consolidates media queries so we don't get duplicates, see the `css/packed.css` file. Something 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
+
+- 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 we 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.
« no previous file with comments | « .hgignore ('k') | gulpfile.js » ('j') | gulpfile.js » ('J')

Powered by Google App Engine
This is Rietveld