WebUI: For cr-dialog, handle Enter keys in paper-inputs.

WebUI: For cr-dialog, handle Enter keys in paper-inputs.

This allows us to remove a ton of duplicate Enter key handlers.

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

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

[tommycli, 2017-04-04 18:36:23]

Description was changed from

==========
WebUI: For cr-dialog, handle Enter keys in paper-inputs.

This allows us to remove a ton of duplicate Enter key handlers.

BUG=626420
==========

to

==========
WebUI: For cr-dialog, handle Enter keys in paper-inputs.

This allows us to remove a ton of duplicate Enter key handlers.

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

[tommycli, 2017-04-04 18:36:29]

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

[commit-bot: I haz the power, 2017-04-04 18:37:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tommycli, 2017-04-04 18:41:43]

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

[commit-bot: I haz the power, 2017-04-04 18:42:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tommycli, 2017-04-04 19:00:51]

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

[commit-bot: I haz the power, 2017-04-04 19:01:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tommycli, 2017-04-04 19:19:56]

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

[commit-bot: I haz the power, 2017-04-04 19:20:35]

Dry run: 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-04-04 21:51:44]

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

[commit-bot: I haz the power, 2017-04-04 21:51:45]

Dry run: This issue passed the CQ dry run.

[tommycli, 2017-04-04 21:52:32]

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

[tommycli, 2017-04-04 21:52:33]

dpapad: PTAL, thanks!

[dpapad, 2017-04-04 23:46:15]

https://codereview.chromium.org/2795353002/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js (right):

https://codereview.chromium.org/2795353002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js:116: if (!e.target)
Is that possible?

https://codereview.chromium.org/2795353002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js:122: if (e.target != this
&& e.target.tagName != 'PAPER-INPUT')
Can we add a test for this? You can use a <paper-input> element in the
cr_dialog_tests.js without actually importing the paper-input.html. The browser
will happily create a non-upgraded paper-input element, which will have none of
the actual paper-input's functionality, but it will return 'PAPER-INPUT'  from
tagName.

[tommycli, 2017-04-05 21:07:01]

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

[tommycli, 2017-04-05 21:07:08]

Thanks!

https://codereview.chromium.org/2795353002/diff/60001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js (right):

https://codereview.chromium.org/2795353002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js:116: if (!e.target)
On 2017/04/04 23:46:14, dpapad wrote:
> Is that possible?

Done.

https://codereview.chromium.org/2795353002/diff/60001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js:122: if (e.target != this
&& e.target.tagName != 'PAPER-INPUT')
On 2017/04/04 23:46:14, dpapad wrote:
> Can we add a test for this? You can use a <paper-input> element in the
> cr_dialog_tests.js without actually importing the paper-input.html. The
browser
> will happily create a non-upgraded paper-input element, which will have none
of
> the actual paper-input's functionality, but it will return 'PAPER-INPUT'  from
> tagName.

Done.

[commit-bot: I haz the power, 2017-04-05 21:07:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dpapad, 2017-04-05 21:34:45]

LGTM

https://codereview.chromium.org/2795353002/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/cr_elements/cr_dialog_test.js (right):

https://codereview.chromium.org/2795353002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/cr_elements/cr_dialog_test.js:116: clickedCounter += 1;
Nit (optional): clickedCounter++;

https://codereview.chromium.org/2795353002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/cr_elements/cr_dialog_test.js:120: otherElement,
'keypress', 13, undefined, 'Enter');
Nit (optional): Can we make a helper

function pressEnter(element) {
  MockInteractions.keyEventOn(element, 'keypress', 13, undefined, 'Enter');
}

and re-use it throughout this file (I see 5 occurrences currently).

[tommycli, 2017-04-05 21:42:48]

thanks for the review!

https://codereview.chromium.org/2795353002/diff/80001/chrome/test/data/webui/...
File chrome/test/data/webui/cr_elements/cr_dialog_test.js (right):

https://codereview.chromium.org/2795353002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/cr_elements/cr_dialog_test.js:116: clickedCounter += 1;
On 2017/04/05 21:34:45, dpapad wrote:
> Nit (optional): clickedCounter++;

Done.

https://codereview.chromium.org/2795353002/diff/80001/chrome/test/data/webui/...
chrome/test/data/webui/cr_elements/cr_dialog_test.js:120: otherElement,
'keypress', 13, undefined, 'Enter');
On 2017/04/05 21:34:45, dpapad wrote:
> Nit (optional): Can we make a helper
> 
> function pressEnter(element) {
>   MockInteractions.keyEventOn(element, 'keypress', 13, undefined, 'Enter');
> }
> 
> and re-use it throughout this file (I see 5 occurrences currently).

Done.

[tommycli, 2017-04-05 21:43:43]

The CQ bit was checked by tommycli@chromium.org

[tommycli, 2017-04-05 21:43:43]

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/2795353002/#ps100001
(title: "update tests")

[commit-bot: I haz the power, 2017-04-05 21:44:24]

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-04-05 23:18:06]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1491428623099550,
"parent_rev": "c54f19fb954b6b55ea85f4c594e048ca0dd5b207", "commit_rev":
"d7a1c56f2dabcbf4d8c894eaab9da20535f2a621"}

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

Description was changed from

==========
WebUI: For cr-dialog, handle Enter keys in paper-inputs.

This allows us to remove a ton of duplicate Enter key handlers.

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

to

==========
WebUI: For cr-dialog, handle Enter keys in paper-inputs.

This allows us to remove a ton of duplicate Enter key handlers.

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

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

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

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/d7a1c56f2dabcbf4d8c894eaab9d...