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/>,

[juliandoucette, 2017/10/21 16:18:46]

Please keep the spaces before "*" in these comments

[martin, 2017/10/26 10:33:17]

At first I wasn't sure what you meant exactly, but now I think you want the
spaces in order to make comments collapsable? I'll add spaces so that collapsing
would be possible. I actually copied the compiled "css" from website-defaults.
The indentation must have been screwed up during the scss->css process.

-* 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/>.
+  * 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 

[juliandoucette, 2017/10/21 16:18:43]

This doesn't seem to be used anywhere?

[martin, 2017/10/26 10:33:17]

Done.

-{
-  font-family: "Source Sans Pro";
-  font-style: normal;
-  font-weight: 300;
-  font-stretch: normal;
-  src: local ("Ø"),
-  url(fonts/SourceSansPro-Light.woff) format("otf");
-}
 
 @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
 {
   font-family: "Source Sans Pro";
-  font-style: normal;

[juliandoucette, 2017/10/21 16:18:43]

NIT: You could name this "bold"

[martin, 2017/10/26 10:33:17]

Done.

+  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
-{
-  width: 80%;

[juliandoucette, 2017/10/21 16:18:43]

new abp.org uses website-defaults .container width. Why are you setting a custom
width for this page?

[martin, 2017/10/26 10:33:19]

Honestly I chose to override the default for the sole purpose of the buttons in
the first and second columns to horizontally align on wider displays. Due to the
variable height of the <p> the buttons misalign by default. Alternatively we can
give the columns a fixed min-height and pin the buttons to the bottom, however
that approach felt a bit off. I dropped the width rule.

-  max-width: 100%;
-  margin: 0 auto 8em;

[juliandoucette, 2017/10/21 16:18:44]

NIT: `.container` is a very general class. It's meant to centre the content
within, and control the width and padding of, horizontal sections of the page.
And it's currently being used in the main content *and* the footer. I'm guessing
that you are trying to add 8em of space between the main content and the footer.
If I am guessing correctly then I would suggest using a more specific selector
for this purpose e.g. Adding an ID to the main content and applying an 8em
margin-bottom to it.

[martin, 2017/10/26 10:33:18]

Done.

-  padding: 0px 1em;
-}
-
 .ta-centre
 {
   text-align: center;
 }
 
+#content {
+  margin-bottom: 3em;
+}
+
 @media (max-width: 992px) 
 {
-  #content-columns-container > .one-third 

[juliandoucette, 2017/10/21 16:18:44]

NIT: `#content-columns-container` is quite long. We currently use `#content` for
this purpose on adblockplus.org. I suggest we use `#content` for the sake of
consistency.

[martin, 2017/10/26 10:33:18]

Done.

-  {
-    padding: 3em 0;
+  #content .one-third 
+  {
+    padding: 3em 2em;
     border-bottom: 1px solid #d3d3d3;
   }
+  #content .one-third:last-child {
+    border-bottom: none;
+  }
 }
 
 /******************************************************************************
-* .content
+  * .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;
 }
@@ skipping 14 matching lines @@
 }
 
 #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
+  * 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;
   }
 }