Issue 6047 - Updated templates and uninstalled pages

- Refactored default, minimal, and raw templates
- Deprecated simple template
- Updated uninstalled pages styles

---

Please ignore defaults.css changes in this patchset. They have been provided here for the sake of convenience and should be reviewed separately here:

- https://codereview.adblockplus.org/29612659/
- https://codereview.adblockplus.org/29612684/

[juliandoucette, 2017-11-17 19:04:17]

@Jeen @Paul:

See https://drive.google.com/open?id=1B8rxkuGDGM9qh5M7SUccJPzmfJKkIz6q

This change was meant to be an improvement, not a redesign. But I went a little
further than anticipated with my improvisation. Please let me know what you
think.

[jeen, 2017-11-20 09:18:08]

On 2017/11/17 19:04:17, juliandoucette wrote:
> @Jeen @Paul:
> 
> See https://drive.google.com/open?id=1B8rxkuGDGM9qh5M7SUccJPzmfJKkIz6q
> 
> This change was meant to be an improvement, not a redesign. But I went a
little
> further than anticipated with my improvisation. Please let me know what you
> think.

@Julian, I like the overall design and layout of the page my only suggestions
would be to: 
- Update the colour of submit button to red - since this is the intentional
primary action of the page.
- Update the blue bar to use #077CA6 (we use this blue on the Settings page).
- Update the 'Reinstall now' button to have no fill and a white outline of 2px.

[juliandoucette, 2017-11-20 15:55:47]

On 2017/11/20 09:18:08, jeen wrote:
> @Julian, I like the overall design and layout of the page my only suggestions
> would be to: 
> - Update the colour of submit button to red - since this is the intentional
> primary action of the page.
> - Update the blue bar to use #077CA6 (we use this blue on the Settings page).
> - Update the 'Reinstall now' button to have no fill and a white outline of
2px.

Thanks Jeen!

I've updated the screenhsots here
https://drive.google.com/open?id=1LvbhOvPO5wnANyK6noKhEKRKmxS9-pM4

I'm still waiting on feedback about the primary (red) button box shadow and
hover state. I will implement them as specified by Paul and proceed without
another round of design review if you trust me/him and are ok with the
screenshots above.

[juliandoucette, 2017-11-22 15:36:33]

On 2017/11/20 15:55:47, juliandoucette wrote:
> I'm still waiting on feedback about the primary (red) button box shadow and
> hover state. I will implement them as specified by Paul and proceed without
> another round of design review if you trust me/him and are ok with the
> screenshots above.

I got feedback from Paul. The latest Patchset will reflect his specifications.
Removing Paul and Jeen from CC.

[ire, 2017-11-27 19:48:22]

Thanks Julian! Here are my first impressions:

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
File pages/uninstalled-submit.html (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
pages/uninstalled-submit.html:14: <aside id="reinstall" class="bg-accent" >
Suggest: Since this is used on multiple pages, make it an include

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
pages/uninstalled-submit.html:17: {{ reinstall-headline[Text next to the
Reinstallation button] Did you uninstall Adblock Plus by accident?" }}
Unintended quote mark at the end of this sentence

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
pages/uninstalled-submit.html:18: <a href="index" class="button">
This button isn't styled correctly

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.tmpl
File pages/uninstalled.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:2: template=minimal
Suggestion: This page could have a similar to design to how the "First run page"
will eventually look (see
https://bitbucket.org/adblockplus/spec/src/80c6fff4c7370820608796a05aa3a208d6...)

i.e. mainly the grey header, not the three-column content.

I don't think (or I havent seen) any pages that would eventually look like this
page currently is once the new styles have been fully implemented.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:16: <link rel="stylesheet" href="/css/uninstalled.css"
type="text/css" media="all">
NIT: media="all" is the default, I don't think you need to write it here.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:34: <select name="otherAdblockerPredefined">
This select is also unstyled like the textarea. This appears to be due to the
.unstyled class

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:51: <textarea
There is no border around this textarea, and since the background is white, it
blends into the background

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:51: <textarea
The textarea has a fixed width so doesn't fit the screen on mobile

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:81: {{ "index" | linkify(class="button secondary") }}{{
"Reinstall Now" | translate("reinstall", "Reinstall button text") }}</a>
The text in this button is not aligned center (see it on mobile)

https://codereview.adblockplus.org/29611624/diff/29618555/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29611624/diff/29618555/static/css/main.css...
static/css/main.css:201: .button
According to the style guide, buttons should be 50px height. The width is also
specified as "4 columns", which I'm not sure how that will translate here, but I
assume this means there needs to be a fixed width that looks wider than the
current button

[juliandoucette, 2017-12-04 13:34:05]

Thanks Ire :)

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
File pages/uninstalled-submit.html (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
pages/uninstalled-submit.html:14: <aside id="reinstall" class="bg-accent" >
On 2017/11/27 19:48:20, ire wrote:
> Suggest: Since this is used on multiple pages, make it an include

Agreed.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
pages/uninstalled-submit.html:17: {{ reinstall-headline[Text next to the
Reinstallation button] Did you uninstall Adblock Plus by accident?" }}
On 2017/11/27 19:48:20, ire wrote:
> Unintended quote mark at the end of this sentence

Good catch. It's strange that this doesn't show in the website.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
pages/uninstalled-submit.html:18: <a href="index" class="button">
On 2017/11/27 19:48:21, ire wrote:
> This button isn't styled correctly

Correctly how? I styled it according to Paul's specification on IRC (not the
style guide).

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.tmpl
File pages/uninstalled.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:2: template=minimal
On 2017/11/27 19:48:21, ire wrote:
> Suggestion: This page could have a similar to design to how the "First run
page"
> will eventually look (see
>
https://bitbucket.org/adblockplus/spec/src/80c6fff4c7370820608796a05aa3a208d6...)
> 
> i.e. mainly the grey header, not the three-column content.
> 
> I don't think (or I havent seen) any pages that would eventually look like
this
> page currently is once the new styles have been fully implemented.

I agree and I've tried this already myself; unsuccessfully. I couldn't get it to
look "right" IMO.

You are welcome to give it a shot yourself.

I suggest this as the MVP and improve it separately.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:16: <link rel="stylesheet" href="/css/uninstalled.css"
type="text/css" media="all">
On 2017/11/27 19:48:22, ire wrote:
> NIT: media="all" is the default, I don't think you need to write it here.

I must have copy/pasted :(

https://codereview.adblockplus.org/29611624/diff/29618555/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29611624/diff/29618555/static/css/main.css...
static/css/main.css:201: .button
On 2017/11/27 19:48:22, ire wrote:
> According to the style guide, buttons should be 50px height. The width is also
> specified as "4 columns", which I'm not sure how that will translate here, but
I
> assume this means there needs to be a fixed width that looks wider than the
> current button

The 50px height was specified based on a larger font-size than the one that we
are using (we agreed on a smaller font-size separately). The styleguide has not
been updated to reflect this yet.

The button widths specified in columns are naive because they will not scale
down well. Furthermore, the 4 columns was only meant to apply to the install
button. Nonetheless, I think that you're right that we should consider a
min-width for buttons that is wider than the Submit button on this page.  

Paul was fine with it as-is. But I'm sure that he will also be fine with a
slightly wider button.

[ire, 2017-12-05 10:28:01]

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
File pages/uninstalled-submit.html (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
pages/uninstalled-submit.html:18: <a href="index" class="button">
On 2017/12/04 13:34:04, juliandoucette wrote:
> On 2017/11/27 19:48:21, ire wrote:
> > This button isn't styled correctly
> 
> Correctly how? I styled it according to Paul's specification on IRC (not the
> style guide).

Sorry I was going off the spec. Is Paul's change documented anywhere? (Besides
IRC)

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.tmpl
File pages/uninstalled.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:2: template=minimal
On 2017/12/04 13:34:05, juliandoucette wrote:
> On 2017/11/27 19:48:21, ire wrote:
> > Suggestion: This page could have a similar to design to how the "First run
> page"
> > will eventually look (see
> >
>
https://bitbucket.org/adblockplus/spec/src/80c6fff4c7370820608796a05aa3a208d6...)
> > 
> > i.e. mainly the grey header, not the three-column content.
> > 
> > I don't think (or I havent seen) any pages that would eventually look like
> this
> > page currently is once the new styles have been fully implemented.
> 
> I agree and I've tried this already myself; unsuccessfully. I couldn't get it
to
> look "right" IMO.
> 
> You are welcome to give it a shot yourself.

Here's my attempt - https://codereview.adblockplus.org/29630614 . I'm not 100%
sure about the way it looks either :/

> I suggest this as the MVP and improve it separately.

I agree. We should continue with your solution to get this out asap and we can
improve as the other pages are made as well

https://codereview.adblockplus.org/29611624/diff/29618555/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29611624/diff/29618555/static/css/main.css...
static/css/main.css:201: .button
On 2017/12/04 13:34:05, juliandoucette wrote:
> On 2017/11/27 19:48:22, ire wrote:
> > According to the style guide, buttons should be 50px height. The width is
also
> > specified as "4 columns", which I'm not sure how that will translate here,
but
> I
> > assume this means there needs to be a fixed width that looks wider than the
> > current button
> 
> The 50px height was specified based on a larger font-size than the one that we
> are using (we agreed on a smaller font-size separately). The styleguide has
not
> been updated to reflect this yet.
> 
> The button widths specified in columns are naive because they will not scale
> down well. Furthermore, the 4 columns was only meant to apply to the install
> button. Nonetheless, I think that you're right that we should consider a
> min-width for buttons that is wider than the Submit button on this page.  
> 
> Paul was fine with it as-is. But I'm sure that he will also be fine with a
> slightly wider button.

Alright, if Paul is fine with it then that's fine with me.

[juliandoucette, 2017-12-06 16:05:07]

Thanks Ire, I will update and separate this a bit.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
File pages/uninstalled-submit.html (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
pages/uninstalled-submit.html:18: <a href="index" class="button">
On 2017/12/05 10:28:00, ire wrote:
> Sorry I was going off the spec. Is Paul's change documented anywhere? (Besides
> IRC)

Unfortunately not.

[juliandoucette, 2017-12-06 16:30:54]

Updated.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
File pages/uninstalled-submit.html (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
pages/uninstalled-submit.html:17: {{ reinstall-headline[Text next to the
Reinstallation button] Did you uninstall Adblock Plus by accident?" }}
On 2017/12/04 13:34:04, juliandoucette wrote:
> On 2017/11/27 19:48:20, ire wrote:
> > Unintended quote mark at the end of this sentence
> 
> Good catch. It's strange that this doesn't show in the website.

Done.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.tmpl
File pages/uninstalled.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:2: template=minimal
On 2017/12/05 10:28:00, ire wrote:
> Here's my attempt - https://codereview.adblockplus.org/29630614 . I'm not 100%
> sure about the way it looks either :/

That's a good start.

> I agree. We should continue with your solution to get this out asap and we can
> improve as the other pages are made as well

:+1:

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:16: <link rel="stylesheet" href="/css/uninstalled.css"
type="text/css" media="all">
On 2017/12/04 13:34:04, juliandoucette wrote:
> On 2017/11/27 19:48:22, ire wrote:
> > NIT: media="all" is the default, I don't think you need to write it here.
> 
> I must have copy/pasted :(

Done.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:34: <select name="otherAdblockerPredefined">
On 2017/11/27 19:48:21, ire wrote:
> This select is also unstyled like the textarea. This appears to be due to the
> .unstyled class

Done.

Good catch.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:51: <textarea
On 2017/11/27 19:48:21, ire wrote:
> There is no border around this textarea, and since the background is white, it
> blends into the background

Done.

Good catch.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:81: {{ "index" | linkify(class="button secondary") }}{{
"Reinstall Now" | translate("reinstall", "Reinstall button text") }}</a>
On 2017/11/27 19:48:21, ire wrote:
> The text in this button is not aligned center (see it on mobile)

Done.

Good catch.

[ire, 2017-12-07 12:36:27]

Thanks Julian!

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
File pages/uninstalled-submit.html (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
pages/uninstalled-submit.html:14: <aside id="reinstall" class="bg-accent" >
On 2017/12/04 13:34:04, juliandoucette wrote:
> On 2017/11/27 19:48:20, ire wrote:
> > Suggest: Since this is used on multiple pages, make it an include
> 
> Agreed.

You haven't done this yet.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.tmpl
File pages/uninstalled.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:51: <textarea
On 2017/11/27 19:48:21, ire wrote:
> The textarea has a fixed width so doesn't fit the screen on mobile

This hasn't been fixed yet.

https://codereview.adblockplus.org/29611624/diff/29631675/includes/html-attri...
File includes/html-attributes.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29631675/includes/html-attri...
includes/html-attributes.tmpl:1: lang="{{ locale }}"
This include doesn't seem to be working. None of these attributes are applying

https://codereview.adblockplus.org/29611624/diff/29631675/pages/uninstalled.tmpl
File pages/uninstalled.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29631675/pages/uninstalled.t...
pages/uninstalled.tmpl:20: <div class="container content">
Looks like this element is missing the #content ID. It seems like there should
be more space between the end of the content and the reinstall banner

https://codereview.adblockplus.org/29611624/diff/29631675/pages/uninstalled.t...
pages/uninstalled.tmpl:87: <script src="/js/uninstalled.js"></script>
Suggest: Implement a scripts block?

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/main.css...
static/css/main.css:241: .bg-accent .button.secondary
NIT: I think this is a style of button that could exist outside of the
.bg-accent. I think it makes sense to have this be it's own style of button,
e.g. .button.secondary-o , instead of a .button.secondary that is within a
.bg-accent.

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/uninstal...
File static/css/uninstalled.css (right):

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/uninstal...
static/css/uninstalled.css:11: margin: 1em 0;
I don't think this is doing anything

[juliandoucette, 2017-12-08 16:49:53]

Updated.

https://codereview.adblockplus.org/29611624/diff/29631675/includes/html-attri...
File includes/html-attributes.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29631675/includes/html-attri...
includes/html-attributes.tmpl:1: lang="{{ locale }}"
On 2017/12/07 12:36:26, ire wrote:
> This include doesn't seem to be working. None of these attributes are applying

Apparently this was mistaken for page meta data :D

I'll file a bug and add a space to make this work for now.

https://codereview.adblockplus.org/29611624/diff/29631675/pages/uninstalled.tmpl
File pages/uninstalled.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29631675/pages/uninstalled.t...
pages/uninstalled.tmpl:20: <div class="container content">
On 2017/12/07 12:36:26, ire wrote:
> Looks like this element is missing the #content ID. It seems like there should
> be more space between the end of the content and the reinstall banner

Done.

Good catch.

https://codereview.adblockplus.org/29611624/diff/29631675/pages/uninstalled.t...
pages/uninstalled.tmpl:87: <script src="/js/uninstalled.js"></script>
On 2017/12/07 12:36:26, ire wrote:
> Suggest: Implement a scripts block?

Acknowledged.

Separately.

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/main.css...
static/css/main.css:241: .bg-accent .button.secondary
On 2017/12/07 12:36:27, ire wrote:
> NIT: I think this is a style of button that could exist outside of the
> .bg-accent. I think it makes sense to have this be it's own style of button,
> e.g. .button.secondary-o , instead of a .button.secondary that is within a
> .bg-accent. 

What about .button.inverse?

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/uninstal...
File static/css/uninstalled.css (right):

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/uninstal...
static/css/uninstalled.css:11: margin: 1em 0;
On 2017/12/07 12:36:27, ire wrote:
> I don't think this is doing anything

It adds space above/below the button on mobile. It's ~necessary because the
button is inline on desktop and block on mobile.

https://codereview.adblockplus.org/29611624/diff/29633758/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29611624/diff/29633758/static/css/main.css...
static/css/main.css:242: a.button.inverse
necessary to be more specific than `.content a` :/

[ire, 2017-12-11 15:53:49]

I think you missed these comments:

On 2017/12/07 12:36:27, ire wrote:
> Thanks Julian!
> 
>
https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
> File pages/uninstalled-submit.html (right):
> 
>
https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
> pages/uninstalled-submit.html:14: <aside id="reinstall" class="bg-accent" >
> On 2017/12/04 13:34:04, juliandoucette wrote:
> > On 2017/11/27 19:48:20, ire wrote:
> > > Suggest: Since this is used on multiple pages, make it an include
> > 
> > Agreed.
> 
> You haven't done this yet.
> 
>
https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.tmpl
> File pages/uninstalled.tmpl (right):
> 
>
https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
> pages/uninstalled.tmpl:51: <textarea
> On 2017/11/27 19:48:21, ire wrote:
> > The textarea has a fixed width so doesn't fit the screen on mobile
> 
> This hasn't been fixed yet.


-----

https://codereview.adblockplus.org/29611624/diff/29631675/includes/html-attri...
File includes/html-attributes.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29631675/includes/html-attri...
includes/html-attributes.tmpl:1: lang="{{ locale }}"
On 2017/12/08 16:49:51, juliandoucette wrote:
> On 2017/12/07 12:36:26, ire wrote:
> > This include doesn't seem to be working. None of these attributes are
applying
> 
> Apparently this was mistaken for page meta data :D
> 
> I'll file a bug and add a space to make this work for now.

Ah! Ack.

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/main.css...
static/css/main.css:241: .bg-accent .button.secondary
On 2017/12/08 16:49:52, juliandoucette wrote:
> On 2017/12/07 12:36:27, ire wrote:
> > NIT: I think this is a style of button that could exist outside of the
> > .bg-accent. I think it makes sense to have this be it's own style of button,
> > e.g. .button.secondary-o , instead of a .button.secondary that is within a
> > .bg-accent. 
> 
> What about .button.inverse?

That works now, but what if we wanted to have an "inverse" button that uses the
primary colours instead? 

(This is a NIT, if we ever do come across this situation we can handle it then)

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/uninstal...
File static/css/uninstalled.css (right):

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/uninstal...
static/css/uninstalled.css:11: margin: 1em 0;
On 2017/12/08 16:49:52, juliandoucette wrote:
> On 2017/12/07 12:36:27, ire wrote:
> > I don't think this is doing anything
> 
> It adds space above/below the button on mobile. It's ~necessary because the
> button is inline on desktop and block on mobile.

Acknowledged.

https://codereview.adblockplus.org/29611624/diff/29633758/pages/uninstalled-s...
File pages/uninstalled-submit.html (right):

https://codereview.adblockplus.org/29611624/diff/29633758/pages/uninstalled-s...
pages/uninstalled-submit.html:19: {{ reinstall[Reinstall button text] Reinstall
Now }}
This button doesn't look like the one on the uninstalled page. The text is red
and there is no border.

https://codereview.adblockplus.org/29611624/diff/29633758/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29611624/diff/29633758/static/css/main.css...
static/css/main.css:242: a.button.inverse
On 2017/12/08 16:49:52, juliandoucette wrote:
> necessary to be more specific than `.content a` :/

Acknowledged.

[juliandoucette, 2017-12-14 18:35:23]

Thanks Ire!

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
File pages/uninstalled-submit.html (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled-s...
pages/uninstalled-submit.html:14: <aside id="reinstall" class="bg-accent" >
On 2017/12/07 12:36:26, ire wrote:
> On 2017/12/04 13:34:04, juliandoucette wrote:
> > On 2017/11/27 19:48:20, ire wrote:
> > > Suggest: Since this is used on multiple pages, make it an include
> > 
> > Agreed.
> 
> You haven't done this yet.

Done.

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.tmpl
File pages/uninstalled.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29618555/pages/uninstalled.t...
pages/uninstalled.tmpl:51: <textarea
On 2017/12/07 12:36:26, ire wrote:
> On 2017/11/27 19:48:21, ire wrote:
> > The textarea has a fixed width so doesn't fit the screen on mobile
> 
> This hasn't been fixed yet.

Done.

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/main.css
File static/css/main.css (right):

https://codereview.adblockplus.org/29611624/diff/29631675/static/css/main.css...
static/css/main.css:241: .bg-accent .button.secondary
On 2017/12/11 15:53:46, ire wrote:
> That works now, but what if we wanted to have an "inverse" button that uses
the
> primary colours instead? 
> 
> (This is a NIT, if we ever do come across this situation we can handle it
then)

Will address separately if necessary.

https://codereview.adblockplus.org/29611624/diff/29633758/pages/uninstalled-s...
File pages/uninstalled-submit.html (right):

https://codereview.adblockplus.org/29611624/diff/29633758/pages/uninstalled-s...
pages/uninstalled-submit.html:19: {{ reinstall[Reinstall button text] Reinstall
Now }}
On 2017/12/11 15:53:48, ire wrote:
> This button doesn't look like the one on the uninstalled page. The text is red
> and there is no border. 

Done.

[ire, 2017-12-18 08:43:09]

LGTM + NITs

https://codereview.adblockplus.org/29611624/diff/29639595/includes/reinstall....
File includes/reinstall.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29639595/includes/reinstall....
includes/reinstall.tmpl:1: <aside id="reinstall" class="bg-accent" >
NIT: Extra space before closing ">"

https://codereview.adblockplus.org/29611624/diff/29639595/includes/reinstall....
includes/reinstall.tmpl:5: {{ "index" | linkify(class="button inverse") }}{{
"Reinstall Now" | translate("reinstall", "Reinstall button text") }}</a>
There's no closing tag for the <p> here 

(but for some reason it's added to the generated site. does the cms do this?)

[juliandoucette, 2017-12-18 12:58:12]

Thanks Ire, Fixed.

https://codereview.adblockplus.org/29611624/diff/29639595/includes/reinstall....
File includes/reinstall.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29639595/includes/reinstall....
includes/reinstall.tmpl:1: <aside id="reinstall" class="bg-accent" >
On 2017/12/18 08:43:07, ire wrote:
> NIT: Extra space before closing ">"

Acknowledged.

https://codereview.adblockplus.org/29611624/diff/29639595/includes/reinstall....
includes/reinstall.tmpl:5: {{ "index" | linkify(class="button inverse") }}{{
"Reinstall Now" | translate("reinstall", "Reinstall button text") }}</a>
On 2017/12/18 08:43:07, ire wrote:
> There's no closing tag for the <p> here 
> 
> (but for some reason it's added to the generated site. does the cms do this?)

Nope (check the source in your browser). Closing tags are optional in HTML5
(most of the time) / modern browsers have pretty good broken HTML heuristics. 

Anyway, thank you for pointing this out! I will fix it.

[ire, 2017-12-20 08:12:59]

(forgot to publish this)

https://codereview.adblockplus.org/29611624/diff/29639595/includes/reinstall....
File includes/reinstall.tmpl (right):

https://codereview.adblockplus.org/29611624/diff/29639595/includes/reinstall....
includes/reinstall.tmpl:5: {{ "index" | linkify(class="button inverse") }}{{
"Reinstall Now" | translate("reinstall", "Reinstall button text") }}</a>
On 2017/12/18 12:58:11, juliandoucette wrote:
> On 2017/12/18 08:43:07, ire wrote:
> > There's no closing tag for the <p> here 
> > 
> > (but for some reason it's added to the generated site. does the cms do
this?)
> 
> Nope (check the source in your browser). Closing tags are optional in HTML5
> (most of the time) / modern browsers have pretty good broken HTML heuristics. 
> 
> Anyway, thank you for pointing this out! I will fix it.

Cool, TIL!

[juliandoucette, 2017-12-20 18:18:28]

https://hg.adblockplus.org/web.adblockplus.org/rev/2d47b1d5f75c