408.88 KB, image/jpeg
129.32 KB, image/jpeg
47 bytes, text/x-github-pull-request
|Details | Review | Splinter Review|
47 bytes, text/x-github-pull-request
|Details | Review | Splinter Review|
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
(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!
(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" :-)
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.
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.
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.
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
(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.
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 :)
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  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.  http://i.imgur.com/xrJaU.png
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.
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
Priority: -- → P3
Meh, go for it.
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+
Yup, I've removed that partial and squashed, we should be all good to go.
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)
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
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 → ---
Gah, thanks. Need to migrate the ng-clicks from the retired menu.
Created attachment 8813354 [details] [review] [treeherder] tojon:split-retrigger-again > mozilla:master
Attachment #8805178 - Attachment is obsolete: true
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
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+
Commit pushed to master at https://github.com/mozilla/treeherder https://github.com/mozilla/treeherder/commit/16188c4d84f5bab0d5fbcdf3fe92b7b77d0e2935 Bug 1207460 - Add actionbar button color (#2010)
Bug 1321910 is now fine, so I think we're good here :)
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago → 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.