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

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P3)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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.

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(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!

Comment 3

[Ed Morley [:emorley]]

(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" :-)

Comment 4

[Jonathan French (:jfrench)]

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.

Comment 5

[Jonathan French (:jfrench)]

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

Comment 7

[Jonathan French (:jfrench)]

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.

Comment 8

[Jonathan French (:jfrench)]

Attachment: 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.

Comment 9

[Jonathan French (:jfrench)]

Some possible icon options for a backfill button

http://fontawesome.io/icon/arrow-down
http://www.flaticon.com/free-icon/shovel_259644 (svg, like our log icon)
http://www.flaticon.com/free-icon/shovel_248126
http://www.flaticon.com/free-icon/dump-truck_224102

Comment 10

[Jonathan French (:jfrench)]

(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.

Comment 11

[GitHub Autolander Bot]

Attachment: [treeherder] tojon:split-retrigger-ui > mozilla:master

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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.

Comment 13

[Jonathan French (:jfrench)]

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.

Comment 14

[James Graham [:jgraham]]

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 :)

Comment 15

[Jonathan French (:jfrench)]

Attachment: 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.

Comment 16

[Andrew Halberstadt [:ahal]]

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

Comment 17

[Jonathan French (:jfrench)]

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.

Comment 18

[Jonathan French (:jfrench)]

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?

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Meh, go for it.

Comment 20

[Jonathan French (:jfrench)]

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

Ok, adding Will for a review.

Comment 21

[William Lachance (:wlach)]

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

Code changes lgtm, some now-dead code should be removed.

Comment 22

[Jonathan French (:jfrench)]

Yup, I've removed that partial and squashed, we should be all good to go.

Comment 23

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/86235ae3e4ee5b47f6cf59fce2e0f7c6c7268bdb
Bug 1207460 - Split retrigger menu into separate btns (#1960)

Comment 24

[William Lachance (:wlach)]

Thanks jfrench!

Comment 25

[Treeherder GitHub Bugbot]

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.

Comment 26

[Cameron Dawson [:camd]]

I had to revert this change because the retrigger button became a no-op.  I think some ng-clicks got lost in the shuffle.  :)

Comment 27

[Jonathan French (:jfrench)]

Gah, thanks. Need to migrate the ng-clicks from the retired menu.

Comment 28

[GitHub Autolander Bot]

Attachment: [treeherder] tojon:split-retrigger-again > mozilla:master

Comment 29

[Jonathan French (:jfrench)]

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.

Comment 30

[William Lachance (:wlach)]

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

This worked well for me on staging, so I merged.

Comment 31

[GitHub Autolander Bot]

Attachment: [treeherder] tojon:actionbar-btn-disable-color > mozilla:master

Comment 32

[Jonathan French (:jfrench)]

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

Adding that little tweak PR for review. :)

Comment 33

[William Lachance (:wlach)]

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

lgtm

Comment 34

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/16188c4d84f5bab0d5fbcdf3fe92b7b77d0e2935
Bug 1207460 - Add actionbar button color (#2010)

Comment 35

[Jonathan French (:jfrench)]

Bug 1321910 is now fine, so I think we're good here :)