Issue 1292 - Optimized first-run page share section for smaller screens

Issue 1292 - Optimized first-run page share section for smaller screens

[saroyanm, 2015-11-27 18:15:06]

https://codereview.adblockplus.org/29329048/diff/29329049/skin/firstRun.css
File skin/firstRun.css (right):

https://codereview.adblockplus.org/29329048/diff/29329049/skin/firstRun.css#n...
skin/firstRun.css:380: -webkit-border-start-style: none;
I do remember we had discussion several times about this. 
Shouldn't we also consider using adblockplusUI on other platforms beside webkit
based and mozilla ? Otherwise we can have issues with this solution, but it's
just note to refresh memory.
Feel free to keep it as it is right now while it's not part of the review.

https://codereview.adblockplus.org/29329048/diff/29329049/skin/firstRun.css#n...
skin/firstRun.css:673: display: block;
Detail: You can move this rule with selector also under  #general, #general >
.block

https://codereview.adblockplus.org/29329048/diff/29329049/skin/firstRun.css#n...
skin/firstRun.css:681: width: auto;
I guess you have to set this rule again because of Specificity, what about
having both width of the elements 49% ?
IMO same could work for general section and would be possible combine and have
more generic styles.
I think we do not want to use "important".
Currently whenever we will add generic rule for desktop version we will need to
consider mobile as well, I think the suggestion above will be less error prone.

[Thomas Greiner, 2015-12-03 15:14:51]

https://codereview.adblockplus.org/29329048/diff/29329049/skin/firstRun.css
File skin/firstRun.css (right):

https://codereview.adblockplus.org/29329048/diff/29329049/skin/firstRun.css#n...
skin/firstRun.css:380: -webkit-border-start-style: none;
On 2015/11/27 18:15:06, saroyanm wrote:
> I do remember we had discussion several times about this. 
> Shouldn't we also consider using adblockplusUI on other platforms beside
webkit
> based and mozilla ? Otherwise we can have issues with this solution, but it's
> just note to refresh memory.
> Feel free to keep it as it is right now while it's not part of the review.

The platforms we support are either Gecko (Firefox) or WebKit (Chrome, Opera,
Safari). According to http://caniuse.com/#feat=css-logical-props this is not
something we can use in IE and there's little hope that it's going to be added
to Edge anytime soon either.

Therefore we'll need to come back to find a different solution for Edge anyway,
when we decide to reuse it there.

https://codereview.adblockplus.org/29329048/diff/29329049/skin/firstRun.css#n...
skin/firstRun.css:673: display: block;
On 2015/11/27 18:15:06, saroyanm wrote:
> Detail: You can move this rule with selector also under  #general, #general >
> .block

Done.

https://codereview.adblockplus.org/29329048/diff/29329049/skin/firstRun.css#n...
skin/firstRun.css:681: width: auto;
On 2015/11/27 18:15:06, saroyanm wrote:
> I guess you have to set this rule again because of Specificity, what about
> having both width of the elements 49% ?
> IMO same could work for general section and would be possible combine and have
> more generic styles.
> I think we do not want to use "important".
> Currently whenever we will add generic rule for desktop version we will need
to
> consider mobile as well, I think the suggestion above will be less error
prone. 

Done. You're right. Changing the regular width from `50%` to `49%` is much more
sane than overriding it here.

[saroyanm, 2015-12-04 09:56:17]

LGTM