Issue 2359 - Polished design of options page sidebar

skin/options.css

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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/>.
  */
 
+@font-face
+{
+  font-family: "Source Sans Pro";
+  src: url(fonts/SourceSansPro-Light.woff) format("woff");

[saroyanm, 2015/07/09 12:56:33]

I think we can use local to force local system fonts messing up things as
suggested here:
http://www.paulirish.com/2010/font-face-gotchas/#smiley 

[Thomas Greiner, 2015/07/09 16:42:46]

Not sure whether I understand what you mean. Using locally installed fonts
doesn't make a difference for us here because we don't have the performance
penalty that webpages would have.

[saroyanm, 2015/07/09 17:48:14]

This solution is for being sure that no local font is being used.
Local fonts can be modified by softwares as noted in the post, ex.: Webkit+Font
Management software can mess up local references. 

Also please note we forcing not to use local font also in the firstRun.css:
https://hg.adblockplus.org/adblockplusui/file/819f1aca30cc/skin/firstRun.css#l23

[Thomas Greiner, 2015/07/10 09:48:14]

Done.

+  font-weight: 300;
+  font-style: normal;
+}
+
+@font-face
+{
+  font-family: "Source Sans Pro";
+  src: url(fonts/SourceSansPro-Regular.woff) format("woff");
+  font-weight: 400;
+  font-style: normal;
+}
+
+@font-face
+{
+  font-family: "Source Sans Pro";
+  src: url(fonts/SourceSansPro-Semibold.woff) format("woff");
+  font-weight: 600;
+  font-style: normal;
+}
+
 body
 {
   background-color: #F5F5F5;
   display: flex;
+  margin: 20px 10px;
   font-family: "Source Sans Pro", sans-serif;
   font-size: 14px;
-  white-space: nowrap;
 }
 
 h1 
 {
   font-size: 24px;
-  font-weight: 100;
+  line-height: 1em;
+  font-weight: 300;
 }
 
 h2
 {
   font-size: 16px;
-  font-weight: 500;
+  font-weight: 600;
 }
 
 p
 {
-  font-weight: 100;
+  font-weight: 300;
 }
 
 hr
 {
   background-color: #CDCDCD;
   border: 0px;
   height: 1px;
   margin: 0px;
 }
 
 input[type="search"]::-webkit-search-cancel-button
 {
   display: none;
 }
 
 .option-name
 {
   display: flex;
 }
 
 .option-name > *
 {
   -moz-margin-end: 6px;
   -webkit-margin-end: 6px;
 }
 
 #nav-sidebar
 {
-  position: relative;
-  padding: 0px 0px 16px 0px;
-  -moz-padding-start: 10px;
-  -webkit-padding-start: 10px;
   min-width: 198px;
 }
 
 #nav-sidebar .fixed
 {
-  height: 100%;
+  top: 40px;
+  bottom: 2px;
+  height: auto;
   position: fixed;
 }
 
 #page-title
 {
-  -webkit-margin-start: 10px;
-  -moz-margin-start: 10px;
+  padding: 0px 20px;
 }
 
 #page-title p
 {
   margin: 0px;
-  padding: 40px 0px 0px 0px;
   font-size: 16px;
+  line-height: 1em;
 }
 
 #page-title h1
 {
   margin: 0px;
   padding: 8px 0px 16px 0px;
 }
 
 .flex-container
 {
   display: flex;
   flex-flow: row wrap;
 }
 
 .tabs li
 {
   cursor: pointer;
   display: flex;
 }
 
-.tabs li a:first-child
+.tabs li a
 {
   flex: 1;
   color: inherit;
   text-decoration: none;
 }
 
 .tabs.vertical
 {
   list-style: none;
   margin: 0px;
   position: relative;
   padding: 0px;
   width: 198px;
 }
 
 .tabs.vertical li
 {
-  border-color: #CDCDCD;
+  border-color: #CDCDCD transparent;
   border-style: solid;
-  border-width: 1px 0px 0px 0px;
+  border-width: 1px;
   font-size: 16px;
-  font-weight: 100;
-  padding: 16px 0px;
-  -moz-padding-end: 20px;
-  -webkit-padding-end: 20px;
-  -moz-padding-start: 11px;
-  -webkit-padding-start: 11px;
-}
-
-body[data-tab="general"] #tab-general + li,
-body[data-tab="advanced"] #tab-advanced + li,
-body[data-tab="help"] #tab-help + li
-{
-  border-width: 0px;
-}
-
-#tab-help,
-body[data-tab="advanced"] #tab-advanced + li
-{
-  border-bottom-width: 1px;
+  font-weight: 300;
+  line-height: 1em;
+  margin-top: -1px;
+  padding: 14px 20px;
 }
 
 body[data-tab="general"] #tab-general,

[saroyanm, 2015/07/09 12:56:33]

during "RTL" direction the active tab's "start" borders look inconsistent. 
I can't find the issue, make sense to investigate this behavior.

[Thomas Greiner, 2015/07/09 16:42:46]

Done.

 body[data-tab="advanced"] #tab-advanced,
 body[data-tab="help"] #tab-help
 {
   background-color: #FFFFFF;
   border-radius: 3px 0px 0px 3px;
   border-width: 1px;
-  -moz-border-end: 0px;
-  -webkit-border-end: 0px;
-  font-weight: 900;
+  -moz-border-start-color: #CDCDCD;
+  -webkit-border-start-color: #CDCDCD;
   -moz-margin-end: -1px;
   -webkit-margin-end: -1px;
-  -moz-padding-start: 10px;
-  -webkit-padding-start: 10px;
-  position: relative;
+  font-weight: 600;
+  -moz-padding-end: 21px;
+  -webkit-padding-end: 21px;
+}
+
+html[dir="rtl"] body[data-tab="general"] #tab-general,
+html[dir="rtl"] body[data-tab="advanced"] #tab-advanced,
+html[dir="rtl"] body[data-tab="help"] #tab-help
+{
+  border-radius: 0px 3px 3px 0px;
 }
 
 .tabs.vertical.bottom
 {
   bottom: 0px;
   position: absolute;
 }
 
 .tabs.vertical.bottom li:first-child
 {
   border-top: 0px;
 }
 
-.tabs.vertical .icon
+#tab-contribute
 {
-  margin: 2px 0px 0px 0px;
+  border-bottom: none;
+}
+
+#nav-sidebar ul li .icon
+{
+  height: 14px;

[saroyanm, 2015/07/09 12:56:33]

Maybe
height: 15px;
width: 15px;

But it still needs to be tested, with current approach some icons are being cut
off for 1 pixel, but it's more about how browser render them.
So I'll suggest to use the approach above but be sure that all the browser show
icons correctly even it's not consistent with styleguide.

[Thomas Greiner, 2015/07/09 16:42:46]

Yes, some icons are cut off because the values of the "background-position"
property are incorrect. I fixed the values for the icons in the sidebar but left
the other ones to be changed in a future review.

[saroyanm, 2015/07/09 17:48:14]

What about changing the the height and width of sidebar icons as I mentioned in
my first comment of this line ?

[Thomas Greiner, 2015/07/10 09:48:14]

According to the style guide the icons are supposed to be 14x14 pixels and I
didn't find any issues with that. So please let me know if you discovered an
issue that requires the width and height values to be adapted.

Let me also know if you want me to change the "background-position" property of
the other icons on the page within this review. Because as I mentioned, I
intended that to be done in a future review.

+  width: 14px;
 }
 
 #tab-general .icon
 {
-  background-position: -15px -17px;
-  height: 16px;
-  width: 16px;
+  background-position: -16px -18px;
 }
 
 #tab-advanced .icon
 {
-  background-position: -45px -18px;
-  height: 16px;
-  width: 16px;
+  background-position: -46px -18px;
 }
 
 #tab-help .icon
 {
-  background-position: 0px -17px;
-  height: 16px;
-  width: 16px;
+  background-position: -1px -18px;
 }
 
 #tab-share .icon
 {
-  background-position: -60px -17px;
-  height: 16px;
-  width: 16px;
+  background-position: -61px -18px;
 }
 
 #tab-contribute .icon
 {
-  background-position: -30px -17px;
-  height: 16px;
-  width: 16px;
+  background-position: -31px -18px;
 }
 
 #tab-content
 {
   background-color: #FFFFFF;
   border: 1px solid #CDCDCD;
   border-radius: 8px;
   padding: 0px 60px 40px 60px;
   width: 960px;
 }
@@ skipping 186 matching lines @@
   color: #3A7BA6;
   padding: 10px 0px 11px 0px;
   text-align: center;
   width: 50%;
 }
 
 .tabs.horizontal li.active
 {
   border-bottom: 2px solid #1E8728;
   color: black;
-  font-weight: bold;
+  font-weight: 600;
   padding-bottom: 10px;
 }
 
 .icon, .table input[type="checkbox"]::before, .table button.delete,
 #content-help a::before, #dialog-close::before
 {
   background-image: url(options-sprite.png);
   display: inline-block;
 }
 
@@ skipping 222 matching lines @@
 #content-advanced .tooltip
 {
   -moz-margin-start: 8px;
   -webkit-margin-start: 8px;
 }
 
 #block-element-explanation a
 {
   color: black;
   border-bottom-color: #3A7BA6;
-  font-weight: bold;
+  font-weight: 600;
   -moz-padding-start: 0px;
   -webkit-padding-start: 0px;
 }
 
 /*
   Help tab content
 */
 
 #content-help
 {
@@ skipping 193 matching lines @@
   -moz-padding-end: 10px;
   -webkit-padding-start: 0px;
   -moz-padding-start: 0px;
 }
 
 #other-language .display
 {
   -webkit-margin-start: 10px;
   -moz-margin-start: 10px;
 }