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

Issue 2888723002: Make the table cells clickable, instead of the strings inside. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 1 week ago by BigBossZhiling
Modified:
2 months, 1 week ago
CC:
agrieve+watch_chromium.org, chromium-reviews, jbudorick+watch_chromium.org, mikecase+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Make the table cells clickable, instead of the strings inside. Previously the strings inside table cells on clickable, but it takes accurate mouse control and aiming. In this cl, we are making the whole cell clickable, to make the table easier to use. BUG=717682 Review-Url: https://codereview.chromium.org/2888723002 Cr-Commit-Position: refs/heads/master@{#473035} Committed: https://chromium.googlesource.com/chromium/src/+/e111db0385aeceaf3e0c03f3f57568f908be46b5

Patch Set 1 #

Patch Set 2 : fixes #

Patch Set 3 : fixes #

Patch Set 4 : after sync #

Patch Set 5 : git fix sync error #

Patch Set 6 : cleaning up #

Total comments: 6

Patch Set 7 : address John's comments #

Total comments: 8

Patch Set 8 : fixes #

Total comments: 1

Patch Set 9 : address Yoland's comments #

Messages

Total messages: 15 (6 generated)
BigBossZhiling
Please look at the trybot of patch 5. Patch 6 is just cleaning up.
2 months, 1 week ago (2017-05-17 18:29:30 UTC) #2
jbudorick
Seems reasonable to me, but at least one of case or yoland should look, too. ...
2 months, 1 week ago (2017-05-17 18:34:07 UTC) #4
BigBossZhiling
https://codereview.chromium.org/2888723002/diff/100001/build/android/pylib/results/presentation/javascript/main_html.js File build/android/pylib/results/presentation/javascript/main_html.js (right): https://codereview.chromium.org/2888723002/diff/100001/build/android/pylib/results/presentation/javascript/main_html.js#newcode196 build/android/pylib/results/presentation/javascript/main_html.js:196: var tableCells=document.getElementsByTagName('td'); On 2017/05/17 18:34:06, jbudorick wrote: > nit: ...
2 months, 1 week ago (2017-05-17 20:08:23 UTC) #5
mikecase
https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/results/presentation/javascript/main_html.js File build/android/pylib/results/presentation/javascript/main_html.js (right): https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/results/presentation/javascript/main_html.js#newcode196 build/android/pylib/results/presentation/javascript/main_html.js:196: var tableCells = document.getElementsByTagName('td'); s/var/const ? https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/results/presentation/javascript/main_html.js#newcode197 build/android/pylib/results/presentation/javascript/main_html.js:197: for(var ...
2 months, 1 week ago (2017-05-17 21:20:22 UTC) #6
BigBossZhiling
Addressed case's comments. Good point on the 'let' keyword. Makes the code simpler. https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/results/presentation/javascript/main_html.js File ...
2 months, 1 week ago (2017-05-17 23:25:48 UTC) #7
the real yoland
lgtm https://codereview.chromium.org/2888723002/diff/140001/build/android/pylib/results/presentation/javascript/main_html.js File build/android/pylib/results/presentation/javascript/main_html.js (right): https://codereview.chromium.org/2888723002/diff/140001/build/android/pylib/results/presentation/javascript/main_html.js#newcode199 build/android/pylib/results/presentation/javascript/main_html.js:199: if (links.length == 1) { nit: you prob ...
2 months, 1 week ago (2017-05-18 00:17:14 UTC) #8
mikecase
lgtm
2 months, 1 week ago (2017-05-18 00:58:56 UTC) #9
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/2888723002/160001
2 months, 1 week ago (2017-05-18 19:04:33 UTC) #12
commit-bot: I haz the power
2 months, 1 week ago (2017-05-19 02:18:32 UTC) #15
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/e111db0385aeceaf3e0c03f3f575...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 25c286973