If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Delayed insertion of the Cancel button creates a UI pitfall

RESOLVED FIXED

Status

Tree Management
Treeherder
RESOLVED FIXED
2 years ago
7 months ago

People

(Reporter: jfkthame, Assigned: wlach)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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.
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

2 years ago
Created attachment 8706515 [details] [review]
[treeherder] wlach:1238468 > mozilla:master
Comment on attachment 8706515 [details] [review]
[treeherder] wlach:1238468 > mozilla:master

Hey :jfrench, do you want to give this a quick sanity check?
Attachment #8706515 - Flags: review?(tojonmz)
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.
Attachment #8706515 - Flags: review?(tojonmz) → review+
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.
Keywords: autoland

Comment 6

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

Comment 7

2 years ago
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

Updated

2 years ago
Keywords: autoland
(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.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED

Updated

7 months ago
Assignee: nobody → wlachance
You need to log in before you can comment on or make changes to this bug.