No Issue - Implemented first run page

skin/firstRun.css

 /*

[Thomas Greiner, 2018/02/16 16:19:15]

It is difficult to distinguish styles that are coming from websites-defaults
from styles you introduced for the purpose of the first-run page. However, both
are mixed together in "firstRun.css", implying that all of those were made
specifically for the first-run page.

Therefore, I strongly suggest to split those styles up and only include styles
specifically created for the first-run page in here.

Thereby you can keep any non-relevant styles (e.g. ones targeting IE9) but in a
separate file so that we know that we've simply imported them from Websites and
don't need to change them.

   * This file is part of Adblock Plus <https://adblockplus.org/>,
   * Copyright (C) 2006-present eyeo GmbH
   *
   * Adblock Plus is free software: you can redistribute it and/or modify
   * it under the terms of the GNU General Public License version 3 as
   * published by the Free Software Foundation.
   *
   * Adblock Plus is distributed in the hope that it will be useful,
   * but WITHOUT ANY WARRANTY; without even the implied warranty of
   * 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/>.
 */

[Thomas Greiner, 2018/02/16 16:19:15]

Detail: What happened to the whitespaces here?

 
 @font-face 
 {
   font-family: "Source Sans Pro";
   font-style: normal;
   font-weight: 400;
   font-stretch: normal;
   src: local ("Ø"),
   url(fonts/SourceSansPro-Regular.woff) format("woff");
 }
 
 @font-face

[juliandoucette, 2017/10/30 15:02:28]

NIT: new adblockplus.org is currently using semi-bold 600 instead of bold 700

(This may not be documented in the spec/styleguide yet - sorry)

 {
   font-family: "Source Sans Pro";
   font-style: bold;
   font-weight: 700;
   font-stretch: normal;
   src: local ("Ø"),
   url(fonts/SourceSansPro-bold.woff) format("woff");
 }
 
 html
 {
   font-family: "Source Sans Pro", Helvetica, Arial, sans-serif;
 }
 
 body
 {
   margin: 0;
   padding: 0;
 }
 
-.container

[juliandoucette, 2017/10/30 15:02:28]

NIT: Both of these styles are covered by default.css. See
https://codereview.adblockplus.org/29565721/diff/29589564/skin/defaults.css#n...

[martin, 2018/01/30 14:10:04]

Done.

-{
-  max-width: 100%;
-  padding: 0px 1em;
-}
-
-#main-content-container {

[juliandoucette, 2017/10/30 15:02:28]

NIT: .container already applies margin 0 auto. It would be clearer/more specific
to apply margin-bottom: 8em.

[martin, 2018/01/30 14:10:04]

Done.

-  margin: 0 auto 8em;
-}
-
 .ta-centre
 {
   text-align: center;
 }
 
+#content {
+  margin-bottom: 3em;
+}
+
 @media (max-width: 992px) 
 {
-  #content > .one-third 
+  #content .one-third 
   {
     padding: 3em 2em;
     border-bottom: 1px solid #d3d3d3;
   }
-  #content > .one-third:last-child {
+  #content .one-third:last-child {
     border-bottom: none;
   }
 }
 
 /******************************************************************************
   * .content
 *****************************************************************************/

[Thomas Greiner, 2018/02/16 16:19:17]

Detail: Missing whitespace at beginning of line causing the line to be offset a
bit.

Also applies to other occurrences in this file.

 
 .content h1,
 .content h2,
 .content h3,
 .content h4,
 .content h5
 {
   font-weight: bold;

[Thomas Greiner, 2018/02/16 16:19:19]

We consistently use number values for the font weight across all our files to
avoid confusion so please use `700` here.

   margin: 32px 0px;

[Thomas Greiner, 2018/02/16 16:19:19]

Coding style: "Omit unit specification after “0” values, unless required."

Also applies to other occurrences in this file.

See https://google.github.io/styleguide/htmlcssguide.html#0_and_Units

   margin: 2rem 0rem;
 }
 
 .content h1 { font-size: 1.8em; }
 .content h2 { font-size: 1.4em; }
 .content h3 { font-size: 1.3em; }
 .content h4 { font-size: 1.2em; }
 .content h5 { font-size: 1.1em; }
 
 @media(min-width: 768px)
 {
   .content h1 { font-size: 2.4em; }
   .content h2 { font-size: 1.6em; }
 }
 
 .content a,
 .content a:visited
 {
   color: #C70D2C;
 }
 
-
 /*******************************************************************************
-  * #navbar
-******************************************************************************/
+ * #navbar
+ ******************************************************************************/
 
 #navbar
 {
-  height: 4em;
+  min-height: 4em;
   background-color: #c70d2c;
 }
 
+@media(max-width: 991px)
+{
+  #navbar
+  {
+    overflow: auto;
+  }
+}
+
 #navbar li
 {
   list-style-type: none;
 }
 
-/* padding provided by navbar contents */
-.navbar-container
-{
-  width: 80%;
-  margin: 0 auto;
-}
-
-/* full-width under .container max-width */
-@media(max-width: 1199px)
-{
-  .navbar-container
-  {
-    width: 100%;
-  }
+#navbar .container
+{
+  padding-left: 0;
+  padding-right: 0;
 }
 
 /* #navbar #navbar-logo
-  ******************************************************************************/
+ ******************************************************************************/
 
 #navbar-logo
 {
   /* padding is visible in hover background-color change */
   padding: 0em 1em;
   color: #fff;
 }
 
 #navbar-logo:hover,
 #navbar-logo:active,
@@ skipping 28 matching lines @@
 }
 
 #navbar-logo > span
 {
   /* full-height: 2.91 * 1.375 = ~4em */
   line-height: 2.91em;
   font-size: 1.375em;
 }
 
 /* #navbar #navbar-menu-toggle
-  ******************************************************************************/
+ ******************************************************************************/
 
 #navbar-menu-toggle
 {
   float: right;
   padding: 1em;
 }
 
 [dir="rtl"] #navbar-menu-toggle
 {
   float: left;
@@ skipping 22 matching lines @@
 }
 
 #navbar-menu-toggle:hover,
 #navbar-menu-toggle:active,
 #navbar-menu-toggle:focus
 {
   background-color: #AE0013;
 }
 
 /* #navbar #navbar-menu
-  ******************************************************************************/
+ ******************************************************************************/
 
 #navbar-menu a
 {
   display: block;
   color: #fff;
   text-decoration: none;
 }
 
+/* #navbar #navbar-menu #navbar-locale-menu
+ ******************************************************************************/
+
+/* caret */
+#navbar-locale-selected:after
+{
+  display: inline-block;
+  width: 0;
+  height: 0;
+  margin: 0em 0em 0em 0.255em;
+  vertical-align: .255em;

[Thomas Greiner, 2018/02/16 16:19:16]

Coding style: "Don't omit the optional leading 0 for decimal numbers."

Also applies to other occurrences in this file.

+  content: "";
+  border-top: .3em solid;
+  border-right: .3em solid transparent;
+  border-left: .3em solid transparent;
+}
+
+[dir="rtl"] #navbar-locale-selected:after
+{
+  margin: 0em 0.255em 0em 0em;
+}
+
+#navbar-locale-menu
+{
+  display: none;
+}
+
+#navbar-locale-menu.visible
+{
+  display: block;
+}
+
+#navbar-locale-menu > li > a
+{
+  padding: 0.75em 1em;
+}
+
+#navbar-locale-menu > li > a:hover,
+#navbar-locale-menu > li > a:active,
+#navbar-locale-menu > li > a:focus
+{
+  background-color: #434343;
+}
+
 /* #navbar #navbar-menu (desktop)
-  ******************************************************************************/
+ ******************************************************************************/
 
 @media(min-width: 992px)
 {
   #navbar-menu
   {
     float: right;
   }
-  
+
   [dir="rtl"] #navbar-menu
   {
     float: left;
   }
-  
+
   #navbar-menu > li,
   #navbar-menu > li > a
   {
     display: inline-block;
   }
-  
+
   #navbar-menu > li > a
   {
     padding: 0em 1em;
     line-height: 4em;
   }
-  
+
   #navbar-menu > li > a:hover,
   #navbar-menu > li > a:active,
   #navbar-menu > li > a:focus
   {
     background-color: #AE0013;
   }
-  
+
   /* #navbar #navbar-menu #navbar-locale-menu

[Thomas Greiner, 2018/02/16 16:19:20]

You don't include this section of the navbar so let's not include its styles
either.

-    ****************************************************************************/
-  
+   ****************************************************************************/
+
   #navbar-locale-menubar
   {
     position: relative;
   }
-  
+
   #navbar-locale-menu
   {
     position: absolute;
     min-width: 16em;
     max-height: 20em;
     max-height: 50vh;
     top: 100%;
     right: 0px;
     overflow: auto;
     z-index: 9001;
     background-color: #292929;
   }
-  
+
   [dir="rtl"] #navbar-locale-menu
   {
     right: auto;
     left: 0px;
   }
 }
 
 /* #navbar #navbar-menu (mobile)
-  ******************************************************************************/
+ ******************************************************************************/
 
 @media(max-width: 991px)
 {
   #navbar-menu
   {
     display: none;
     float: left;

[Thomas Greiner, 2018/02/16 16:19:18]

I don't see a rule for reversing this for right-to-left locales.

     clear: both;
     width: 100%;
     background-color: #292929;
   }
-  
+
   #navbar-menu.visible
   {
     display: block;
   }
-  
+
   #navbar-menu li a
   {
     padding: 0.75em 1em;
   }
-  
+
   #navbar-menu li a:hover,
   #navbar-menu li a:active,
   #navbar-menu li a:focus
   {
     background-color: #434343;
   }
 }
 
 /******************************************************************************
   * Main Page Content
 *****************************************************************************/
 
 #installation-status-header
 {
   text-align: center;
   background: #f9f9f9;

[Thomas Greiner, 2018/02/16 16:19:20]

Detail: Do you actually want to override other background-* properties or do you
merely want to change the color? Because for the latter, using
"background-color" would make this more maintainable.

Also applies to other occurrences in this file.

   padding: 4em 0;
   margin-bottom: 4em;
 }
 
 #installation-status-header > h1 {
   text-transform: uppercase;
 }
 
 .section-header > h2
 {
   margin: 1em 0 0;
 }
 
 .button-primary
 {
   display: inline-block;
   text-decoration: none;
   text-transform: uppercase;
   color: #fff !important;

[Thomas Greiner, 2018/02/16 16:19:17]

What's the reason for using `!important` here?

   text-align: center;
   width: 100%;
   border-radius: 6px;
   padding: 1em 0;
   background: #C70D2C;
 }
 
 .store-buttons
 {
   width:  100%;
@@ skipping 31 matching lines @@
 {
   .store-buttons 
   {
     flex-direction: column;
   }
   
   .store-button 
   {
     width: 100%;
   }
   .applestore-button

[Thomas Greiner, 2018/02/16 16:19:16]

Coding style: "Separate rules by new lines."

See https://google.github.io/styleguide/htmlcssguide.html#Rule_Separation

   {
     margin: 0 0 1em;
   }
 }
 
 #installation-status-header > h1
 {
   font-size: 3em;
   margin: 0;
 }
 
+
 /*******************************************************************************
-  * #footer
-******************************************************************************/
+ * #footer
+ ******************************************************************************/
 
 #footer
 {
   overflow: auto;
   margin-top: 2em;
   padding: 2em 0em;
   color: #ececec;
   background-color: #292929;
   font-size: 0.9em;
 }
 
 /* #footer body
-  ******************************************************************************/
+ ******************************************************************************/
 
 #footer h5
 {
   margin-top: 1em;
   margin-bottom: 1.5em;
 }
 
 #footer h5:after

[Thomas Greiner, 2018/02/16 16:19:16]

Detail: The standard way to write this is `#footer h5::after`. Unlike Websites,
we don't need to support older browsers who don't understand the standard syntax
yet.

See also https://developer.mozilla.org/en-US/docs/Web/CSS/Pseudo-elements

 {
   content: "";
   display: block;
   width: 1.25em;
   height: 0.125em;
   margin-top: 0.5em;
   background-color: #ececec;
 }
 
 #footer ul
 {
   padding: 0em;
   list-style-type: none;
 }
 
 #footer a
 {
   color: #ececec;
 }
 
 #footer a:hover,
 #footer a:active,
 #footer a:focus
 {
   color: #fff;
   text-decoration: underline;
 }
 
 /* #footer #social-list
-  ******************************************************************************/
+ ******************************************************************************/
 
 #social-list ul
 {
   /* negative margin canceled out by li margin below */
   margin: 0em -0.375em;
 }
 
 #social-list li
 {
   float: left;
@@ skipping 12 matching lines @@
 
 @media(min-width: 1200px)
 {
   #social-list img
   {
     height: 3em;
   }
 }
 
 /* #footer #footer-legal
-  ******************************************************************************/
+ ******************************************************************************/
 
 #footer-legal
 {
   margin-top: 1em;
 }
 
 @media(min-width: 1200px)
 {
   #footer-legal
   {
     margin-top: 2em;
   }
 }
 
 /* #footer #footer-legal #legal-list
-  ******************************************************************************/
+ ******************************************************************************/
 
 #legal-list li
 {
   float: left;
 }
 
 [dir="rtl"] #legal-list li
 {
   float: right;
 }
 
 #legal-list li:after
 {
   margin: 0em 0.375em;
 }
 
 /* IE9+ only */

[Thomas Greiner, 2018/02/16 16:19:19]

This is not relevant for us since we don't support IE9 so let's remove it.

 #legal-list li::after
 {
   content: "|"
 }
 
 #legal-list li:last-of-type::after
 {
   content: none;
 }
 
 /* float copyright and legal links left and right on ~desktop */
 @media(min-width: 1200px)
 {
   #copyright-notice
   {
     float: left;
   }
-  
+
   [dir="rtl"] #copyright-notice
   {
     float: right;
   }
-  
+
   #legal-list
   {
     float: right;
   }
-  
+
   [dir="rtl"] #legal-list
   {
     float: left;
   }
 }
 
 /* hide all footer links except #social-list on ~mobile */
 @media(max-width: 767px)
 {
   #footer .column
   {
     display: none;
   }
-  
+
   /* overriding display:none above */
   #footer #social-list
   {
     display: block;
   }
 }