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.

+  src: url(fonts/SourceSansPro-Light.woff);
+  /* local("Ø") forces using no local font called Source Sans Pro */
+  src: local("Ø"), url(fonts/SourceSansPro-Light.woff) format("woff");
   font-weight: 300;
   font-style: normal;
 }
 
 @font-face
 {
   font-family: "Source Sans Pro";
-  src: url(fonts/SourceSansPro-Regular.woff) format("woff");
+  src: url(fonts/SourceSansPro-Regular.woff);
+  /* local("Ø") forces using no local font called Source Sans Pro */
+  src: local("Ø"), 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");
+  src: url(fonts/SourceSansPro-Semibold.woff);
+  /* local("Ø") forces using no local font called Source Sans Pro */
+  src: local("Ø"), 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;
@@ skipping 106 matching lines @@
   border-color: #CDCDCD transparent;
   border-style: solid;
   border-width: 1px;
   font-size: 16px;
   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;
+  font-weight: 600;
   -moz-border-start-color: #CDCDCD;
   -webkit-border-start-color: #CDCDCD;
   -moz-margin-end: -1px;
   -webkit-margin-end: -1px;
-  font-weight: 600;
+  -moz-margin-start: -1px;
+  -webkit-margin-start: -1px;
   -moz-padding-end: 21px;
   -webkit-padding-end: 21px;
+  -moz-padding-start: 21px;
+  -webkit-padding-start: 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;
 }
 
 #tab-contribute
 {
   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: -16px -18px;
 }
 
 #tab-advanced .icon
 {
@@ skipping 669 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;
 }