[CUPS] Implement the Add Printer dialogs.

[CUPS] Implement the Add Printer dialogs.
- Implement the discovery dialog.
- Implement the manually add printer dialog.
- Implement the configuring-in-progress dialog.

Note the APIs is not ready yet, So these are just UI dialogs and not
functional.

BUG=626752
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/f5c87797ec6b0b20b40e599b2cba2f6971280ce3
Cr-Commit-Position: refs/heads/master@{#415983}

[xdai1, 2016-08-26 23:53:45]

The CQ bit was checked by xdai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-26 23:54:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xdai1, 2016-08-26 23:58:18]

xdai@chromium.org changed reviewers:
+ michaelpg@chromium.org

[xdai1, 2016-08-26 23:58:18]

michaelpg@, could you help review this CL please? Thanks!

[commit-bot: I haz the power, 2016-08-27 01:09:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-27 01:09:39]

Dry run: This issue passed the CQ dry run.

[michaelpg, 2016-08-29 17:10:33]

https://codereview.chromium.org/2281353002/diff/1/chrome/app/settings_strings...
File chrome/app/settings_strings.grdp (right):

https://codereview.chromium.org/2281353002/diff/1/chrome/app/settings_strings...
chrome/app/settings_strings.grdp:768: <message
name="IDS_SETTINGS_PRINTING_CUPS_PRINTER_DETAILS_QUEUE" desc="Lable for the CUPS
printer queue in the manually add printer dialog.">
Label

https://codereview.chromium.org/2281353002/diff/1/chrome/app/settings_strings...
chrome/app/settings_strings.grdp:783: <message
name="IDS_SETTINGS_PRINTING_CUPS_PRINTER_PROTOCOL_IPP" desc="The dropdown menu
of the printer protocol on the manully add printer dialog.">
update these descriptions

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html
(right):

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html:9:
<style include="settings-shared"></style>
You can merge this with the <style> below, you don't need a separate style
element for it:

<style include="foo bar baz">stuff { color: lemonchiffon; }</style>

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js
(right):

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:6: *
@fileoverview 'settings-cups-add-printer-dialog' is the dialog for setting up
update this comment to explain the multi-dialog setup

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:204:
}.bind(this));
no need to .bind(this) -- this.async calls the callback with "this" as the
context by default

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:207:
/** @private */
document @return type

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:213:
return null;
why not ''?

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html
(right):

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:21:

nit: remove trailing blank line

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:61:
<content select=":not(paper-button)"></content>
IMO this is a bit dangerous -- if I try to add a <paper-button> in the dialog,
it wouldn't be obvious to me why it shows up in the wrong place. I'd suggest
using a class or a containing div instead, but it's up to you.

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.js
(right):

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.js:34:
is: 'add-printer-dialog-template',
please find another name -- <template> has a very specific meaning in HTML and
Polymer :-)

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.js:38:
this.$$('#dialog').showModal();
this.$.dialog

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.js:42:
this.$$('#dialog').close();
same

[xdai1, 2016-08-29 18:45:46]

michaelpg@, I've addressed your comments. Please take another look, thanks!

https://codereview.chromium.org/2281353002/diff/1/chrome/app/settings_strings...
File chrome/app/settings_strings.grdp (right):

https://codereview.chromium.org/2281353002/diff/1/chrome/app/settings_strings...
chrome/app/settings_strings.grdp:768: <message
name="IDS_SETTINGS_PRINTING_CUPS_PRINTER_DETAILS_QUEUE" desc="Lable for the CUPS
printer queue in the manually add printer dialog.">
On 2016/08/29 17:10:32, michaelpg wrote:
> Label

Done.

https://codereview.chromium.org/2281353002/diff/1/chrome/app/settings_strings...
chrome/app/settings_strings.grdp:783: <message
name="IDS_SETTINGS_PRINTING_CUPS_PRINTER_PROTOCOL_IPP" desc="The dropdown menu
of the printer protocol on the manully add printer dialog.">
On 2016/08/29 17:10:32, michaelpg wrote:
> update these descriptions

Done.

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html
(right):

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html:9:
<style include="settings-shared"></style>
On 2016/08/29 17:10:32, michaelpg wrote:
> You can merge this with the <style> below, you don't need a separate style
> element for it:
> 
> <style include="foo bar baz">stuff { color: lemonchiffon; }</style>

Done. Also modified all the other pages in the settings/printing_page directory.

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js
(right):

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:6: *
@fileoverview 'settings-cups-add-printer-dialog' is the dialog for setting up
On 2016/08/29 17:10:32, michaelpg wrote:
> update this comment to explain the multi-dialog setup

Done.

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:204:
}.bind(this));
On 2016/08/29 17:10:32, michaelpg wrote:
> no need to .bind(this) -- this.async calls the callback with "this" as the
> context by default

Done.

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:207:
/** @private */
On 2016/08/29 17:10:33, michaelpg wrote:
> document @return type

Done.

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:213:
return null;
On 2016/08/29 17:10:32, michaelpg wrote:
> why not ''?

Oh you're right. Thanks!

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html
(right):

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:21:

On 2016/08/29 17:10:33, michaelpg wrote:
> nit: remove trailing blank line

Done.

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:61:
<content select=":not(paper-button)"></content>
On 2016/08/29 17:10:33, michaelpg wrote:
> IMO this is a bit dangerous -- if I try to add a <paper-button> in the dialog,
> it wouldn't be obvious to me why it shows up in the wrong place. I'd suggest
> using a class or a containing div instead, but it's up to you.

Thanks! This logic works for this particular setup flow. But I still modified it
as you suggested to avoid future problem.

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.js
(right):

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.js:34:
is: 'add-printer-dialog-template',
On 2016/08/29 17:10:33, michaelpg wrote:
> please find another name -- <template> has a very specific meaning in HTML and
> Polymer :-)

Done.

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.js:38:
this.$$('#dialog').showModal();
On 2016/08/29 17:10:33, michaelpg wrote:
> this.$.dialog

Done.

https://codereview.chromium.org/2281353002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.js:42:
this.$$('#dialog').close();
On 2016/08/29 17:10:33, michaelpg wrote:
> same

Done.

[michaelpg, 2016-08-31 16:59:57]

lgtm

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html
(right):

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html:72:
--paper-input-container-color: rgb(192, 199, 205);
where does this color come from? can we use any existing variables?

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js
(right):

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:116:
this.$.configuringMessage.textContent = loadTimeData.getStringF(
import i18n_setup.html for loadTimeData

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html
(right):

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:61:
<content select=".dialog-button"></content>
nit: make the class "dialog-buttons" (plural)

[xdai1, 2016-09-01 00:00:19]

michaelpg@, I've addressed your comments. Thanks for your review!

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html
(right):

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.html:72:
--paper-input-container-color: rgb(192, 199, 205);
On 2016/08/31 16:59:57, michaelpg wrote:
> where does this color come from? can we use any existing variables?

I manually picked the color to best match the color in the mock. I modified it
to use the existing google color variables as you suggested.

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js
(right):

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js:116:
this.$.configuringMessage.textContent = loadTimeData.getStringF(
On 2016/08/31 16:59:57, michaelpg wrote:
> import i18n_setup.html for loadTimeData

Done.

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html
(right):

https://codereview.chromium.org/2281353002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:61:
<content select=".dialog-button"></content>
On 2016/08/31 16:59:57, michaelpg wrote:
> nit: make the class "dialog-buttons" (plural)

Done.

[xdai1, 2016-09-01 00:00:27]

The CQ bit was checked by xdai@chromium.org

[xdai1, 2016-09-01 00:00:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org
Link to the patchset: https://codereview.chromium.org/2281353002/#ps40001
(title: "Address michaelpg@'s comments.")

[commit-bot: I haz the power, 2016-09-01 00:01:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-01 01:20:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-01 01:20:47]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[xdai1, 2016-09-01 16:21:55]

The CQ bit was checked by xdai@chromium.org

[commit-bot: I haz the power, 2016-09-01 16:22:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-01 17:15:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-01 17:17:30]

Description was changed from

==========
[CUPS] Implement the Add Printer dialogs.
- Implement the discovery dialog.
- Implement the manually add printer dialog.
- Implement the configuring-in-progress dialog.

Note the APIs is not ready yet, So these are just UI dialogs and not
functional.

BUG=626752
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[CUPS] Implement the Add Printer dialogs.
- Implement the discovery dialog.
- Implement the manually add printer dialog.
- Implement the configuring-in-progress dialog.

Note the APIs is not ready yet, So these are just UI dialogs and not
functional.

BUG=626752
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/f5c87797ec6b0b20b40e599b2cba2f6971280ce3
Cr-Commit-Position: refs/heads/master@{#415983}
==========

[commit-bot: I haz the power, 2016-09-01 17:17:31]

Patchset 3 (id:??) landed as
https://crrev.com/f5c87797ec6b0b20b40e599b2cba2f6971280ce3
Cr-Commit-Position: refs/heads/master@{#415983}