MD Settings: omnibox extensions list to use focus-row-behavior.

MD Settings: omnibox extensions list to use focus-row-behavior.

Makes it behave the same as the other iron-lists on the page.

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

Review-Url: https://codereview.chromium.org/2852413003
Cr-Commit-Position: refs/heads/master@{#469533}
Committed: https://chromium.googlesource.com/chromium/src/+/e21cf8f0b7ad21d58dcd2c6d06b35ce722e7e94f

[scottchen, 2017-05-02 21:33:59]

Description was changed from

==========
MD Settings: omnibox extensions list to use focus-row-behavior.

Makes it behave the same as the other iron-lists on the page.

BUG=None
==========

to

==========
MD Settings: omnibox extensions list to use focus-row-behavior.

Makes it behave the same as the other iron-lists on the page.

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

[scottchen, 2017-05-02 21:34:18]

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

[commit-bot: I haz the power, 2017-05-02 21:34:36]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[scottchen, 2017-05-02 21:34:43]

scottchen@chromium.org changed reviewers:
+ hcarmona@chromium.org

[scottchen, 2017-05-02 21:45:22]

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

[commit-bot: I haz the power, 2017-05-02 22:38:25]

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

[commit-bot: I haz the power, 2017-05-02 22:38:26]

Dry run: This issue passed the CQ dry run.

[dpapad, 2017-05-04 16:35:33]

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/omnibox_extension_entry.html
(right):

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/omnibox_extension_entry.html:38:
focus-type="cr-menu-button">
What is focus-type="cr-menu-button" ? I could not find any docs for focus-type
or any other references for cr-menu-button (besides the one in
search_engine_entry.html).

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

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_page.js:51:
/** @private {Object}*/
What is the type of this member variable? Can we do something better than just
Object (maybe Element or HTMLElement)?

Also, space missing after "}"

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_page.js:52:
OmniboxExtensionlastFocused_: Object,
s/Omnibox/omnibox

[hcarmona, 2017-05-04 21:03:35]

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engine_entry.html
(right):

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engine_entry.html:59:
focus-type="cr-menu-button">
👍

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engines_page.html
(right):

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_page.html:55:
tabindex$="[[tabIndex]]" iron-list-tab-index="[[tabIndex]]"
do you need both tabindex and iron-list-tab-index?

[scottchen, 2017-05-04 21:57:38]

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/omnibox_extension_entry.html
(right):

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/omnibox_extension_entry.html:38:
focus-type="cr-menu-button">
On 2017/05/04 16:35:32, dpapad wrote:
> What is focus-type="cr-menu-button" ? I could not find any docs for focus-type
> or any other references for cr-menu-button (besides the one in
> search_engine_entry.html).

https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/focus_...

it doesn't really matter what the string is, as long as the same type of control
across rows have the same string value for focus-type.

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engine_entry.html
(right):

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engine_entry.html:59:
focus-type="cr-menu-button">
On 2017/05/04 21:03:35, hcarmona wrote:
> 👍

Done.

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engines_page.html
(right):

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_page.html:55:
tabindex$="[[tabIndex]]" iron-list-tab-index="[[tabIndex]]"
On 2017/05/04 21:03:35, hcarmona wrote:
> do you need both tabindex and iron-list-tab-index?

yeah, the js code uses them to do slightly different things.

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

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_page.js:51:
/** @private {Object}*/
On 2017/05/04 16:35:32, dpapad wrote:
> What is the type of this member variable? Can we do something better than just
> Object (maybe Element or HTMLElement)?
> 
> Also, space missing after "}"

Done.

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_page.js:52:
OmniboxExtensionlastFocused_: Object,
On 2017/05/04 16:35:32, dpapad wrote:
> s/Omnibox/omnibox

Done.

[dpapad, 2017-05-04 22:35:22]

LGTM

[hcarmona, 2017-05-04 22:51:35]

LGTM

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engines_page.html
(right):

https://codereview.chromium.org/2852413003/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_page.html:55:
tabindex$="[[tabIndex]]" iron-list-tab-index="[[tabIndex]]"
On 2017/05/04 21:57:38, scottchen wrote:
> On 2017/05/04 21:03:35, hcarmona wrote:
> > do you need both tabindex and iron-list-tab-index?
> 
> yeah, the js code uses them to do slightly different things.

SG

[scottchen, 2017-05-04 23:04:37]

The CQ bit was checked by scottchen@chromium.org

[commit-bot: I haz the power, 2017-05-04 23:05:14]

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-05-05 00:08:36]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493939077759250,
"parent_rev": "b76b83389ebeb5b4e7a84c7bf7fbcab1bbf92a29", "commit_rev":
"e21cf8f0b7ad21d58dcd2c6d06b35ce722e7e94f"}

[commit-bot: I haz the power, 2017-05-05 00:09:35]

Description was changed from

==========
MD Settings: omnibox extensions list to use focus-row-behavior.

Makes it behave the same as the other iron-lists on the page.

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

to

==========
MD Settings: omnibox extensions list to use focus-row-behavior.

Makes it behave the same as the other iron-lists on the page.

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

Review-Url: https://codereview.chromium.org/2852413003
Cr-Commit-Position: refs/heads/master@{#469533}
Committed:
https://chromium.googlesource.com/chromium/src/+/e21cf8f0b7ad21d58dcd2c6d06b3...
==========

[commit-bot: I haz the power, 2017-05-05 00:09:37]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/e21cf8f0b7ad21d58dcd2c6d06b3...