Issue 2359 - Polished design of options page sidebar

skin/options.css

Index: skin/options.css
===================================================================
--- a/skin/options.css
+++ b/skin/options.css
@@ -15,30 +15,55 @@
  * 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
@@ -67,30 +92,27 @@
 
 #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
@@ -111,7 +133,7 @@
   display: flex;
 }
 
-.tabs li a:first-child
+.tabs li a
 {
   flex: 1;
   color: inherit;
@@ -129,29 +151,14 @@
 
 .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.

@@ -161,14 +168,20 @@
   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
@@ -182,44 +195,40 @@
   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
@@ -426,7 +435,7 @@
 {
   border-bottom: 2px solid #1E8728;
   color: black;
-  font-weight: bold;
+  font-weight: 600;
   padding-bottom: 10px;
 }
 
@@ -669,7 +678,7 @@
 {
   color: black;
   border-bottom-color: #3A7BA6;
-  font-weight: bold;
+  font-weight: 600;
   -moz-padding-start: 0px;
   -webkit-padding-start: 0px;
 }