Fixes #33 - Adjustments to features section of index page

Fixes #33 - Adjustments to features section of index page

[ire, 2018-03-14 08:21:39]

Thanks Julian!

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.css
File static/css/index.css (right):

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.cs...
static/css/index.css:239: padding: 0 3em 2em 10.5em;
The bottom spacing here is causing > 3em spacing in the bottom of this section.
i.e. there is extra bottom padding

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.cs...
static/css/index.css:239: padding: 0 3em 2em 10.5em;
This spacing only works for LTR

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.cs...
static/css/index.css:252: right: 0;
right should be 3em

[ire, 2018-03-14 08:44:01]

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.css
File static/css/index.css (right):

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.cs...
static/css/index.css:200: * #features
TOL: I'm working on the download page, and I think we should consider using the
same layout for that as we do here.

[juliandoucette, 2018-03-14 11:50:38]

Thanks! New Patchset up!

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.css
File static/css/index.css (right):

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.cs...
static/css/index.css:200: * #features
On 2018/03/14 08:44:01, ire wrote:
> TOL: I'm working on the download page, and I think we should consider using
the
> same layout for that as we do here. 

Acknowledged.

I'd encourage you to change this if I finished before you did :p

We can sync about this if you like?

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.cs...
static/css/index.css:239: padding: 0 3em 2em 10.5em;
On 2018/03/14 08:21:39, ire wrote:
> This spacing only works for LTR

Done.

Good catch.

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.cs...
static/css/index.css:239: padding: 0 3em 2em 10.5em;
On 2018/03/14 08:21:39, ire wrote:
> The bottom spacing here is causing > 3em spacing in the bottom of this
section.
> i.e. there is extra bottom padding

Done.

Good catch.

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.cs...
static/css/index.css:252: right: 0;
On 2018/03/14 08:21:39, ire wrote:
> right should be 3em

Done.

Good catch.

[ire, 2018-03-14 13:07:42]

Just one more thing

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.css
File static/css/index.css (right):

https://codereview.adblockplus.org/29721780/diff/29721781/static/css/index.cs...
static/css/index.css:200: * #features
On 2018/03/14 11:50:37, juliandoucette wrote:
> On 2018/03/14 08:44:01, ire wrote:
> > TOL: I'm working on the download page, and I think we should consider using
> the
> > same layout for that as we do here. 
> 
> Acknowledged.
> 
> I'd encourage you to change this if I finished before you did :p
> 
> We can sync about this if you like?

Sure. I'll confirm from Jeen, and handle this in the review for the download
page.

https://codereview.adblockplus.org/29721780/diff/29722576/static/css/index.css
File static/css/index.css (right):

https://codereview.adblockplus.org/29721780/diff/29722576/static/css/index.cs...
static/css/index.css:209: padding-bottom: 3em;
TOL: It seems like we should have another `.section` class for the larger
sections with 3em padding instead of 2. Or just change the .section to use 3em
padding... (we should handle this later)

https://codereview.adblockplus.org/29721780/diff/29722576/static/css/index.cs...
static/css/index.css:234: text-align: left;
This should be text-align: right on RTL

[juliandoucette, 2018-03-14 13:54:42]

Thanks Ire! New Patchset up!

https://codereview.adblockplus.org/29721780/diff/29722576/static/css/index.css
File static/css/index.css (right):

https://codereview.adblockplus.org/29721780/diff/29722576/static/css/index.cs...
static/css/index.css:209: padding-bottom: 3em;
On 2018/03/14 13:07:42, ire wrote:
> TOL: It seems like we should have another `.section` class for the larger
> sections with 3em padding instead of 2. Or just change the .section to use 3em
> padding... (we should handle this later)

Agreed.

https://codereview.adblockplus.org/29721780/diff/29722576/static/css/index.cs...
static/css/index.css:234: text-align: left;
On 2018/03/14 13:07:42, ire wrote:
> This should be text-align: right on RTL

Good catch!

I tested this today and didn't even notice :D

[ire, 2018-03-14 14:04:35]

LGTM

https://codereview.adblockplus.org/29721780/diff/29722576/static/css/index.css
File static/css/index.css (right):

https://codereview.adblockplus.org/29721780/diff/29722576/static/css/index.cs...
static/css/index.css:234: text-align: left;
On 2018/03/14 13:54:42, juliandoucette wrote:
> On 2018/03/14 13:07:42, ire wrote:
> > This should be text-align: right on RTL
> 
> Good catch!
> 
> I tested this today and didn't even notice :D

Haha I didn't notice either the first time :P

[juliandoucette, 2018-03-14 14:55:12]

Done https://hg.adblockplus.org/web.adblockplus.org/rev/8e2aaa2f35f1

[juliandoucette, 2018-03-14 14:59:17]

Undone https://hg.adblockplus.org/web.adblockplus.org/rev/f75d9970d569

:P

[juliandoucette, 2018-03-14 15:00:37]

Redone https://hg.adblockplus.org/web.adblockplus.org/rev/d9b6ae317dab