Make the table cells clickable, instead of the strings inside.

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

[BigBossZhiling, 2017-05-17 18:29:29]

hzl@google.com changed reviewers:
+ jbudorick@chromium.org

[BigBossZhiling, 2017-05-17 18:29:30]

Please look at the trybot of patch 5. Patch 6 is just cleaning up.

[jbudorick, 2017-05-17 18:34:06]

jbudorick@chromium.org changed reviewers:
+ mikecase@chromium.org, yolandyan@chromium.org

[jbudorick, 2017-05-17 18:34:07]

Seems reasonable to me, but at least one of case or yoland should look, too.

https://codereview.chromium.org/2888723002/diff/100001/build/android/pylib/re...
File build/android/pylib/results/presentation/javascript/main_html.js (right):

https://codereview.chromium.org/2888723002/diff/100001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:196: var
tableCells=document.getElementsByTagName('td');
nit: add spaces around =

https://codereview.chromium.org/2888723002/diff/100001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:197: for(var i
= 0; i < tableCells.length; i++){
nits: also add spaces between ){, here and below

https://codereview.chromium.org/2888723002/diff/100001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:201:
tableCells[i].addEventListener('click',function(){
nits: space after , here and below

[BigBossZhiling, 2017-05-17 20:08:23]

https://codereview.chromium.org/2888723002/diff/100001/build/android/pylib/re...
File build/android/pylib/results/presentation/javascript/main_html.js (right):

https://codereview.chromium.org/2888723002/diff/100001/build/android/pylib/re...
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: add spaces around =

Done.

https://codereview.chromium.org/2888723002/diff/100001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:197: for(var i
= 0; i < tableCells.length; i++){
On 2017/05/17 18:34:06, jbudorick wrote:
> nits: also add spaces between ){, here and below

Done.

https://codereview.chromium.org/2888723002/diff/100001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:201:
tableCells[i].addEventListener('click',function(){
On 2017/05/17 18:34:06, jbudorick wrote:
> nits: space after , here and below

Done.

[mikecase (-- gone --), 2017-05-17 21:20:22]

https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/re...
File build/android/pylib/results/presentation/javascript/main_html.js (right):

https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/re...
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/re...
build/android/pylib/results/presentation/javascript/main_html.js:197: for(var i
= 0; i < tableCells.length; i++) {
nit: s/var/let

And then I think you can remove that function(i){}(), maybe?

https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:199: var links
= tableCells[i].getElementsByTagName('a');
s/var/const

https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:201:
tableCells[i].addEventListener('click',function() {
nit: app space after comma. Same nit applies a few time below

[BigBossZhiling, 2017-05-17 23:25:48]

Addressed case's comments. Good point on the 'let' keyword. Makes the code
simpler.

https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/re...
File build/android/pylib/results/presentation/javascript/main_html.js (right):

https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:196: var
tableCells = document.getElementsByTagName('td');
On 2017/05/17 21:20:21, mikecase wrote:
> s/var/const ?

Done.

https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:197: for(var i
= 0; i < tableCells.length; i++) {
On 2017/05/17 21:20:21, mikecase wrote:
> nit: s/var/let
> 
> And then I think you can remove that function(i){}(), maybe?

Good point. I didn't know about 'let'.

https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:199: var links
= tableCells[i].getElementsByTagName('a');
On 2017/05/17 21:20:21, mikecase wrote:
> s/var/const

Done.

https://codereview.chromium.org/2888723002/diff/120001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:201:
tableCells[i].addEventListener('click',function() {
On 2017/05/17 21:20:21, mikecase wrote:
> nit: app space after comma. Same nit applies a few time below

Done.

[the real yoland, 2017-05-18 00:17:14]

lgtm

https://codereview.chromium.org/2888723002/diff/140001/build/android/pylib/re...
File build/android/pylib/results/presentation/javascript/main_html.js (right):

https://codereview.chromium.org/2888723002/diff/140001/build/android/pylib/re...
build/android/pylib/results/presentation/javascript/main_html.js:199: if
(links.length == 1) {
nit: you prob want to comment on that only make the table clickable if there are
only one link in the table

[mikecase (-- gone --), 2017-05-18 00:58:56]

lgtm

[BigBossZhiling, 2017-05-18 19:03:29]

The CQ bit was checked by hzl@google.com

[BigBossZhiling, 2017-05-18 19:03:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
yolandyan@chromium.org
Link to the patchset: https://codereview.chromium.org/2888723002/#ps160001
(title: "address Yoland's comments")

[commit-bot: I haz the power, 2017-05-18 19:04:33]

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-05-19 02:18:17]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1495134209037660,
"parent_rev": "2334c53d452d01b8ce2f5b40b94eca119c424234", "commit_rev":
"e111db0385aeceaf3e0c03f3f57568f908be46b5"}

[commit-bot: I haz the power, 2017-05-19 02:18:31]

Description was changed from

==========
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
==========

to

==========
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/+/e111db0385aeceaf3e0c03f3f575...
==========

[commit-bot: I haz the power, 2017-05-19 02:18:32]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/e111db0385aeceaf3e0c03f3f575...