No Issue - Implemented first run page

skin/firstRun.css

Index: skin/firstRun.css
===================================================================
--- a/skin/firstRun.css
+++ b/skin/firstRun.css
@@ -11,509 +11,239 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 @font-face {
-  font-family: "CreteRound";
+  font-family: 'Source Sans Pro';
   font-style: normal;
-  src: url(fonts/CreteRound-Regular.otf);
+  font-weight: 300;
+  font-stretch: normal;
   src: local ("Ø"),

[juliandoucette, 2017/10/08 13:18:12]

NIT: What the?

[martin, 2017/10/16 09:09:09]

My reaction exactly. It was like this in the original source, so I decided to
keep it that way. Let me know what it does.

[saroyanm, 2017/10/16 10:22:59]

It's used to force not using local font, I think the previous comments applies
here as well.
https://www.paulirish.com/2009/bulletproof-font-face-implementation-syntax/#s...

-  /*local ("Ø") forces using no local font called CreteRound*/
-  url(fonts/CreteRound-Regular.otf) format("otf");
+  url(fonts/SourceSansPro-Light.woff) format("otf");
 }
 
 @font-face {
-  font-family: "CreteRound";
-  font-style: italic;
-  src: url(fonts/CreteRound-Italic.otf);
+  font-family: 'Source Sans Pro';
+  font-style: normal;
+  font-weight: 400;
+  font-stretch: normal;
   src: local ("Ø"),
-  /*local ("Ø") forces using no local font called CreteRound*/
-  url(fonts/CreteRound-Italic.otf) format("otf");
-}
-
-body
-{
-  font-family: Helvetica, Arial, sans-serif;
-  font-size: 15px;
-  line-height: 140%;
-  color: #7f776b;
-  background-color: #f8f6f2;
-  background-image: url(background-main.png);
-  margin: 0;
-  padding: 0;
-}
-
-a, a:link, a:visited
-{
-  color: #5a84b3;
-  text-decoration: underline;
-}
-
-a:hover
-{
-  text-decoration: none;
-}
-
-button
-{
-  cursor: pointer;
-}
-
-ul
-{
-  margin: 0;
-  padding: 0;
-}
-
-li
-{
-  margin: 0;
-  padding: 0;
-  list-style-type: none;
+  url(fonts/SourceSansPro-Regular.woff) format("woff");
 }
 
-header
-{
-  background-image: url(background.png);
-  background-repeat: repeat-x;
-  width: 100%;
-  padding-top: 25px;
-}
-
-header h1
-{
-  font-size: 24px;
-  font-weight: normal;
-  color: #57ab5b;
-  text-align: center;
-  margin: 21px auto;
-  padding: 16px 0px;
-  border: 1px #57ab5b;
-  border-style: dashed none;
-  /* border parallel fix - 957px is the value
-   * closest to 960px(page width) which makes
-   * the dashed border parallel
-   */
-  max-width: 957px;
+@font-face {
+  font-family: 'Source Sans Pro';
+  font-style: normal;
+  font-weight: 700;
+  font-stretch: normal;
+  src: local ("Ø"),
+  url(fonts/SourceSansPro-bold.woff) format("woff");
 }
 
-h1,
-h2,
-h3
-{
- font-family: CreteRound, Helvetica, Arial, sans-serif;
-}
 
-h2
-{
-  font-size: 26px;
-  font-weight: lighter;
-  color: #968d81;
-  line-height: 28px;
-  text-align: center;
-  margin: 0;
-  padding: 0;
-}
-
-h3
-{
-  font-size: 22px;
-  color: #7F776B;
-  font-weight: normal;
-  margin-top: 0px;
-  margin-bottom: 10px;
-  padding: 0;
-  line-height: 24px;
-}
-
-h4
-{
-  font-size: 15px;
-  color: #7f776b;
-  font-weight: normal;
-  text-align: center;
+body {
+  font-family: 'Source Sans Pro', Helvetica, Arial, sans-serif;
+  font-size: 16px;

[juliandoucette, 2017/10/08 13:18:11]

NIT: I prefer to set document wide styles on the root (html) element.
(Acknowledging that it doesn't make a difference to this CSS e.g. because you
are not using rem.)

[martin, 2017/10/16 14:36:00]

Done.

+  background-color: white;

[juliandoucette, 2017/10/08 13:18:11]

NIT/Reminder: "CSS color values should be specified in hexadecimal where
possible." (also, "#fff" is fewwer characters than "white" :P)

[martin, 2017/10/16 09:09:09]

Done.

   margin: 0;
   padding: 0;
 }
 
-section
-{
-  margin: 0 auto;
-  margin-bottom: 30px;
-  max-width: 960px;
-  background-image: url(background.png);
-  padding: 40px 100px;
+.container {
+  width: 100%;
+  max-width: 100%;
+  margin: 0px auto;
+  padding: 0px 1em; 
+}
+
+header {

[juliandoucette, 2017/10/08 13:18:11]

NOTE: Padding inside the .content body causes this to be offcenter ~400px

+  text-align: center;
+  background: #f9f9f9;
+  padding: 4em 0;
+  margin-bottom: 4em;
+}
+
+@media (min-width: 768px) {
+  .container {
+    width: 720px; 
+  } 
+}
+
+@media (min-width: 992px) {

[juliandoucette, 2017/10/08 13:18:11]

NIT: This doesn't make sense.

- You are setting a width larger than the screen at 992px
    - Which doesn't take effect until > 1194px because the media query below is
more specific
        - Where did you get 1194 from anyway? 1170 + 2em (at 16px) is 1202px?
- This causes the layout to be offcenter at 1200px width

+  .container {
+    width: 1200px;
+  } 
+}
+
+@media (max-width: 1194px) {
+  .container {
+    width: 80%;
+    text-align: center;
+  }
+}
+
+@media (max-width: 474px) {
+  .container {
+    width: 100%;

[juliandoucette, 2017/10/08 13:18:11]

NIT: This kills the padding left/right because html is not box-sizing:
border-box.

+  }
+}
+
+.button-primary {

[juliandoucette, 2017/10/08 13:18:11]

NIT/Suggest: Make this button full-width on small mobile devices

+  display: inline-block;
+  text-decoration: none;
+  text-transform: uppercase;
+  color: white !important;

[juliandoucette, 2017/10/08 13:18:11]

NIT: This important is unnecessary? 

+  text-align: center;
+  width: 100%;
+  border-radius: 6px;
+  height: 54px;

[juliandoucette, 2017/10/08 13:18:12]

NIT: px [height, width, line-height] kinda defeats the purpose of using em
font-size (acknowledging that this is a very edge cases). You can use em here
(and optionally use margin and padding to vertically centre).

[martin, 2017/10/16 14:36:00]

Done.

+  line-height: 54px;
+  background: #C70D2C;
+}
+
+@media (max-width: 1194px) {
+  .button-primary {
+    width: 50%;

[juliandoucette, 2017/10/08 13:18:11]

NIT: This gets pretty awkward around 336px.

+  }
+}
+
+.store-buttons {
+  height: 54px;
+  width: 100%;
+}
+
+.store-button {
+  display: block;
+  height: 54px;
+  width: 180px;
+}
+
+.applestore-button {
+  background: url("./icons/first-run/appstore-bg.svg");
+  float: left;
+  margin-right: 1em;
+}
+
+.googleplay-button {
+  background: url("./icons/first-run/googleplay-bg.svg");
+  float: left;
+}
+
+@media (max-width: 1194px) {
+  .store-buttons {
+    text-align: center;
+  }
+  .store-button {
+    display: inline-block;

[juliandoucette, 2017/10/08 13:18:11]

NIT: These buttons are not horizontally aligned on small screens (e.g. 320px)
because of inline-block + whitespace.

+    float: none;
+  }
+}
+
+@media (max-width: 1207px) {
+  .applestore-button {
+    margin-right: .5em;
+  }
+}
+
+h1 {

[juliandoucette, 2017/10/08 13:18:11]

NIT: This doesn't fit on the screen ~320 Firefox

+  font-size: 3em;
+  margin: 0;
+}
+
+h2 {
+  font-size: 2em;
+}
+
+p {
+  margin: 0 0 1em 0;
+  font-size: 1.25em;

[juliandoucette, 2017/10/08 13:18:12]

NIT: I see that you followed Paul's stytleguide ~exactly... I would prefer that
all flow content inherited font-size from html (except for heading content). 

+  font-weight: 300;

[juliandoucette, 2017/10/08 13:18:11]

NIT: We agreed (privately - sorry) to use 400 instead of 300 for websites.

+  line-height: 150%;
+}
+
+p > a {
+  color: #C70D2C;
+  text-decoration: none;
+  font-size: 1em;

[juliandoucette, 2017/10/08 13:18:12]

NIT: This doesn't actually do anything because em is relative. p of 1.25em > a
of 1em = 1.25em.

+  font-weight: 400;
+}
+
+p > a:hover {
+  text-decoration: underline;
+}
+
+/*******************************************************************************
+ * Grid component

[juliandoucette, 2017/10/08 13:18:11]

Now that I think about it, you could include all of website-defaults main.css
(suggest "defaults.css") instead of inserting this component into this CSS file.
[the performance impact should be negligible, it should make updating easier, it
should reduce maintenance cost e.g. if you change the grid component like you
did with .column box-sizing below (which is normally applied by website-default
base styles)]. I doubt that we will introduce npm & scss into adblockplusui
anytime soon :(

+ ******************************************************************************/
+/**
+ * - .row contains one or more .column(s)
+ * - .row clears .column(s)
+ * - .row negates the left & right padding of it's left-most & right-most
+ *   .column(s) while preserving consistent padding between .column(s)
+ */
+ .row {
+  margin: 0px -1em; }
+
+.row:after {
+  display: block;
+  clear: both;
+  content: ""; }
+
+/**
+ * - .column is 100% width by default
+ * - Modifier classes are applied to .column to change it's width
+ * - Modifier classes behave differently on different device widths
+ */
+.column {
+  position: relative;
+  width: 100%;
+  min-height: 1px;
+  padding: 1em;
   box-sizing: border-box;
 }
 
-#general
-{
-  display: table;
-  padding-left: 0px;
-  padding-right: 0px;
-}
-
-#general > .block
-{
-  display: table-cell;
-  width: 50%;
-  vertical-align: top;
-  padding: 0px 50px;
-  border: dashed 0 #969085;
-  -webkit-border-start-width: 1px;
-  -moz-border-start-width: 1px;
-  border-inline-start-width: 1px;
-}
-
-#general > .block:first-child
-{
-  border: none;
-}
-
-#acceptable-ads-block h2
-{
-  margin-bottom: 34px;
-}
-
-#abb-promotion-block:lang(fr)
-{
-  display: none;
-}
-
-#abb-promotion-block
-{
-  text-align: center;
-}
+/* - .column(s) within .row .reverse appear in reverse order
+ * - .column(s) within [dir=rtl] appear in reverse order respectively
+ */
+.column,
+[dir="rtl"] .reverse .column {
+  float: left; }
 
-#abb-promotion-block h2
-{
-  color: #7795b6;
-  margin-bottom: 40px;
-}
-
-#abb-promotion-block a
-{
-  border: solid 1px #446a96;
-  border-radius: 5px;
-  display: inline-block;
-  padding: 6px 16px;
-  background: -webkit-linear-gradient(bottom, #294e76, #6b92be);
-  background: linear-gradient(to top, #294e76, #6b92be);
-  text-decoration: none;
-  text-align: start;
-}
+.reverse .column,
+[dir="rtl"] .column {
+  float: right; }
 
-#abb-promotion-block a > div
-{
-  display: table-cell;
-  vertical-align: middle;
-  padding: 6px;
-}
-
-#abb-promotion-block .title
-{
-  font-size: 21px;
-  color: #ffffff;
-  font-weight: bold;
-}
-
-#abb-promotion-block .subtitle
-{
-  font-size: 12px;
-  color: #9ab7d6;
-}
-
-#abb-promotion-block a:hover
-{
-  box-shadow: 0px 0px 5px #5D5D5D;
-}
-
-#abb-promotion-block a:active
-{
-  box-shadow: 0px 0px 5px 1px #5D5D5D;
-}
+@media (min-width: 768px) {
+  .one-half,
+  .one-fourth {
+    width: 50%; } }
 
-section > p
-{
-  margin-top: 15px;
-}
-
-#logo
-{
-  margin: 0 auto;
-  height: 128px;
-  width: 128px;
-  display: block;
-}
-
-#share
-{
-  background-image: url(background-share.png);
-  text-align: center;
-  font-family: CreteRound, Helvetica, Arial, sans-serif;
-}
-
-#share h2
-{
-  margin-bottom: 30px;
-}
-
-#share h2,
-#share h2 > a
-{
-  color: #fff;
-}
-
-#share h2 > a:hover
-{
-  color: #2e5075;
-}
-
-#share span
-{
-  color: #bed1e6;
-  text-align: center;
-  line-height: 34px;
-  font-size: 22px;
-  display: block;
-}
+@media (min-width: 992px) {
+  .one-third {
+    width: 33.333333%; }
+  .two-thirds {
+    width: 66.666667%; } }
 
-#donate:hover, .share-buttons a:hover
-{
-  box-shadow: 0px 0px 0px 2px #8ca7c4;
-  border-radius: 30px;
-}
-
-#share > .block
-{
-  display: inline-block;
-  vertical-align: top;
-  width: 49%;
-  padding: 5px 0px;
-  border: 1px dashed #37506d;
-  border-style: none dashed;
-}
-
-#share > .block:last-child
-{
-  -webkit-border-start-style: none;
-  -moz-border-start-style: none;
-}
+@media (min-width: 992px) {
+  .one-fourth {
+    width: 25%; }
+  .three-fourths {
+    width: 75%; } }
 
-#share > .block > span
-{
-  margin: 10px 20px;
-}
-
-.share-buttons
-{
-  margin-top: 6px;
-  vertical-align: top;
-  /* because inline block creates
-   * space if not captured with
-   * font-size and line-height 0px
-   */
-  font-size: 0px;
-  line-height: 0px;
-}
-
-.share-buttons > a
-{
-  width: 50px;
-  height: 50px;
-  margin: 0 8px;
-  text-decoration: none;
-  display: inline-block;
+@media (max-width: 1194px) {
+  .one-third {
+    width: 100%;
+  }
 }
 
-#share-general,
-#share-chinese:lang(zh)
-{
-  display: block;
-}
-
-#share-general:lang(zh),
-#share-chinese
-{
-  display: none;
-}
-
-#share-chinese > a
-{
-  border-bottom: 2px solid transparent;
-}
-
-#share-chinese > a:hover
-{
-  box-shadow: none;
-  border-radius: 0;
-  border-bottom: 2px solid #8CA7C4;
-}
-
-#share-facebook
-{
-  background-image: url(social/facebook.png);
-  background-repeat: no-repeat;
-}
-
-#share-twitter
-{
-  background-image: url(social/twitter.png);
-  background-repeat: no-repeat;
-}
-
-#share-gplus
-{
-  background-image: url(social/googleplus.png);
-  background-repeat: no-repeat;
-}
-
-#share-renren
-{
-  background-image: url(social/renren.png);
-  background-repeat: no-repeat;
-}
-
-#share-weibo
-{
-  background-image: url(social/weibo.png);
-  background-repeat: no-repeat;
-}
-
-.warning
-{
-  font-size: 160%;
-  border: 3px dashed red;
-  text-align: center;
-  line-height: 1.3em;
-}
-
-#social ul
-{
-  list-style: none;
-  display: inline-block;
-  padding: 0;
-  margin: 0;
-}
-
-#social:hover h1
-{
-  opacity: 0;
-}
-
-#social ul:hover li {
-  opacity: 0.3;
-}
-
-#social ul li
-{
-  display: inline-block;
-  margin: 0px 5px;
-  -webkit-transition: opacity .5s ease, bottom .3s ease;
-  transition: opacity .5s ease, bottom .3s ease;
-  position: relative;
-  bottom: -30px;
-}
-
-#social ul li:hover
-{
-  opacity: 1.0;
-  bottom: 0px;
+@media (max-width: 1194px) {
+  .column {
+    padding: 4em 0;
+    border-bottom: 1px solid lightgrey;
+  }
 }
 
-.share-button
-{
-  display: inline-block;
-  width: 82px;
-  height: 82px;
-}
 
-/* Change order of the blocks for French */
-#content:lang(fr)
-{
-  display: table;
-  margin: auto;
-  caption-side: bottom;
-}
-
-#share:lang(fr)
-{
-  display: table-caption;
-}
-
-#donate
-{
-  height: 21px;
-  display: inline-block;
-  margin-top: 15px;
-  margin-bottom: 2px;
-  font-size: 16px;
-  color: #003366;
-  cursor: pointer;
-  font-weight: bold;
-  padding: 5px 18px;
-  text-decoration: none;
-  border-radius: 20px;
-  border: 1px solid #FF9933;
-  overflow: hidden;
-  font-family: arial, sans-serif;
-  background-image: url(donate.png);
-  background-repeat: repeat-x;
-}
-
-footer
-{
-  margin: 0 auto 30px;
-  max-width: 960px;
-  text-align: center;
-}
-
-@media (max-width: 970px)
-{
-  header
-  {
-    background-size: 100px 57px;
-  }
-
-  #logo
-  {
-    width: 64px;
-    height: 64px;
-  }
-
-  section
-  {
-    padding: 40px 20px;
-  }
-
-  #general,
-  #general > .block,
-  #share > .block
-  {
-    display: block;
-  }
-
-  #general > .block
-  {
-    width: auto;
-    margin-left: 20px;
-    margin-right: 20px;
-    padding: 10px 0px;
-    border-width: 1px 0px 0px;
-  }
-
-  #share > .block
-  {
-    width: auto;
-    padding: 20px 0px;
-    border-style: dashed none none;
-  }
-
-  #donate,
-  .share-buttons
-  {
-    margin-top: 10px;
-  }
-}
+