Convert a11y page to Polymer 0.8

Convert a11y page to Polymer 0.8

BUG=485381
Committed: https://crrev.com/459dcc68462818c9acfb069d658f8cfcf923e3f1
Cr-Commit-Position: refs/heads/master@{#329569}

[Jeremy Klein, 2015-05-12 02:07:58]

jlklein@chromium.org changed reviewers:
+ khorimoto@chromium.org

[michaelpg, 2015-05-12 05:52:05]

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

[michaelpg, 2015-05-12 05:52:05]

https://codereview.chromium.org/1137103002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/a11y_page/a11y_page.js (right):

https://codereview.chromium.org/1137103002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/a11y_page/a11y_page.js:68: * Name of the
'core-icon' to show.
iron-icon

https://codereview.chromium.org/1137103002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/a11y_page/a11y_page.js:78:
this.$.autoclickDropdown.value =
for this and the below, I would just bind to the <select> in the template until
paper-dropdown is available.

[Jeremy Klein, 2015-05-12 06:23:22]

https://codereview.chromium.org/1137103002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/a11y_page/a11y_page.js (right):

https://codereview.chromium.org/1137103002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/a11y_page/a11y_page.js:68: * Name of the
'core-icon' to show.
On 2015/05/12 05:52:05, michaelpg wrote:
> iron-icon

Done.

https://codereview.chromium.org/1137103002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/a11y_page/a11y_page.js:78:
this.$.autoclickDropdown.value =
On 2015/05/12 05:52:05, michaelpg wrote:
> for this and the below, I would just bind to the <select> in the template
until
> paper-dropdown is available.

But that won't actually do anything. I'd rather have it continue to be
functional and just swap this out later.

[michaelpg, 2015-05-12 07:04:33]

https://codereview.chromium.org/1137103002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/a11y_page/a11y_page.js (right):

https://codereview.chromium.org/1137103002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/a11y_page/a11y_page.js:78:
this.$.autoclickDropdown.value =
On 2015/05/12 06:23:22, Jeremy Klein wrote:
> On 2015/05/12 05:52:05, michaelpg wrote:
> > for this and the below, I would just bind to the <select> in the template
> until
> > paper-dropdown is available.
> 
> But that won't actually do anything. I'd rather have it continue to be
> functional and just swap this out later.

Oh, you're right, the changes from the <select> won't be picked up by Polymer so
they won't propagate to the pref.

I still think you don't need this ready function though; you can still get the
one-way binding from the pref to the select attribute. That would also make the
select update dynamically if the pref changes elsewhere.

[Jeremy Klein, 2015-05-12 07:11:46]

On 2015/05/12 07:04:33, michaelpg wrote:
>
https://codereview.chromium.org/1137103002/diff/20001/chrome/browser/resource...
> File chrome/browser/resources/settings/a11y_page/a11y_page.js (right):
> 
>
https://codereview.chromium.org/1137103002/diff/20001/chrome/browser/resource...
> chrome/browser/resources/settings/a11y_page/a11y_page.js:78:
> this.$.autoclickDropdown.value =
> On 2015/05/12 06:23:22, Jeremy Klein wrote:
> > On 2015/05/12 05:52:05, michaelpg wrote:
> > > for this and the below, I would just bind to the <select> in the template
> > until
> > > paper-dropdown is available.
> > 
> > But that won't actually do anything. I'd rather have it continue to be
> > functional and just swap this out later.
> 
> Oh, you're right, the changes from the <select> won't be picked up by Polymer
so
> they won't propagate to the pref.
> 
> I still think you don't need this ready function though; you can still get the
> one-way binding from the pref to the select attribute. That would also make
the
> select update dynamically if the pref changes elsewhere.

Fair enough. Done.

[michaelpg, 2015-05-12 08:03:42]

LGTM w/ one comment.

https://codereview.chromium.org/1137103002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/settings/a11y_page/a11y_page.js (right):

https://codereview.chromium.org/1137103002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/a11y_page/a11y_page.js:52: type: String,
for "constant" properties, set readonly: true (presumably this helps perf by
disabling unnecessary bindings)

[Jeremy Klein, 2015-05-12 17:28:57]

https://codereview.chromium.org/1137103002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/settings/a11y_page/a11y_page.js (right):

https://codereview.chromium.org/1137103002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/settings/a11y_page/a11y_page.js:52: type: String,
On 2015/05/12 08:03:42, michaelpg wrote:
> for "constant" properties, set readonly: true (presumably this helps perf by
> disabling unnecessary bindings)

Great point. I'll add that in all my CLs where appropriate.

[Kyle Horimoto, 2015-05-12 17:53:47]

lgtm

https://codereview.chromium.org/1137103002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/a11y_page/a11y_page.html (right):

https://codereview.chromium.org/1137103002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/a11y_page/a11y_page.html:7: <link rel="import"
type="css"
nit: Un-indent this.

https://codereview.chromium.org/1137103002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_page/settings_page.css (right):

https://codereview.chromium.org/1137103002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_page/settings_page.css:12: display:
flex;
nit: Wouldn't it be better to use layout classes on the paper-material element
instead of adding these?

[Jeremy Klein, 2015-05-12 18:35:20]

https://codereview.chromium.org/1137103002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/a11y_page/a11y_page.html (right):

https://codereview.chromium.org/1137103002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/a11y_page/a11y_page.html:7: <link rel="import"
type="css"
On 2015/05/12 17:53:45, Kyle Horimoto wrote:
> nit: Un-indent this.

Done.

https://codereview.chromium.org/1137103002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_page/settings_page.css (right):

https://codereview.chromium.org/1137103002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_page/settings_page.css:12: display:
flex;
On 2015/05/12 17:53:45, Kyle Horimoto wrote:
> nit: Wouldn't it be better to use layout classes on the paper-material element
> instead of adding these?

The issue is that if we do that we'll need to add the iron-flex-layout import in
every single page. This seems like a cleaner solution since it's only 2 lines of
css.

[Jeremy Klein, 2015-05-13 01:25:38]

The CQ bit was checked by jlklein@chromium.org

[Jeremy Klein, 2015-05-13 01:25:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelpg@chromium.org, khorimoto@chromium.org
Link to the patchset: https://codereview.chromium.org/1137103002/#ps200001
(title: "rebase")

[commit-bot: I haz the power, 2015-05-13 01:26:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1137103002/200001

[commit-bot: I haz the power, 2015-05-13 02:24:35]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2015-05-13 02:25:23]

Patchset 11 (id:??) landed as
https://crrev.com/459dcc68462818c9acfb069d658f8cfcf923e3f1
Cr-Commit-Position: refs/heads/master@{#329569}