Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2620843004:
Radio input keyboard navigation scroll (Closed)
|
Created:
3 years, 11 months ago by preeti.nayak Modified:
3 years, 11 months ago CC:
blink-reviews, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionThis patch scrolls the parent container while navigating through the radio input using keyboard
BUG=674567
Review-Url: https://codereview.chromium.org/2620843004
Cr-Commit-Position: refs/heads/master@{#443999}
Committed: https://chromium.googlesource.com/chromium/src/+/f817f12c1ee224c86e0baf45c816d89dfb345aa3
Patch Set 1 #Patch Set 2 : This patch scrolls the parent container while navigating through the radio input using keyboard #
Total comments: 8
Patch Set 3 : This patch scrolls the parent container while navigating through the radio input using keyboard #
Messages
Total messages: 19 (11 generated)
Description was changed from ========== Not for review BUG= ========== to ========== BUG=674567 ==========
preeti.nayak@samsung.com changed reviewers: + sanjoy.pal@samsung.com
The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Description was changed from ========== BUG=674567 ========== to ========== This patch scrolls the parent container while navigating through the radio input using keyboard BUG=674567 ==========
preeti.nayak@samsung.com changed reviewers: + tkent@chromium.org
Please add a test
On 2017/01/13 07:13:33, tkent wrote: > Please add a test Done. PTAL.
https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html (right): https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:3: <head> Please do not indent HTML tags. It has almost no benefit. Write <html> <head> <script ...> <script ...> instead of <html> <head> <script ..> <script ..> https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:11: height:65px; Add a space after ':' for consistency. https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:13: input[type="radio"]{ Add a space after ']' for consistency. https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:21: <input id="radio_2" type="radio" name="foo"> id="radio_2" to id="radio_6" are unnecessary. https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:29: document.getElementById("radio_1").focus(); nit: You can remove id="radio_1" by document.querySelector("input").focus(); https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:30: if(window.eventSender) { assert_exists(window, "eventSender"); https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:31: for(var i = 0; i < 5; i++){ Add a space after "for". Add a space after ')'. https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/platform/linux/fast/forms/radio/radio-input-keyboard-navigation-expected.txt (right): https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/platform/linux/fast/forms/radio/radio-input-keyboard-navigation-expected.txt:1: This is a testharness.js-based test. This file should be removed.
On 2017/01/17 00:11:53, tkent wrote: > https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... > File > third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html > (right): > > https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:3: > <head> > Please do not indent HTML tags. It has almost no benefit. > Write > > <html> > <head> > <script ...> > <script ...> > > instead of > > <html> > <head> > <script ..> > <script ..> > > https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:11: > height:65px; > Add a space after ':' for consistency. > > https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:13: > input[type="radio"]{ > Add a space after ']' for consistency. > > https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:21: > <input id="radio_2" type="radio" name="foo"> > id="radio_2" to id="radio_6" are unnecessary. > > https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:29: > document.getElementById("radio_1").focus(); > nit: You can remove id="radio_1" by document.querySelector("input").focus(); > > https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:30: > if(window.eventSender) { > assert_exists(window, "eventSender"); > > https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/fast/forms/radio/radio-input-keyboard-navigation.html:31: > for(var i = 0; i < 5; i++){ > Add a space after "for". > Add a space after ')'. > > https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... > File > third_party/WebKit/LayoutTests/platform/linux/fast/forms/radio/radio-input-keyboard-navigation-expected.txt > (right): > > https://codereview.chromium.org/2620843004/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/platform/linux/fast/forms/radio/radio-input-keyboard-navigation-expected.txt:1: > This is a testharness.js-based test. > This file should be removed. @tkent Thanks for your inputs. I have addressed them in Patch Set 3. PTAL.
The CQ bit was checked by tkent@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484632893974280,
"parent_rev": "cf39aa8ec1f314d198a3caa12922c254e49d1e7c", "commit_rev":
"f817f12c1ee224c86e0baf45c816d89dfb345aa3"}
Message was sent while issue was closed.
Description was changed from ========== This patch scrolls the parent container while navigating through the radio input using keyboard BUG=674567 ========== to ========== This patch scrolls the parent container while navigating through the radio input using keyboard BUG=674567 Review-Url: https://codereview.chromium.org/2620843004 Cr-Commit-Position: refs/heads/master@{#443999} Committed: https://chromium.googlesource.com/chromium/src/+/f817f12c1ee224c86e0baf45c816... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/f817f12c1ee224c86e0baf45c816... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
