Noissue - Add option to enable large desktop

Noissue - Add option to enable large desktop

[ire, 2017-10-17 08:32:10]

Can't figure out why all these files were uploaded. The only two files I
actually changed were:

_base.scss and _variables.scss

[ire, 2017-10-17 08:33:45]

https://codereview.adblockplus.org/29581555/diff/29581556/static/scss/_base.scss
File static/scss/_base.scss (right):

https://codereview.adblockplus.org/29581555/diff/29581556/static/scss/_base.s...
static/scss/_base.scss:66: @if $enable-large-desktop
I also added this

https://codereview.adblockplus.org/29581555/diff/29581556/static/scss/_variab...
File static/scss/_variables.scss (right):

https://codereview.adblockplus.org/29581555/diff/29581556/static/scss/_variab...
static/scss/_variables.scss:87: $enable-large-desktop: true !default;
I added this

[juliandoucette, 2017-10-17 11:05:51]

On 2017/10/17 08:32:10, ire wrote:
> Can't figure out why all these files were uploaded. The only two files I
> actually changed were:
> 
> _base.scss and _variables.scss

You probably diffed against `master` which hasn't been moved forward (oops).

[juliandoucette, 2017-10-17 11:07:23]

On 2017/10/17 11:05:51, juliandoucette wrote:
> You probably diffed against `master` which hasn't been moved forward (oops).

I just pushed master forward. You can `hg up` and then re-upload.

[juliandoucette, 2017-10-17 11:08:40]

Why make *only* large desktop conditional?

[ire, 2017-10-17 15:06:32]

On 2017/10/17 11:08:40, juliandoucette wrote:
> Why make *only* large desktop conditional?

That's just the only one that doesn't need to be used right now. I can imagine
us wanting to make the mobile conditional too, but we can add that if/when we
need it

[juliandoucette, 2017-10-17 15:26:25]

On 2017/10/17 15:06:32, ire wrote:
> That's just the only one that doesn't need to be used right now. I can imagine
> us wanting to make the mobile conditional too, but we can add that if/when we
> need it

Agreed. LGTM. See https://codereview.adblockplus.org/29581874/ first though (if
we make that change then this one may not be necessary).

[ire, 2017-10-17 17:32:51]

On 2017/10/17 15:26:25, juliandoucette wrote:
> On 2017/10/17 15:06:32, ire wrote:
> > That's just the only one that doesn't need to be used right now. I can
imagine
> > us wanting to make the mobile conditional too, but we can add that if/when
we
> > need it
> 
> Agreed. LGTM. See https://codereview.adblockplus.org/29581874/ first though
(if
> we make that change then this one may not be necessary).

I'll wait until that review is resolved. This will still be relevant because we
may want the $container-max-width to be $desktop-width if $enable-large-desktop
is false

[juliandoucette, 2017-10-26 15:10:20]

On 2017/10/17 17:32:51, ire wrote:
> I'll wait until that review is resolved. This will still be relevant because
we
> may want the $container-max-width to be $desktop-width if
$enable-large-desktop
> is false

Do you still want this?

[ire, 2017-10-27 08:49:29]

On 2017/10/26 15:10:20, juliandoucette wrote:
> On 2017/10/17 17:32:51, ire wrote:
> > I'll wait until that review is resolved. This will still be relevant because
> we
> > may want the $container-max-width to be $desktop-width if
> $enable-large-desktop
> > is false
> 
> Do you still want this?

No its no longer necessary. Closing this review