Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(893)

Issue 2795353002: WebUI: For cr-dialog, handle Enter keys in paper-inputs. (Closed)

Created:
8 months, 2 weeks ago by tommycli
Modified:
8 months, 2 weeks ago
Reviewers:
dpapad
CC:
arv+watch_chromium.org, chromium-reviews, dbeam+watch-elements_chromium.org, dbeam+watch-settings_chromium.org, gcasto+watchlist_chromium.org, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, michaelpg+watch-elements_chromium.org, oshima+watch_chromium.org, stevenjb+watch-md-settings_chromium.org, vabr+watchlistpasswordmanager_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

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

Patch Set 1 #

Patch Set 2 : remove unnecessary ids #

Patch Set 3 : remove a11y key handlers #

Patch Set 4 : restore some ids that are used in tests #

Total comments: 4

Patch Set 5 : fix test #

Total comments: 4

Patch Set 6 : update tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+66 lines, -111 lines) Patch
M chrome/browser/resources/settings/certificate_manager_page/certificate_password_decryption_dialog.html View 2 3 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/resources/settings/certificate_manager_page/certificate_password_decryption_dialog.js View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/resources/settings/certificate_manager_page/certificate_password_encryption_dialog.html View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/settings/certificate_manager_page/certificate_password_encryption_dialog.js View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/resources/settings/internet_page/compiled_resources2.gyp View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/resources/settings/internet_page/network_summary_item.html View 1 2 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/resources/settings/internet_page/network_summary_item.js View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/settings/on_startup_page/startup_url_dialog.html View 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/resources/settings/on_startup_page/startup_url_dialog.js View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/resources/settings/passwords_and_forms_page/address_edit_dialog.html View 1 2 4 chunks +2 lines, -8 lines 0 comments Download
M chrome/browser/resources/settings/passwords_and_forms_page/credit_card_edit_dialog.html View 1 2 2 chunks +8 lines, -13 lines 0 comments Download
M chrome/browser/resources/settings/passwords_and_forms_page/password_edit_dialog.html View 1 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/resources/settings/passwords_and_forms_page/password_edit_dialog.js View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/resources/settings/people_page/password_prompt_dialog.html View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/resources/settings/people_page/password_prompt_dialog.js View 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/browser/resources/settings/people_page/users_add_user_dialog.html View 1 2 2 chunks +0 lines, -3 lines 0 comments Download
M chrome/browser/resources/settings/search_engines_page/search_engine_dialog.html View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/settings/search_engines_page/search_engine_dialog.js View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/browser/resources/settings/site_settings/add_site_dialog.html View 1 2 2 chunks +0 lines, -3 lines 0 comments Download
M chrome/browser/resources/settings/site_settings/edit_exception_dialog.html View 1 2 3 1 chunk +3 lines, -4 lines 0 comments Download
M chrome/browser/resources/settings/site_settings/edit_exception_dialog.js View 1 chunk +0 lines, -9 lines 0 comments Download
M chrome/test/data/webui/cr_elements/cr_dialog_test.js View 1 2 3 4 5 3 chunks +40 lines, -4 lines 0 comments Download
M ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js View 1 2 3 4 1 chunk +4 lines, -2 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 25 (18 generated)
tommycli
dpapad: PTAL, thanks!
8 months, 2 weeks ago (2017-04-04 21:52:33 UTC) #13
dpapad
https://codereview.chromium.org/2795353002/diff/60001/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js File ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js (right): https://codereview.chromium.org/2795353002/diff/60001/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js#newcode116 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_elements/cr_dialog/cr_dialog.js#newcode122 ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js:122: if (e.target ...
8 months, 2 weeks ago (2017-04-04 23:46:15 UTC) #14
tommycli
Thanks! https://codereview.chromium.org/2795353002/diff/60001/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js File ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js (right): https://codereview.chromium.org/2795353002/diff/60001/ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js#newcode116 ui/webui/resources/cr_elements/cr_dialog/cr_dialog.js:116: if (!e.target) On 2017/04/04 23:46:14, dpapad wrote: > ...
8 months, 2 weeks ago (2017-04-05 21:07:08 UTC) #16
dpapad
LGTM https://codereview.chromium.org/2795353002/diff/80001/chrome/test/data/webui/cr_elements/cr_dialog_test.js File chrome/test/data/webui/cr_elements/cr_dialog_test.js (right): https://codereview.chromium.org/2795353002/diff/80001/chrome/test/data/webui/cr_elements/cr_dialog_test.js#newcode116 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/cr_elements/cr_dialog_test.js#newcode120 chrome/test/data/webui/cr_elements/cr_dialog_test.js:120: ...
8 months, 2 weeks ago (2017-04-05 21:34:45 UTC) #18
tommycli
thanks for the review! https://codereview.chromium.org/2795353002/diff/80001/chrome/test/data/webui/cr_elements/cr_dialog_test.js File chrome/test/data/webui/cr_elements/cr_dialog_test.js (right): https://codereview.chromium.org/2795353002/diff/80001/chrome/test/data/webui/cr_elements/cr_dialog_test.js#newcode116 chrome/test/data/webui/cr_elements/cr_dialog_test.js:116: clickedCounter += 1; On 2017/04/05 ...
8 months, 2 weeks ago (2017-04-05 21:42:48 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2795353002/100001
8 months, 2 weeks ago (2017-04-05 21:44:24 UTC) #22
commit-bot: I haz the power
8 months, 2 weeks ago (2017-04-05 23:18:55 UTC) #25
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/d7a1c56f2dabcbf4d8c894eaab9d...

Powered by Google App Engine
This is Rietveld 0eb02b776