Issue 5742 - Added container and 4 column grid to adblockplus.org

Issue 5742 - Added container and 4 column grid to adblockplus.org

[juliandoucette, 2017-10-04 19:13:52]

Note: The homepage width is different than the regular page width (as defined in
main.css) because the homepage implements custom widths (instead of relying on
standard widths, e.g. column widths) in index*.css. I have chosen to address
these one-by-one (foreach homepage component) separately.

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main-des...
File static/css/main-desktop.css (right):

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main-des...
static/css/main-desktop.css:7: header, #footer-content
Note: I left header and footer alone because I have changed this code in the
header and footer reviews.

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main.css...
static/css/main.css:2: * Base styles
Note: This will be automated ASAP.

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main.css...
static/css/main.css:83: @media (min-width: 1200px) {
Note: I customized this breakpoint and width to match the new abp.org design

[juliandoucette, 2017-10-04 19:15:30]

Note: I have added the grid without using it because we will use it on the
firstRun page in adblockplusui. I also intend to use this grid in the footer
after this patch lands.

[ire, 2017-10-04 22:17:10]

Question: Is there a reason we aren't using SCSS?


> Note: The homepage width is different than the regular page width (as defined
in
> main.css) because the homepage implements custom widths (instead of relying on
> standard widths, e.g. column widths) in index*.css. I have chosen to address
> these one-by-one (foreach homepage component) separately.

Ack.

> Note: I have added the grid without using it because we will use it on the
> firstRun page in adblockplusui. I also intend to use this grid in the footer
> after this patch lands.

Ack.

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main-des...
File static/css/main-desktop.css (right):

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main-des...
static/css/main-desktop.css:7: header, #footer-content
On 2017/10/04 19:13:52, juliandoucette wrote:
> Note: I left header and footer alone because I have changed this code in the
> header and footer reviews.

Acknowledged.

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main.css...
static/css/main.css:2: * Base styles
On 2017/10/04 19:13:52, juliandoucette wrote:
> Note: This will be automated ASAP.

Sorry, not sure what you mean. What will be automated?

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main.css...
static/css/main.css:4: html {
The formatting doesn't follow our coding style.

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main.css...
static/css/main.css:83: @media (min-width: 1200px) {
On 2017/10/04 19:13:52, juliandoucette wrote:
> Note: I customized this breakpoint and width to match the new http://abp.org
design

Acknowledged.

[juliandoucette, 2017-10-05 12:07:36]

On 2017/10/04 22:17:10, ire wrote:
> Question: Is there a reason we aren't using SCSS?

Yes and No :D 

See:
[adblockplus.org
1.1.0](https://issues.adblockplus.org/milestone/adblockplus.org%201.1.0)
[Websites code
sharing](https://issues.adblockplus.org/milestone/Websites%20code%20sharing)

adblockplus.org 1.1.0 is higher priority than Websites code sharing. And,
although I would like to do Websites code sharing first, I don't think we can;
realistically. Of course, I could push SCSS *and* generated CSS to
adblockplus.org like I did acceptableads.com. But I don't really want to. I'd
rather just push the generated CSS from website-defaults and then replace it
after we have implemented Websites code sharing. Maybe this is less work. But
this decision was mostly arbitrary. And you may suggest otherwise if you think
it makes sense to.

[juliandoucette, 2017-10-05 17:17:26]

Fixed typo mentioned in footer review.

[ire, 2017-10-06 07:14:50]

On 2017/10/05 12:07:36, juliandoucette wrote:
> On 2017/10/04 22:17:10, ire wrote:
> > Question: Is there a reason we aren't using SCSS?
> 
> Yes and No :D 
> 
> See:
> [adblockplus.org
> 1.1.0](https://issues.adblockplus.org/milestone/adblockplus.org%201.1.0)
> [Websites code
> sharing](https://issues.adblockplus.org/milestone/Websites%20code%20sharing)
> 
> http://adblockplus.org 1.1.0 is higher priority than Websites code sharing.
And,
> although I would like to do Websites code sharing first, I don't think we can;
> realistically. Of course, I could push SCSS *and* generated CSS to
> http://adblockplus.org like I did http://acceptableads.com. But I don't really
want to. I'd
> rather just push the generated CSS from website-defaults and then replace it
> after we have implemented Websites code sharing. Maybe this is less work. But
> this decision was mostly arbitrary. And you may suggest otherwise if you think
> it makes sense to.

That's fine. I was just wondering if there was a reason that it wasn't going to
be used at all in the project. If it's quicker for you to not include it yet,
then that's fine by me :)

[ire, 2017-10-06 07:15:05]

On 2017/10/05 17:17:26, juliandoucette wrote:
> Fixed typo mentioned in footer review.

Ack.

LGTM

[juliandoucette, 2017-10-06 10:23:21]

https://bitbucket.org/adblockplus/adblockplus.org/commits/d003844664608af1652...

[ire, 2017-10-09 09:34:52]

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29564718/diff/29564719/static/css/main.css...
static/css/main.css:4: html {
On 2017/10/04 22:17:10, ire wrote:
> The formatting doesn't follow our coding style.

I realised I forgot to follow up on this. Can you fix the formatting for these
styles in a separate issue?