Issue 2359 - Polished design of options page sidebar

Mainly tackled this because I was already working on #2383 through which I discovered some issues. This review includes the following style changes:

- Removed some vendor-specific properties where possible (e.g. margin and padding)
- Removed margin-top for icons
- Made navigation menu items independent of each other (i.e. independent of whether neighbor is selected or not)
- Corrected proportions as specified in design guide
- Fixed: fonts were missing
- Fixed: `border-radius` of selected navigation menu item is incorrect in RTL scenario (unfortunately, we cannot use `:dir()` for that yet)
- Fixed: sidebar was overflowing to the bottom
- Fixed: incorrect font-weight values (light: 300, normal: 400, semibold: 600)
- Fixed: incorrect background-position, width and height for sidebar icons
- Fixed: `white-space:nowrap` caused long navigation menu item texts to overflow

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

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css
File skin/options.css (right):

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css#ne...
skin/options.css:21: src: url(fonts/SourceSansPro-Light.woff) format("woff");
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

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css#ne...
skin/options.css:164: body[data-tab="general"] #tab-general,
during "RTL" direction the active tab's "start" borders look inconsistent. 
I can't find the issue, make sense to investigate this behavior.

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css#ne...
skin/options.css:205: height: 14px;
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]

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css
File skin/options.css (right):

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css#ne...
skin/options.css:21: src: url(fonts/SourceSansPro-Light.woff) format("woff");
On 2015/07/09 12:56:33, saroyanm wrote:
> 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 

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.

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css#ne...
skin/options.css:164: body[data-tab="general"] #tab-general,
On 2015/07/09 12:56:33, saroyanm wrote:
> during "RTL" direction the active tab's "start" borders look inconsistent. 
> I can't find the issue, make sense to investigate this behavior.

Done.

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css#ne...
skin/options.css:205: height: 14px;
On 2015/07/09 12:56:33, saroyanm wrote:
> 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.

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]

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css
File skin/options.css (right):

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css#ne...
skin/options.css:21: src: url(fonts/SourceSansPro-Light.woff) format("woff");
On 2015/07/09 16:42:46, Thomas Greiner wrote:
> On 2015/07/09 12:56:33, saroyanm wrote:
> > 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 
> 
> 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.

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

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css#ne...
skin/options.css:205: height: 14px;
On 2015/07/09 16:42:46, Thomas Greiner wrote:
> On 2015/07/09 12:56:33, saroyanm wrote:
> > 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.
> 
> 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.

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

https://codereview.adblockplus.org/29321240/diff/29321552/skin/options.css
File skin/options.css (right):

https://codereview.adblockplus.org/29321240/diff/29321552/skin/options.css#ne...
skin/options.css:180: -moz-padding-start: 21px;
I assume you are not using "padding: 14px 20px;" not to specify bottom and top
paddings in two places ?

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

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css
File skin/options.css (right):

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css#ne...
skin/options.css:21: src: url(fonts/SourceSansPro-Light.woff) format("woff");
On 2015/07/09 17:48:14, saroyanm wrote:
> On 2015/07/09 16:42:46, Thomas Greiner wrote:
> > On 2015/07/09 12:56:33, saroyanm wrote:
> > > 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 
> > 
> > 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.
> 
> 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

Done.

https://codereview.adblockplus.org/29321240/diff/29321241/skin/options.css#ne...
skin/options.css:205: height: 14px;
On 2015/07/09 17:48:14, saroyanm wrote:
> On 2015/07/09 16:42:46, Thomas Greiner wrote:
> > On 2015/07/09 12:56:33, saroyanm wrote:
> > > 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.
> > 
> > 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.
> 
> What about changing the the height and width of sidebar icons as I mentioned
in
> my first comment of this line ?

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.

https://codereview.adblockplus.org/29321240/diff/29321552/skin/options.css
File skin/options.css (right):

https://codereview.adblockplus.org/29321240/diff/29321552/skin/options.css#ne...
skin/options.css:180: -moz-padding-start: 21px;
On 2015/07/09 17:48:14, saroyanm wrote:
> I assume you are not using "padding: 14px 20px;" not to specify bottom and top
> paddings in two places ?

Exactly, unfortunately there's no nice way to handle this without duplication so
I chose this one for the reason you mentioned. It'd be great if we could just
do:

margin-horizontal: -1px;
padding-horizontal: 21px;

But that doesn't exist. :D

[Thomas Greiner, 2015-07-10 11:50:14]

Apparently, the offset of the sidebar icons was due to them being mispositioned
in the sprite so I fixed the graphic.

[saroyanm, 2015-07-10 14:36:25]

On 2015/07/10 11:50:14, Thomas Greiner wrote:
> Apparently, the offset of the sidebar icons was due to them being
mispositioned
> in the sprite so I fixed the graphic.

LGTM