1238468 - Delayed insertion of the Cancel button creates a UI pitfall

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect)

Description

[Jonathan Kew [:jfkthame]]

When I click on the symbol of an individual running job in Treeherder, the buttons above the info panel are refreshed. Initially, four buttons appear:
- Parsed log
- Full log
- Pin job
- Retrigger/backfill
(although the first two are unavailable for an in-progress job).

Then, a moment later (sometimes rather a long moment, depending on the latency of querying the job status, I guess), if it's a pending or running job then the Cancel button is inserted.

The problem is that Cancel is inserted *before* the Retrigger/Backfill button, causing it to move across to the right. What happened to me recently was that I clicked on a running job I wished to retrigger (trying a possible fix for an intermittent failure), and then went to click the Retrigger button. But just as I clicked it, Cancel was inserted in its place and Retrigger moved away -- so I ended up accidentally cancelling the job. :(

Two alternative possibilities to avoid this scenario:
(a) add Cancel *after* the Retrigger button instead of before it, so that inserting it doesn't cause any already-visible controls to move; or
(b) let Cancel always be present, just disable it when not relevant.

Comment 1

[William Lachance (:wlach)]

I was going to say (b), but after reading this I think hiding it for completed jobs is probably better, since it's never going to make sense to cancel a completed job:

http://www.uxbooth.com/articles/interaction-design/who-killed-the-inactive-button-state/

That suggests (a) is the right thing to do. I did up a quick patch.

Comment 2

[GitHub Autolander Bot]

Attachment: [treeherder] wlach:1238468 > mozilla:master

Comment 3

[William Lachance (:wlach)]

Comment on attachment 8706515 [details] [review]
[treeherder] wlach:1238468 > mozilla:master

Hey :jfrench, do you want to give this a quick sanity check?

Comment 4

[Jonathan French (:jfrench)]

Comment on attachment 8706515 [details] [review]
[treeherder] wlach:1238468 > mozilla:master

The code change is an r+, but I notice having additionally moved the Reftest button from its trailing position, when transitioning selection from a non-refest job to a reftest job (or vice versa) the last 3 buttons bounce around a lot and change position. Where previously the first 5 (log, raw log, pin, retrigger, cancel) would stay put since reftest got appended.

I defer to you guys if that is what you want. I totally get the logic that it is similar to the logs, I'm just not sure that is desirable in the context of this bug.

Comment 5

[William Lachance (:wlach)]

Ok, let's just move the cancel button for now. I suspect it doesn't really make that much of a difference where the reftest button is.

Comment 6

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla/treeherder/commit/29a7a096e00bd11cdf5e90950da415950db7faaa

Comment 7

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/d799e3c724477a49acc2a3a8e318fabed747240b
Bug 1238468 - Update ordering of job panel buttons

* Reftest analyzer goes after log buttons (since it's logically similar)
* Cancel button comes after retrigger (so it doesn't displace the retrigger
  button if it's slow to load)

https://github.com/mozilla/treeherder/commit/29a7a096e00bd11cdf5e90950da415950db7faaa
Merge pull request #1241 from wlach/1238468

Comment 8

[William Lachance (:wlach)]

(In reply to Treeherder Bugbot from comment #7)
> Commits pushed to master at https://github.com/mozilla/treeherder
> 
> https://github.com/mozilla/treeherder/commit/
> d799e3c724477a49acc2a3a8e318fabed747240b
> Bug 1238468 - Update ordering of job panel buttons
> 
> * Reftest analyzer goes after log buttons (since it's logically similar)
> * Cancel button comes after retrigger (so it doesn't displace the retrigger
>   button if it's slow to load)

Eugh, forgot to update the commit message. Rest assured that the reftest button is back where it was before.