MD Settings: Remove last usage of paper-item.

MD Settings: Remove last usage of paper-item.

 - Replace <paper-item> usage with a styled <button>.
 - Remove all references to paper-item shared style files.
 - Move iron-dropdown rules to cups_add_printer_dialog_util.html, since
   it has the only usage of iron-dropdown.

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

Review-Url: https://codereview.chromium.org/2677863004
Cr-Commit-Position: refs/heads/master@{#448534}
Committed: https://chromium.googlesource.com/chromium/src/+/6477751a2c1a2c3fba3ba77917efd51f48b0e2ab

[dpapad, 2017-02-06 20:52:31]

Description was changed from

==========
MD Settings: Remove last usage of paper-item.

BUG=
==========

to

==========
MD Settings: Remove last usage of paper-item.

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

[dpapad, 2017-02-06 20:52:35]

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

[commit-bot: I haz the power, 2017-02-06 20:53:16]

Dry run: 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, 2017-02-06 21:05:24]

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

[commit-bot: I haz the power, 2017-02-06 21:05:25]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[dpapad, 2017-02-06 23:08:04]

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

[dpapad, 2017-02-06 23:09:04]

Description was changed from

==========
MD Settings: Remove last usage of paper-item.

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

to

==========
MD Settings: Remove last usage of paper-item.

 - Replace <paper-item> usage with a styled <button>.
 - Remove all references to paper-item from settings_shared_css.html

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

[commit-bot: I haz the power, 2017-02-06 23:09:19]

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

[dpapad, 2017-02-06 23:10:27]

Description was changed from

==========
MD Settings: Remove last usage of paper-item.

 - Replace <paper-item> usage with a styled <button>.
 - Remove all references to paper-item from settings_shared_css.html

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

to

==========
MD Settings: Remove last usage of paper-item.

 - Replace <paper-item> usage with a styled <button>.
 - Remove all references to paper-item shared style files.
 - Move iron-dropdown rules to cups_add_printer_dialog_util.html, since
   it has the only usage of iron-dropdown.

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

[dpapad, 2017-02-06 23:15:39]

Description was changed from

==========
MD Settings: Remove last usage of paper-item.

 - Replace <paper-item> usage with a styled <button>.
 - Remove all references to paper-item shared style files.
 - Move iron-dropdown rules to cups_add_printer_dialog_util.html, since
   it has the only usage of iron-dropdown.

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

to

==========
MD Settings: Remove last usage of paper-item.

 - Replace <paper-item> usage with a styled <button>.
 - Remove all references to paper-item shared style files.
 - Move iron-dropdown rules to cups_add_printer_dialog_util.html, since
   it has the only usage of iron-dropdown.

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

[dpapad, 2017-02-06 23:33:06]

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

[commit-bot: I haz the power, 2017-02-06 23:33:50]

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

[dpapad, 2017-02-07 00:02:35]

dpapad@chromium.org changed reviewers:
+ stevenjb@chromium.org

[dpapad, 2017-02-07 00:02:36]

After screenshots: http://imgur.com/a/BRkJt

[stevenjb, 2017-02-07 00:10:43]

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

[stevenjb, 2017-02-07 00:10:43]

Issue should be 603976

+xdai@

[stevenjb, 2017-02-07 00:13:08]

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

https://codereview.chromium.org/2677863004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:25:
@apply(--settings-actionable);
This seems like a lot of unfortunatel customization. Also, Do we need a style
include to apply --settings-actionable?

https://codereview.chromium.org/2677863004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:34:
</dom-module>
nit: blank line

[dpapad, 2017-02-07 00:13:10]

Description was changed from

==========
MD Settings: Remove last usage of paper-item.

 - Replace <paper-item> usage with a styled <button>.
 - Remove all references to paper-item shared style files.
 - Move iron-dropdown rules to cups_add_printer_dialog_util.html, since
   it has the only usage of iron-dropdown.

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

to

==========
MD Settings: Remove last usage of paper-item.

 - Replace <paper-item> usage with a styled <button>.
 - Remove all references to paper-item shared style files.
 - Move iron-dropdown rules to cups_add_printer_dialog_util.html, since
   it has the only usage of iron-dropdown.

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

[xdai1, 2017-02-07 00:19:08]

lgtm
Thanks for fixing this!

[dpapad, 2017-02-07 00:22:58]

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

https://codereview.chromium.org/2677863004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:25:
@apply(--settings-actionable);
On 2017/02/07 at 00:13:08, stevenjb wrote:
> This seems like a lot of unfortunatel customization. Also, Do we need a style
include to apply --settings-actionable?

This is copied from cr-action-menu
https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_action....
Unfortunately we can't reuse cr-action-menu (which was my original suggestion).

It is less unfortunate IMO than including paper-item and iron-dropdown styles in
settings_shared_css.html, which gets injected to every single element we use.
The fact that Vulcanize/polymer-css-build inline all included styles N times
makes the problem even worse.

So, although I agree that repeated custom style is not ideal, I think the
overall tradeoff is beneficial here.

https://codereview.chromium.org/2677863004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:34:
</dom-module>
On 2017/02/07 at 00:13:08, stevenjb wrote:
> nit: blank line

Done.

[dpapad, 2017-02-07 00:25:01]

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

https://codereview.chromium.org/2677863004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:25:
@apply(--settings-actionable);

> So, although I agree that repeated custom style is not ideal, I think the
overall tradeoff is beneficial here.

Regarding -settings-actionable, it seems to work without an include (note that
it is defined in settings_vars_css.html in a ":root" selector, not in
settings_shared_css.html).

[stevenjb, 2017-02-07 00:27:12]

lgtm with one suggestion

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

https://codereview.chromium.org/2677863004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:25:
@apply(--settings-actionable);
On 2017/02/07 00:22:58, dpapad wrote:
> On 2017/02/07 at 00:13:08, stevenjb wrote:
> > This seems like a lot of unfortunatel customization. Also, Do we need a
style
> include to apply --settings-actionable?
> 
> This is copied from cr-action-menu
>
https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_action....
> Unfortunately we can't reuse cr-action-menu (which was my original
suggestion).
> 
> It is less unfortunate IMO than including paper-item and iron-dropdown styles
in
> settings_shared_css.html, which gets injected to every single element we use.
> The fact that Vulcanize/polymer-css-build inline all included styles N times
> makes the problem even worse.
> 
> So, although I agree that repeated custom style is not ideal, I think the
> overall tradeoff is beneficial here.

OK, that's fair. We should probably audit settings_shared at some point and
remove some infrequently used CSS (or use custom vars instead which I would
expect to be cheaper?)

https://codereview.chromium.org/2677863004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:25:
@apply(--settings-actionable);
On 2017/02/07 00:25:01, dpapad wrote:
> 
> > So, although I agree that repeated custom style is not ideal, I think the
> overall tradeoff is beneficial here.
> 
> Regarding -settings-actionable, it seems to work without an include (note that
> it is defined in settings_vars_css.html in a ":root" selector, not in
> settings_shared_css.html).

OK, maybe include settings_shared_css explicitly then?

[dpapad, 2017-02-07 00:56:28]

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

https://codereview.chromium.org/2677863004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:25:
@apply(--settings-actionable);
On 2017/02/07 at 00:27:12, stevenjb wrote:
> On 2017/02/07 00:25:01, dpapad wrote:
> > 
> > > So, although I agree that repeated custom style is not ideal, I think the
> > overall tradeoff is beneficial here.
> > 
> > Regarding -settings-actionable, it seems to work without an include (note
that
> > it is defined in settings_vars_css.html in a ":root" selector, not in
> > settings_shared_css.html).
> 
> OK, maybe include settings_shared_css explicitly then?

I think you meant settings_var_css instead. Done.

Regarding auditing settings_shared at some point, yes we could do that and move
shared styles that are only used by few cases accordingly. Similarly, I had
advocated in the past for more targeted shared styles, similar to
md_select_css.html, such that there is more granularity on what is included by
each element.

[stevenjb, 2017-02-07 01:11:06]

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

https://codereview.chromium.org/2677863004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:25:
@apply(--settings-actionable);
On 2017/02/07 00:27:12, stevenjb wrote:
> On 2017/02/07 00:25:01, dpapad wrote:
> > 
> > > So, although I agree that repeated custom style is not ideal, I think the
> > overall tradeoff is beneficial here.
> > 
> > Regarding -settings-actionable, it seems to work without an include (note
that
> > it is defined in settings_vars_css.html in a ":root" selector, not in
> > settings_shared_css.html).
> 
> OK, maybe include settings_shared_css explicitly then?

Yup :)

https://codereview.chromium.org/2677863004/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/printing_page/cups_add_printer_dialog_util.html:25:
@apply(--settings-actionable);
On 2017/02/07 00:56:28, dpapad wrote:
> On 2017/02/07 at 00:27:12, stevenjb wrote:
> > On 2017/02/07 00:25:01, dpapad wrote:
> > > 
> > > > So, although I agree that repeated custom style is not ideal, I think
the
> > > overall tradeoff is beneficial here.
> > > 
> > > Regarding -settings-actionable, it seems to work without an include (note
> that
> > > it is defined in settings_vars_css.html in a ":root" selector, not in
> > > settings_shared_css.html).
> > 
> > OK, maybe include settings_shared_css explicitly then?
> 
> I think you meant settings_var_css instead. Done.
> 
> Regarding auditing settings_shared at some point, yes we could do that and
move
> shared styles that are only used by few cases accordingly. Similarly, I had
> advocated in the past for more targeted shared styles, similar to
> md_select_css.html, such that there is more granularity on what is included by
> each element.

SGTM

[dpapad, 2017-02-07 01:18:48]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2017-02-07 01:18:48]

The patchset sent to the CQ was uploaded after l-g-t-m from xdai@chromium.org,
stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2677863004/#ps80001
(title: "Nit.")

[commit-bot: I haz the power, 2017-02-07 01:19:46]

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, 2017-02-07 03:18:42]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1486430328360810,
"parent_rev": "64348506a8b488017055340cedc9ce27bbff832d", "commit_rev":
"6477751a2c1a2c3fba3ba77917efd51f48b0e2ab"}

[commit-bot: I haz the power, 2017-02-07 03:19:18]

Description was changed from

==========
MD Settings: Remove last usage of paper-item.

 - Replace <paper-item> usage with a styled <button>.
 - Remove all references to paper-item shared style files.
 - Move iron-dropdown rules to cups_add_printer_dialog_util.html, since
   it has the only usage of iron-dropdown.

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

to

==========
MD Settings: Remove last usage of paper-item.

 - Replace <paper-item> usage with a styled <button>.
 - Remove all references to paper-item shared style files.
 - Move iron-dropdown rules to cups_add_printer_dialog_util.html, since
   it has the only usage of iron-dropdown.

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

Review-Url: https://codereview.chromium.org/2677863004
Cr-Commit-Position: refs/heads/master@{#448534}
Committed:
https://chromium.googlesource.com/chromium/src/+/6477751a2c1a2c3fba3ba77917ef...
==========

[commit-bot: I haz the power, 2017-02-07 03:19:20]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/6477751a2c1a2c3fba3ba77917ef...