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

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

Created:
1 year, 1 month ago by BigBossZhiling
Modified:
1 year, 1 month 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 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -4 lines) Patch
M build/android/pylib/results/presentation/javascript/main_html.js View 1 2 3 4 5 6 7 8 1 chunk +21 lines, -0 lines 0 comments Download
M build/android/pylib/results/presentation/template/main.html View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M build/android/pylib/results/presentation/template/table.html View 1 2 3 2 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 15 (6 generated)
BigBossZhiling
Please look at the trybot of patch 5. Patch 6 is just cleaning up.
1 year, 1 month 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. ...
1 year, 1 month 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: ...
1 year, 1 month ago (2017-05-17 20:08:23 UTC) #5
mikecase (-- gone --)
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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month ago (2017-05-18 00:17:14 UTC) #8
mikecase (-- gone --)
lgtm
1 year, 1 month 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
1 year, 1 month ago (2017-05-18 19:04:33 UTC) #12
commit-bot: I haz the power
1 year, 1 month 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...

Powered by Google App Engine
This is Rietveld 408576698