Use md-select instead of paper-dropdown-menu-light in display.html

Use md-select instead of paper-dropdown-menu-light in display.html

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

Committed: https://crrev.com/5f65e96ed2c7b20476644e157fb8b6b97f86888f
Cr-Commit-Position: refs/heads/master@{#426544}

[stevenjb, 2016-10-17 21:29:16]

Description was changed from

==========
Use md-select instead of paper-dropdown-menu-light in display.html

BUG=653689
==========

to

==========
Use md-select instead of paper-dropdown-menu-light in display.html

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

[stevenjb, 2016-10-17 22:36:14]

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

[dpapad, 2016-10-18 00:14:46]

LGTM with nits.

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/display.html (right):

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/display.html:80:
on-change="onSetOrientation_">
Nit(optional): Consider renaming this to onOrientationChange_ for consistency
with other listener names.

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/display.js (right):

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/display.js:162:
this.$.orientation.value = this.selectedDisplay.rotation;
Nit: Since your drodpown is not using dom-repeat, you can use 1-way data binding
without any worries here (all <option> instances are rendered in when ready() is
called). So no need to programmatically set the value.

<select .... value="[[this.selectedDisplay.rotation]]">

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/display.js:284: * @param
{!{target: !{value: string}}} e
@param {!Event}

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/display.js:289: rotation:
parseInt(e.target.value, 10)
Cast to HTMLSelectElement to satisfy the compiler.

[stevenjb, 2016-10-18 00:42:38]

Patchset #3 (id:40001) has been deleted

[stevenjb, 2016-10-18 00:43:11]

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/display.html (right):

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/display.html:80:
on-change="onSetOrientation_">
On 2016/10/18 00:14:46, dpapad wrote:
> Nit(optional): Consider renaming this to onOrientationChange_ for consistency
> with other listener names.

Done.

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/device_page/display.js (right):

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/display.js:162:
this.$.orientation.value = this.selectedDisplay.rotation;
On 2016/10/18 00:14:46, dpapad wrote:
> Nit: Since your drodpown is not using dom-repeat, you can use 1-way data
binding
> without any worries here (all <option> instances are rendered in when ready()
is
> called). So no need to programmatically set the value.
> 
> <select .... value="[[this.selectedDisplay.rotation]]">

Yup. I just finished making this consistent with the internet changes.

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/display.js:284: * @param
{!{target: !{value: string}}} e
On 2016/10/18 00:14:46, dpapad wrote:
> @param {!Event}

Acknowledged.

https://codereview.chromium.org/2427833002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/device_page/display.js:289: rotation:
parseInt(e.target.value, 10)
On 2016/10/18 00:14:46, dpapad wrote:
> Cast to HTMLSelectElement to satisfy the compiler.

Acknowledged.

[stevenjb, 2016-10-18 00:43:29]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2016-10-18 00:43:29]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2427833002/#ps20002
(title: "Use data binding")

[commit-bot: I haz the power, 2016-10-18 00:44:06]

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-10-18 01:58:05]

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

[commit-bot: I haz the power, 2016-10-18 01:58:06]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[stevenjb, 2016-10-19 23:57:03]

The CQ bit was checked by stevenjb@chromium.org

[stevenjb, 2016-10-19 23:57:04]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2427833002/#ps70001
(title: "Fix test")

[commit-bot: I haz the power, 2016-10-19 23:57:32]

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-10-20 02:03:27]

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

[commit-bot: I haz the power, 2016-10-20 02:03:28]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[stevenjb, 2016-10-20 17:07:14]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2016-10-20 17:08:15]

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-10-20 18:50:09]

Committed patchset #4 (id:70001)

[commit-bot: I haz the power, 2016-10-21 13:20:42]

Description was changed from

==========
Use md-select instead of paper-dropdown-menu-light in display.html

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

to

==========
Use md-select instead of paper-dropdown-menu-light in display.html

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

Committed: https://crrev.com/5f65e96ed2c7b20476644e157fb8b6b97f86888f
Cr-Commit-Position: refs/heads/master@{#426544}
==========

[commit-bot: I haz the power, 2016-10-21 13:20:43]

Patchset 4 (id:??) landed as
https://crrev.com/5f65e96ed2c7b20476644e157fb8b6b97f86888f
Cr-Commit-Position: refs/heads/master@{#426544}