1118253 - Not possible to clear job selection when clicking in page footer or push header

Status: VERIFIED FIXED

(Tree Management :: Treeherder, defect, P3)

Description

[James Graham [:jgraham]]

Once a job has been selected it isn't possible to get back to a state where no job is selected. This is unexpected (it's not how focus usually works) and, because of the way that focusing a job changes the layout, makes the overall status of a push harder to read.

Comment 1

[Jonathan French (:jfrench)]

You should be able to click anywhere on the page that isn't a job or anchor, and you will get back to the state where no job is selected, per above. Or are you requesting something else?

Comment 2

[James Graham [:jgraham]]

Oh I see, so it does work, but it's buggy. Specifically consider a single-push view like https://treeherder.mozilla.org/#/jobs?repo=try&revision=c99df8976076 If you focus a job and then click somewhere else on the job lines it will defocus. If you click in the white space below the list of jobs it won't.

Comment 3

[Jonathan French (:jfrench)]

Cool, I see it. Ya, it ignores clear events in the footer region. Also observable when issuing a clearing click in the 'get next | 10 20 50' footer in a regular multi-resultset view.

Comment 4

[Jonathan French (:jfrench)]

Adjusting the title accordingly.

Comment 5

[Ed Morley [:emorley]]

It's also not possible to clear the focus when clicking on the push header too.

Comment 7

[Julien Wajsberg [:julienw]]

Same when clicking on the white space below a job when one job only is selected.
(for example here: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=f1fdc185ca0d)

Comment 8

[Jonathan French (:jfrench)]

Having a look at this now, will assign myself if I get all the bits sorted.

Comment 9

[Jonathan French (:jfrench)]

I have a basic fix we can try out on dev for the footers. It's true the jobs don't clear in the empty areas in the resultset header either. I chatted with camd on that, I was concerned the mechanics of a fix in the header might be a bit more fragile. I think it would require a clearJob() be issued across the entire th-content, and then the propagation blocked for each of the buttons, filters, the SHA string, and elsewhere across the UI so users could interact without closing the job.

Anyway, just curious what Ed thinks about that, if we maybe bypass the header fix for now.

The simple fix I have for the footers addresses the get-next-job footer, and also creates a fixed empty div below it, which receives the clearJob() for single resultsets that are really short. It could be made dynamic later to always match the page dimension, but for now I was just going to make it a fixed height for simplicity, 500, 1000px or similar if everyone agrees.

Comment 10

[Ed Morley [:emorley]]

Spoke on IRC (tl;dr 302 camd + partial solution sgtm)

Comment 11

[Jonathan French (:jfrench)]

Attachment: treeherder-ui-PR#350

Please see above PR for status and review.

I think we should have folks try it out on dev first, before push to stage/prod. Just to confirm people can live without a dynamically sized spacer for now.

Comment 12

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder-ui

https://github.com/mozilla/treeherder-ui/commit/c8816abf3d6e3411078b411e8c7eda30db293afd
Bug 1118253 - Clear job selection in page bottom and get-next-footer

https://github.com/mozilla/treeherder-ui/commit/2177456a8b3f8fd011f83cbc8bfeb43b571a7721
Merge pull request #350 from tojonmz/clear-footer

Bug 1118253 - Clear job selection in page bottom and get-next-footer

Comment 13

[Jonathan French (:jfrench)]

Marking fixed per above merge. James, Julien, we have the footer fixes sitting on the treeherder-dev instance if you wish to try it out.

Comment 14

[Julien Wajsberg [:julienw]]

Where's the treeherder-dev instance? If it's treeherder.allizom.org then it doesn't work for me :/

Comment 15

[Mauro Doglio [:mdoglio]]

http://treeherder-dev.allizom.org

Comment 16

[Ed Morley [:emorley]]

Links to dev/stage/prod are on:
https://wiki.mozilla.org/Auto-tools/Projects/Treeherder#Contributing

Comment 17

[Julien Wajsberg [:julienw]]

Ok then, it works fine :) I still miss to remove the selection clicking on the headers but it's already a huge improvement for us working on Gaia with only a few builds/jobs :)

Comment 18

[Jonathan French (:jfrench)]

Thanks Julien! We've had some concerns from sheriffs after a push to prod this morning about the 'spacer approach' to deselection below the get-next footer. We may have to try to implement it differently, but that request is still a valid expected feature. We're going to just fix get-next clear for now, and keep this bug open for the clearing behavior below it.

Comment 19

[Jonathan French (:jfrench)]

I've opened the PR to revert the footer spacer portion of the fix.
https://github.com/mozilla/treeherder-ui/pull/352

Comment 20

[Jonathan French (:jfrench)]

Attachment: treeherder-ui-PR#352

Adding wlach for review, per above.

Comment 21

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder-ui

https://github.com/mozilla/treeherder-ui/commit/edb2261f0c3652cf7e8b84f3f5d9778b88e028c7
Bug 1118253 - Remove footer spacer for a future clearing method

https://github.com/mozilla/treeherder-ui/commit/5112dd259ea978a8e0d61e2e3eceee7fb9ab3f3c
Merge pull request #352 from tojonmz/footer-spacer-removal

Bug 1118253 - Remove footer spacer for a future clearing method

Comment 22

[William Lachance (:wlach)]

Comment on attachment 8557179 [details] [review]
treeherder-ui-PR#352

Looks reasonable from what I've been able to observe. Merged.

Comment 23

[Jonathan French (:jfrench)]

I've made some progress locally for Julien for general deselection in th-content. Still working on it.

Comment 24

[Jonathan French (:jfrench)]

I've got most of the main UI elements working as desired (ie. not clearing the selected job when clicked), while supporting clear everywhere else on the main page. It's pretty cool. I still have a few pieces of UI still to do, and a few other things.

Comment 25

[Jonathan French (:jfrench)]

Attachment: treeherder-ui-PR#358

Preliminary PR for review by Cameron, and testing by Ed at their leisure.

Comment 26

[Ed Morley [:emorley]]

Comment on attachment 8560293 [details] [review]
treeherder-ui-PR#358

:-)

Comment 27

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder-ui

https://github.com/mozilla/treeherder-ui/commit/3f45f266da3f8023e5d73005cba691f6fc9edfa9
Bug 1118253 - Deselect job when clicking any empty page area

https://github.com/mozilla/treeherder-ui/commit/4452d81a11e9bcf8b73aeec5d46d635985e2f514
Merge pull request #358 from tojonmz/clear-job-openareas

Bug 1118253 - Deselect job when clicking any open area on main page

Comment 28

[Jonathan French (:jfrench)]

Marking Fixed per above merge. I will Verify on stage/prod when its pushed.

Comment 29

[Jonathan French (:jfrench)]

Verified fixed on stage.

Comment 30

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/67420de5dde1bb72246dad0ba5d19d1d699beb6d
Bug 1118253 - Clear job selection in page bottom and get-next-footer

https://github.com/mozilla/treeherder/commit/c4d887f8190acb62ee204752654f804b323a7963
Merge pull request #350 from tojonmz/clear-footer

Bug 1118253 - Clear job selection in page bottom and get-next-footer

https://github.com/mozilla/treeherder/commit/6e9e7d09abb930abff4c066611dbb241c9ca7396
Bug 1118253 - Remove footer spacer for a future clearing method

https://github.com/mozilla/treeherder/commit/0cdb89686c8985a9069967f4c2a9d12f0ede8d1f
Merge pull request #352 from tojonmz/footer-spacer-removal

Bug 1118253 - Remove footer spacer for a future clearing method

https://github.com/mozilla/treeherder/commit/ffdf0a7265bf710e362dc54581129786246a0648
Bug 1118253 - Deselect job when clicking any empty page area

https://github.com/mozilla/treeherder/commit/016537a108808cc8ee12086d81c95a61a70047cd
Merge pull request #358 from tojonmz/clear-job-openareas

Bug 1118253 - Deselect job when clicking any open area on main page