Switch MD Settings to paper-dropdown-menu-light

Switch MD Settings to paper-dropdown-menu-light

paper-dropdown-menu-light is similar to paper-dropdown-menu but uses
a native <input> directly instead of a <paper-input>.

BUG=602896
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/7a572e07f74c26470f9651cb39161fbcab67d0ec
Cr-Commit-Position: refs/heads/master@{#415049}

[michaelpg, 2016-08-06 00:34:00]

Description was changed from

==========
Switch MD Settings to paper-dropdown-menu-light

paper-dropdown-menu-light is similar to paper-dropdown-menu but uses
a native <input> directly instead of a <paper-input>.

BUG=602896
R=stevenjb@chromium.org
==========

to

==========
Switch MD Settings to paper-dropdown-menu-light

paper-dropdown-menu-light is similar to paper-dropdown-menu but uses
a native <input> directly instead of a <paper-input>.

BUG=602896
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[stevenjb, 2016-08-08 18:40:26]

Cheers! LGTM

[michaelpg, 2016-08-09 02:35:57]

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

[michaelpg, 2016-08-09 02:35:58]

michaelpg@chromium.org changed required reviewers:
+ stevenjb@chromium.org

[michaelpg, 2016-08-09 02:35:58]

PTAL at the patch diff: I had to make a couple changes to settings-dropdown-menu
to get the focus underline working right with the next paper-dropdown-menu-light
patch.

The label="[[menuLabel_]]" bit was making everything difficult and triggering a
bug I can't repro externally. It also *doesn't work* even on ToT; the goal is to
set the label to "Loading..." but it just disables the control until the options
populate. So I removed that to simplify.

Also removed the no-label-float option as *everything* uses no-label-float.

(Ignore the changes to settings_shared_css.html from rebasing.)

[stevenjb, 2016-08-09 17:16:03]

dschuyler@ added menuLabel_ to settings-dropdown-menu, so he should take a look
at that change, but this lgtm.

[dschuyler, 2016-08-09 17:25:28]

Is there a current way to set the "loading..." value with this
change? It's unfortunate that got broken, since we need it.

https://codereview.chromium.org/2224613003/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html
(right):

https://codereview.chromium.org/2224613003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html:118:
</span>
Is this span useful at this point?

[michaelpg, 2016-08-10 19:58:58]

Note the change in settings_shared_css.html to fix text alignment. I can't repro
this in a fiddle, seems related to font size, so IDK whether it should be a
change in Polymer.

On 2016/08/09 17:25:28, dschuyler wrote:
> Is there a current way to set the "loading..." value with this
> change? It's unfortunate that got broken, since we need it.

No, not with this change. I didn't feel like debugging
paper-dropdown-menu-light, but I can try again if you want me to leave the
broken behavior in.

> 
>
https://codereview.chromium.org/2224613003/diff/20001/chrome/browser/resource...
> File
>
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html
> (right):
> 
>
https://codereview.chromium.org/2224613003/diff/20001/chrome/browser/resource...
>
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html:118:
> </span>
> Is this span useful at this point?

https://codereview.chromium.org/2224613003/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html
(right):

https://codereview.chromium.org/2224613003/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html:118:
</span>
On 2016/08/09 17:25:28, dschuyler wrote:
> Is this span useful at this point?

Apparently not, removed.

[dschuyler, 2016-08-10 21:11:46]

On 2016/08/10 19:58:58, michaelpg wrote:
> Note the change in settings_shared_css.html to fix text alignment. I can't
repro
> this in a fiddle, seems related to font size, so IDK whether it should be a
> change in Polymer.
> 
> On 2016/08/09 17:25:28, dschuyler wrote:
> > Is there a current way to set the "loading..." value with this
> > change? It's unfortunate that got broken, since we need it.
> 
> No, not with this change. I didn't feel like debugging
> paper-dropdown-menu-light, but I can try again if you want me to leave the
> broken behavior in.

Yeah, it looks like the no-label-float is making it not appear.
I'd rather that the code was fixed than removed. If you're
not up for fixing it, then leaving the code for me to fix is 
better than removing it.

Maybe a TODO would be appropriate?
// TODO(michaelpg): This doesn't currently work (it's blocked by the
no-label-float).
// dschuyler@ is planning on looking into it later. crbug.com/636540.

(I just created that bug for it).

[michaelpg, 2016-08-10 23:50:20]

Patchset #5 (id:80001) has been deleted

[michaelpg, 2016-08-11 00:13:17]

On 2016/08/10 21:11:46, dschuyler wrote:
> On 2016/08/10 19:58:58, michaelpg wrote:
> > Note the change in settings_shared_css.html to fix text alignment. I can't
> repro
> > this in a fiddle, seems related to font size, so IDK whether it should be a
> > change in Polymer.
> > 
> > On 2016/08/09 17:25:28, dschuyler wrote:
> > > Is there a current way to set the "loading..." value with this
> > > change? It's unfortunate that got broken, since we need it.
> > 
> > No, not with this change. I didn't feel like debugging
> > paper-dropdown-menu-light, but I can try again if you want me to leave the
> > broken behavior in.
> 
> Yeah, it looks like the no-label-float is making it not appear.
> I'd rather that the code was fixed than removed. If you're
> not up for fixing it, then leaving the code for me to fix is 
> better than removing it.
> 
> Maybe a TODO would be appropriate?
> // TODO(michaelpg): This doesn't currently work (it's blocked by the
> no-label-float).
> // dschuyler@ is planning on looking into it later. crbug.com/636540.
> 
> (I just created that bug for it).

While debugging the focus underline issue I found a simple fix to 636540.
Rebased this on that so there's no TODO.

[dschuyler, 2016-08-11 23:35:26]

LGTM with a couple comment comments.

https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/settings/controls/settings_dropdown_menu.js
(right):

https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/controls/settings_dropdown_menu.js:99: // Do
not set |menuLabel_| to a falsy value: http://goo.gl/OnKYko.
Please also mention paper-dropdown-menu issue #181 
in case the link goes stale someday.

https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_shared_css.html:42: /** Offsets the
input's border-bottom. */
Should this be a /* comment rather than a /** comment?

[michaelpg, 2016-08-16 23:50:43]

thanks!

https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/settings/controls/settings_dropdown_menu.js
(right):

https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/controls/settings_dropdown_menu.js:99: // Do
not set |menuLabel_| to a falsy value: http://goo.gl/OnKYko.
On 2016/08/11 23:35:26, dschuyler wrote:
> Please also mention paper-dropdown-menu issue #181 
> in case the link goes stale someday.

Done.

Although....

nevermind.

https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_shared_css.html:42: /** Offsets the
input's border-bottom. */
On 2016/08/11 23:35:26, dschuyler wrote:
> Should this be a /* comment rather than a /** comment?

Ugh, I do that all the time, too used to closure. Although, I did some grepping,
apparently we haven't made this mistake in CSS virtually anywhere else. So it's
just me.

[dschuyler, 2016-08-18 00:01:18]

On 2016/08/16 23:50:43, michaelpg wrote:
> thanks!
> 
>
https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
> File chrome/browser/resources/settings/controls/settings_dropdown_menu.js
> (right):
> 
>
https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
> chrome/browser/resources/settings/controls/settings_dropdown_menu.js:99: // Do
> not set |menuLabel_| to a falsy value: http://goo.gl/OnKYko.
> On 2016/08/11 23:35:26, dschuyler wrote:
> > Please also mention paper-dropdown-menu issue #181 
> > in case the link goes stale someday.
> 
> Done.
> 
> Although....
> 
> nevermind.
> 
>
https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
> File chrome/browser/resources/settings/settings_shared_css.html (right):
> 
>
https://codereview.chromium.org/2224613003/diff/100001/chrome/browser/resourc...
> chrome/browser/resources/settings/settings_shared_css.html:42: /** Offsets the
> input's border-bottom. */
> On 2016/08/11 23:35:26, dschuyler wrote:
> > Should this be a /* comment rather than a /** comment?
> 
> Ugh, I do that all the time, too used to closure. Although, I did some
grepping,
> apparently we haven't made this mistake in CSS virtually anywhere else. So
it's
> just me.

SLGTM

[michaelpg, 2016-08-25 02:16:41]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-08-25 02:16:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, dschuyler@chromium.org
Link to the patchset: https://codereview.chromium.org/2224613003/#ps140001
(title: "rebase")

[commit-bot: I haz the power, 2016-08-25 02:17:25]

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-08-25 04:50:23]

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

[commit-bot: I haz the power, 2016-08-25 04:50:24]

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

[michaelpg, 2016-08-26 21:41:36]

The CQ bit was checked by michaelpg@chromium.org

[commit-bot: I haz the power, 2016-08-26 21:42:26]

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-08-27 00:01:11]

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

[commit-bot: I haz the power, 2016-08-27 00:01:12]

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_...)

[michaelpg, 2016-08-27 02:11:02]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-08-27 02:11:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
dschuyler@chromium.org, stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2224613003/#ps160001
(title: "rebase?")

[commit-bot: I haz the power, 2016-08-27 02:11:28]

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-08-27 11:13:43]

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

[commit-bot: I haz the power, 2016-08-27 11:13:44]

Failed to apply patch for
chrome/browser/resources/settings/settings_shared_css.html:
While running git apply --index -3 -p1;
  error: patch failed:
chrome/browser/resources/settings/settings_shared_css.html:34
  Falling back to three-way merge...
  Applied patch to 'chrome/browser/resources/settings/settings_shared_css.html'
with conflicts.
  U chrome/browser/resources/settings/settings_shared_css.html

Patch:       chrome/browser/resources/settings/settings_shared_css.html
Index: chrome/browser/resources/settings/settings_shared_css.html
diff --git a/chrome/browser/resources/settings/settings_shared_css.html
b/chrome/browser/resources/settings/settings_shared_css.html
index
c2fdfb4bfa1f1e8e38b7b6bea4dd8aa51142d7f9..93262d9be1cd3f8e5c9f5b5a94ee965980ba37e5
100644
--- a/chrome/browser/resources/settings/settings_shared_css.html
+++ b/chrome/browser/resources/settings/settings_shared_css.html
@@ -34,24 +34,26 @@
         @apply(--settings-actionable);
       }
 
-      paper-dropdown-menu {
+      paper-dropdown-menu-light {
         --iron-icon-fill-color: var(--paper-grey-600);
         --paper-font-subhead: {
           font-size: inherit;
         };
-        --paper-input-container-underline: {
-          background: var(--paper-grey-300);
+        --paper-dropdown-menu-input: {
+          border-bottom-color: var(--paper-grey-300);
         };
+        /* Offsets the input's border-bottom. */
+        top: 1px;
       }
 
       iron-dropdown .dropdown-content,
-      paper-dropdown-menu paper-listbox {
+      paper-dropdown-menu-light paper-listbox {
         min-width: 128px;
         padding: 8px 0;
       }
 
       iron-dropdown .dropdown-item,
-      paper-dropdown-menu .dropdown-item {
+      paper-dropdown-menu-light .dropdown-item {
         align-items: center;
         background: none;
         border: none;
@@ -64,12 +66,12 @@
         width: 100%;
       }
 
-      paper-dropdown-menu .dropdown-item.iron-selected {
+      paper-dropdown-menu-light .dropdown-item.iron-selected {
         font-weight: bold;
       }
 
       iron-dropdown .dropdown-item:focus,
-      paper-dropdown-menu .dropdown-item:focus {
+      paper-dropdown-menu-light .dropdown-item:focus {
         @apply(--cr-selectable-focus);
       }

[michaelpg, 2016-08-29 22:14:19]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-08-29 22:14:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
dschuyler@chromium.org, stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2224613003/#ps180001
(title: "rebase")

[commit-bot: I haz the power, 2016-08-29 22:45:17]

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-08-30 03:33:46]

Committed patchset #9 (id:180001)

[commit-bot: I haz the power, 2016-08-30 03:35:00]

Description was changed from

==========
Switch MD Settings to paper-dropdown-menu-light

paper-dropdown-menu-light is similar to paper-dropdown-menu but uses
a native <input> directly instead of a <paper-input>.

BUG=602896
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Switch MD Settings to paper-dropdown-menu-light

paper-dropdown-menu-light is similar to paper-dropdown-menu but uses
a native <input> directly instead of a <paper-input>.

BUG=602896
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/7a572e07f74c26470f9651cb39161fbcab67d0ec
Cr-Commit-Position: refs/heads/master@{#415049}
==========

[commit-bot: I haz the power, 2016-08-30 03:35:02]

Patchset 9 (id:??) landed as
https://crrev.com/7a572e07f74c26470f9651cb39161fbcab67d0ec
Cr-Commit-Position: refs/heads/master@{#415049}