Performing a retrigger via the UI now requires two clicks due to backfill submenu

RESOLVED FIXED

Status

P3
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: gerald, Assigned: jfrench)

Tracking

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
Recently the retrigger icon has become a menu that contains two options: Retrigger and backfill.

Issues:
1. Some situations require retriggering certain job many times in a row (e.g., checking if a patch fixes an intermittent problem, and/or getting more info about it). This used to be easy: Click the retrigger icons N times. Now it takes 2 clicks at two different spots to launch 1 retrigger. -- Or is that done on purpose because of resource abuse?

2. The icon is shown with a downward arrowhead next to it. Traditionally (to me) this means the icon is the action that should happen when I click on it, and the arrow will show a menu (which may then modify the main icon).

3. There is ample space to the right of the retrigger icon (especially if the arrow is removed), a backfill icon could be put there.

So please consider removing the time-wasting menu, and moving the backfill action to its own top-level icon next to retrigger.

In case more actions will be added to this menu in the near future, I think issues #1 and #2 would still apply, so we could have the main/last action available with 1 click, and the arrow next to it to access other actions.
I could've sworn I commented somewhere on the bug that implemented the menu about this. Guess it must've been on IRC or something. I agree that it'd be nice to leave the main part of the button to performing retriggers, with the split menu to perform lesser-used functions like backfilling.

Until that happens, you can retrigger a job repeatedly by hitting the 'r' key a bunch of times.
Depends on: 1187394
(Reporter)

Comment 2

3 years ago
(In reply to Wes Kocher (:KWierso) from comment #1)
> I could've sworn I commented somewhere on the bug that implemented the menu
> about this. Guess it must've been on IRC or something.

I admit I didn't look for the bug implementing this menu.

> I agree that it'd be
> nice to leave the main part of the button to performing retriggers, with the
> split menu to perform lesser-used functions like backfilling.
> 
> Until that happens, you can retrigger a job repeatedly by hitting the 'r'
> key a bunch of times.

Thanks for the tip!

Updated

3 years ago
Blocks: 1187394
No longer depends on: 1187394
Summary: UI: Split time-wasting retrigger&backfill submenu into two top-level icons → Performing a retrigger via the UI now requires two clicks due to backfill submenu

Comment 3

3 years ago
(In reply to Gerald Squelart [:gerald] from comment #2)
> Thanks for the tip!

If it helps, that and more keyboard shortcuts are documented under help -> "user guide" :-)
(Assignee)

Comment 4

3 years ago
Yup, given the positive feedback received by Sheriffs prior to landing we went ahead with this menu relocation, but for power users I also posed we might consider a keyboard shortcut for Backfill job, like we already have for Retrigger job ('r').

Backstory is this relocation addressed the UX and Sheriff desire of:

o providing a more consistent access point for the action (Backfill, which was a "job detail" link prior)
o de-emphasizing Backfill similar to its previous location by having it as the 2nd menu entry
o not taking away Failure Summary space with another Actionbar icon unless we had to (currently at 5)
o allowing for significant new Actionbar functionality (via menus) if needed

Yes, if I recall correctly Wes and I chatted about making the button invoke some default action, but since none of the other ^ chevron dropdown menu-buttons in Treeherder/Perfherder/Logviewer do that, I think we held off on that for future consideration. I will enter a follow up for that possibility now :)

And yup for reference, all of our application shortcuts (navigation, pinboard, other actions) are available off the User Guide, from the Help menu. https://treeherder.mozilla.org/userguide.html. Though we have an open bug to make them available via a keyboard ('?') similar to Reviewable, Github, etc, which should be cool.
(Assignee)

Comment 5

3 years ago
Shortcut for Backfill job - bug 1207589
Button action invokes first menu entry - bug 1207592

Updated

3 years ago
See Also: → bug 1221652
Duplicate of this bug: 1221652

Updated

3 years ago
See Also: bug 1221652
(Assignee)

Comment 7

2 years ago
This was raised in a recent th weekly mtg again for some who prefer not to use keyboard shortcuts. I had a look to see how much more space we'd consume, we go from 250px to 300px in info-panel-navbar to accommodate a separate backfill icon in the button row.

I'll attach a screen grab.
(Assignee)

Comment 8

2 years ago
Created attachment 8804913 [details]
backfillBreakout

This shows the typical max icon scenario: pinned to 1 digit, logged in, reftest job. I'm using a mirrored retrigger as a 'backfill' only for the purposes of the check. We end up needing 314px. So +64px from our current width.
(Assignee)

Comment 10

2 years ago
(In reply to Jonathan French (:jfrench) from comment #8)
> We end up needing 314px. So +64px from our current width.

I think reftest and cancel cannot exist at the same time - achieving that economy we get away with almost no change in actionbar width. Anyway I'll put up a PR, in case you guys decide to make the change.
Created attachment 8805178 [details] [review]
[treeherder] tojon:split-retrigger-ui > mozilla:master
I'd still halfheartedly argue that it'd be better as a split button dropdown thing (like the "save classification" button and the menu attached to it), where the button on the left is retrigger, and the dropdown on the right has the backfill button. Having two clicks to get to backfill would hopefully cut down on accidentally backfilling things, which could trigger lots of unintended jobs if you accidentally misclick it.
(Assignee)

Comment 13

2 years ago
One solution could be to generalize backfill in a fa-bars icon w/dropdown menu at the right hand side of that action bar? So if additional 'powerful' actions appear in the future, they could be added as new menus there.
I don't think "hide backfill behind two clicks" is the right solution to the possibility of creating many (usually not that many) jobs. If we think it's dangerous it should have a confirm prompt like the one for cancelling a push.

In other news, I am strongly in favour of this change :)
(Assignee)

Comment 15

2 years ago
Created attachment 8805597 [details]
dedicatedActionbarMenu

In the last hour or so while :jgraham's comment was being written I was blocking out a generalized right hand bar menu for backfill and any future powerful functions, just out of interest :) I've attached it for posterity.

Let me know when we achieve backfill consensus (1 click btn w/no confirm (initial PR) vs. 2 clicks w/menu entry (above attachment) vs. 2 clicks btn w/confirm box) and I'll look at making whichever change is finalized.
Yes, I really like that idea!

What about a split button [1] instead though, since backfill is still related to re-triggering so makes sense to still group it together somehow. I think we could also put the "One-click loaner" link from bug 1285007 in that dropdown, as that is also related to re-triggering.

[1] http://i.imgur.com/xrJaU.png
(Assignee)

Comment 17

2 years ago
I guess that means folks are comfortable with the increased chance of accidentally retriggering a job while attempting to post the split button menu? Since retrigger and its split button will be in somewhat closer proximity to each other, than separate buttons.

I am fine with whatever you guys collectively decide upon. Let me know what gets decided and I will make the change.
(Assignee)

Comment 18

2 years ago
So, so far we have jgraham for single btn, ahal is ok to try either single or split, so wondering KWierso if we could consider landing the single btn implementation to see if it is acceptable? We could always switch it to a split button at a later date if users find Backfill is too prominent?
Assignee: nobody → tojonmz
Status: NEW → ASSIGNED
Flags: needinfo?(wkocher)
Priority: -- → P3
Meh, go for it.
Flags: needinfo?(wkocher)
(Assignee)

Comment 20

2 years ago
Comment on attachment 8805178 [details] [review]
[treeherder] tojon:split-retrigger-ui > mozilla:master

Ok, adding Will for a review.
Attachment #8805178 - Flags: review?(wlachance)
Comment on attachment 8805178 [details] [review]
[treeherder] tojon:split-retrigger-ui > mozilla:master

Code changes lgtm, some now-dead code should be removed.
Attachment #8805178 - Flags: review?(wlachance) → review+
(Assignee)

Comment 22

2 years ago
Yup, I've removed that partial and squashed, we should be all good to go.
Thanks jfrench!
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED

Comment 25

2 years ago
Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/67f83a2033b2e004e8b87e34027d7d2f4145fb00
Revert "Bug 1207460 - Split retrigger menu into separate btns (#1960)"

The retrigger and backfill buttons don't work after this change.  I think
an ng-click was forgotten in the PR.

This reverts commit 86235ae3e4ee5b47f6cf59fce2e0f7c6c7268bdb.
I had to revert this change because the retrigger button became a no-op.  I think some ng-clicks got lost in the shuffle.  :)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 27

2 years ago
Gah, thanks. Need to migrate the ng-clicks from the retired menu.
Created attachment 8813354 [details] [review]
[treeherder] tojon:split-retrigger-again > mozilla:master
(Assignee)

Updated

2 years ago
Attachment #8805178 - Attachment is obsolete: true
(Assignee)

Comment 29

2 years ago
Comment on attachment 8813354 [details] [review]
[treeherder] tojon:split-retrigger-again > mozilla:master

Adding wlach for a deja-vu review at your leisure :)

When I get LDAP again I will try the user.loggedin test cases locally, or feel free to fire off a couple to be sure it works as expected.
Attachment #8813354 - Flags: review?(wlachance)
Comment on attachment 8813354 [details] [review]
[treeherder] tojon:split-retrigger-again > mozilla:master

This worked well for me on staging, so I merged.
Attachment #8813354 - Flags: review?(wlachance) → review+
Created attachment 8815879 [details] [review]
[treeherder] tojon:actionbar-btn-disable-color > mozilla:master
(Assignee)

Comment 32

2 years ago
Comment on attachment 8815879 [details] [review]
[treeherder] tojon:actionbar-btn-disable-color > mozilla:master

Adding that little tweak PR for review. :)
Attachment #8815879 - Flags: review?(wlachance)
Comment on attachment 8815879 [details] [review]
[treeherder] tojon:actionbar-btn-disable-color > mozilla:master

lgtm
Attachment #8815879 - Flags: review?(wlachance) → review+

Updated

2 years ago
Depends on: 1321910
(Assignee)

Comment 35

2 years ago
Bug 1321910 is now fine, so I think we're good here :)
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.