Language settings: fix focus issues

Language settings: fix focus issues

* Allow traversal through iron-list elements with up/down arrow keys.
  For input methods, <Enter> selects that input method.
* Tapping anywhere on a togglable row toggles that row.
* Make the buttons to open a subpage keyboard-focusable by wrapping their
  content in <a is="action-link">.
* Don't focus the Add Language dialog's Cancel button on opening.
* Other minor UI fixes.

BUG=664106, 668307, 658279
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/10719a79d0d6cf3cee559771cb5cca8f78024b5d
Cr-Commit-Position: refs/heads/master@{#436894}

[michaelpg, 2016-11-24 02:38:33]

Description was changed from

==========
Language settings: fix focus issues

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

to

==========
Language settings: fix focus issues

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

[michaelpg, 2016-11-24 02:41:46]

Description was changed from

==========
Language settings: fix focus issues

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

to

==========
Language settings: fix focus issues

* Allow traversal through iron-list elements with up/down arrow keys.
  For input methods, <Enter> selects that input method.
* Tapping anywhere on a togglable row toggles that row.
* Make the buttons to open a subpage keyboard-focusably by wrapping their
  content in <a is="action-link">.

BUG=664106,668307,658279
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-11-24 02:41:46]

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

[michaelpg, 2016-11-24 02:42:10]

PTAL. Can wait til next week :-)

[stevenjb, 2016-11-28 17:49:16]

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/languages_page/add_languages_dialog.js
(right):

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/add_languages_dialog.js:39:
this.$$('.action-button').focus();
This seems klugey. Can we just blur the cancel button instead?

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/add_languages_dialog.js:48: //
Only fire iron-list after the list displayed to prevent flickering.
'fire iron-resize' ?

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/languages_page/languages_page.js (right):

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/languages_page.js:521: var
expandButton = e.currentTarget.querySelector(expandButtonTag);
nit: 'var expandButton =' on prev line?

[michaelpg, 2016-12-01 23:19:13]

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/languages_page/add_languages_dialog.js
(right):

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/add_languages_dialog.js:39:
this.$$('.action-button').focus();
On 2016/11/28 17:49:16, stevenjb wrote:
> This seems klugey. Can we just blur the cancel button instead?

Done. Thought I had tried that!

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/add_languages_dialog.js:48: //
Only fire iron-list after the list displayed to prevent flickering.
On 2016/11/28 17:49:16, stevenjb wrote:
> 'fire iron-resize' ?

Done.

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/languages_page/languages_page.js (right):

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/languages_page.js:521: var
expandButton = e.currentTarget.querySelector(expandButtonTag);
On 2016/11/28 17:49:16, stevenjb wrote:
> nit: 'var expandButton =' on prev line?

meh... i find that less readable. @type above var is more consistent with other
typing (prototype property types and normal typedefs), whereas inline @type is
normally used as a cast.

also, it doesn't seem to be very common at least in settings... by which i mean,
it's only in code that you write :-P unless my search is wrong:
https://cs.chromium.org/search/?q=file:resources%5C/settings+%5E%5Cs*%5C/%5C*...

https://codereview.chromium.org/2523403003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/languages_page.html
(right):

https://codereview.chromium.org/2523403003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.html:178:
class$="settings-box [[getSpellcheckListTwoLine_(
Note this additional change... originally I didn't want the box to change in
height if you check/uncheck spellcheck, but that's minor. It looks ugly when
it's two-line but only has one line of text, so the conditional seems worth it.

[stevenjb, 2016-12-01 23:45:34]

lgtm w/ one suggestion

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/languages_page/languages_page.js (right):

https://codereview.chromium.org/2523403003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/languages_page/languages_page.js:521: var
expandButton = e.currentTarget.querySelector(expandButtonTag);
On 2016/12/01 23:19:13, michaelpg wrote:
> On 2016/11/28 17:49:16, stevenjb wrote:
> > nit: 'var expandButton =' on prev line?
> 
> meh... i find that less readable. @type above var is more consistent with
other
> typing (prototype property types and normal typedefs), whereas inline @type is
> normally used as a cast.
> 
> also, it doesn't seem to be very common at least in settings... by which i
mean,
> it's only in code that you write :-P unless my search is wrong:
>
https://cs.chromium.org/search/?q=file:resources%5C/settings+%5E%5Cs*%5C/%5C*...

*shrug*

https://codereview.chromium.org/2523403003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/languages_page.js (right):

https://codereview.chromium.org/2523403003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.js:403: });
This looks duplicated at the top of getSpellCheckSecondaryText_, which makes
sense given the html. I think this would be more clear if they both used a
helper method.

[michaelpg, 2016-12-02 03:15:09]

Description was changed from

==========
Language settings: fix focus issues

* Allow traversal through iron-list elements with up/down arrow keys.
  For input methods, <Enter> selects that input method.
* Tapping anywhere on a togglable row toggles that row.
* Make the buttons to open a subpage keyboard-focusably by wrapping their
  content in <a is="action-link">.

BUG=664106,668307,658279
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Language settings: fix focus issues

* Allow traversal through iron-list elements with up/down arrow keys.
  For input methods, <Enter> selects that input method.
* Tapping anywhere on a togglable row toggles that row.
* Make the buttons to open a subpage keyboard-focusable by wrapping their
  content in <a is="action-link">.
* Don't focus the Add Language dialog's Cancel button on opening.
* Other minor UI fixes.

BUG=664106,668307,658279
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[michaelpg, 2016-12-02 03:28:00]

Patchset #6 (id:100001) has been deleted

[michaelpg, 2016-12-02 03:31:05]

https://codereview.chromium.org/2523403003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/languages_page/languages_page.js (right):

https://codereview.chromium.org/2523403003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/languages_page/languages_page.js:403: });
On 2016/12/01 23:45:33, stevenjb wrote:
> This looks duplicated at the top of getSpellCheckSecondaryText_, which makes
> sense given the html. I think this would be more clear if they both used a
> helper method.

I've moved this out to a follow-up CL to keep this one somewhat focused on
focus.

[stevenjb, 2016-12-05 16:48:31]

still lgtm

[michaelpg, 2016-12-06 23:25:25]

The CQ bit was checked by michaelpg@chromium.org

[michaelpg, 2016-12-06 23:25:25]

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

[commit-bot: I haz the power, 2016-12-06 23:26: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-12-06 23:34:54]

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

[commit-bot: I haz the power, 2016-12-06 23:34:55]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[michaelpg, 2016-12-07 09:01:05]

The CQ bit was checked by michaelpg@chromium.org

[commit-bot: I haz the power, 2016-12-07 09:01: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-12-07 09:05:24]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1481101265966380,
"parent_rev": "be0ec8edbbb90769dd00b13eadf91c06329fd4ec", "commit_rev":
"02b18ee4f73d880e9434a573bdc2b0b502aeedfa"}

[commit-bot: I haz the power, 2016-12-07 09:05:49]

Description was changed from

==========
Language settings: fix focus issues

* Allow traversal through iron-list elements with up/down arrow keys.
  For input methods, <Enter> selects that input method.
* Tapping anywhere on a togglable row toggles that row.
* Make the buttons to open a subpage keyboard-focusable by wrapping their
  content in <a is="action-link">.
* Don't focus the Add Language dialog's Cancel button on opening.
* Other minor UI fixes.

BUG=664106,668307,658279
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Language settings: fix focus issues

* Allow traversal through iron-list elements with up/down arrow keys.
  For input methods, <Enter> selects that input method.
* Tapping anywhere on a togglable row toggles that row.
* Make the buttons to open a subpage keyboard-focusable by wrapping their
  content in <a is="action-link">.
* Don't focus the Add Language dialog's Cancel button on opening.
* Other minor UI fixes.

BUG=664106,668307,658279
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-12-07 09:05:50]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2016-12-07 09:07:50]

Description was changed from

==========
Language settings: fix focus issues

* Allow traversal through iron-list elements with up/down arrow keys.
  For input methods, <Enter> selects that input method.
* Tapping anywhere on a togglable row toggles that row.
* Make the buttons to open a subpage keyboard-focusable by wrapping their
  content in <a is="action-link">.
* Don't focus the Add Language dialog's Cancel button on opening.
* Other minor UI fixes.

BUG=664106,668307,658279
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Language settings: fix focus issues

* Allow traversal through iron-list elements with up/down arrow keys.
  For input methods, <Enter> selects that input method.
* Tapping anywhere on a togglable row toggles that row.
* Make the buttons to open a subpage keyboard-focusable by wrapping their
  content in <a is="action-link">.
* Don't focus the Add Language dialog's Cancel button on opening.
* Other minor UI fixes.

BUG=664106,668307,658279
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/10719a79d0d6cf3cee559771cb5cca8f78024b5d
Cr-Commit-Position: refs/heads/master@{#436894}
==========

[commit-bot: I haz the power, 2016-12-07 09:07:51]

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