1118342 - Hitting ctrl+enter should submit a classification even if a textbox is focused

Status: VERIFIED FIXED

(Tree Management :: Treeherder, defect, P4)

Description

[Ryan VanderMeulen [:RyanVM]]

Right now, if you hit 'c' or 'r', it'll bring up the appropriate textbox for adding the relevant note. However, if you hit ctrl+enter from that point, it takes it as a plain enter and unfocuses the textbox, forcing you to ctrl+enter it again. TH should just save the extra keystroke and submit immediately when ctrl+enter is hit regardless of whether a textbox is focused or not.

Comment 1

[Jonathan French (:jfrench)]

I will have a look, but this UX seems very odd to me to try to allow the invocation of a hotkey while simultaneously focused in an input (whether it be a web browser or any other GUI desktop application).

Comment 2

[Jonathan French (:jfrench)]

I had a look at this using mousetrap.js and have a fix, but refactoring other shortcuts with mousetrap seems problematic because they suffer major response lag (eg. I tried clear all and it was terrible). The particular bug in question for save works fine for some reason. I don't know why, but I have a fix if we want to have some sort of blend of shortcut approaches in controllers/main.

I'm having a look now at jKey also.

Comment 3

[Jonathan French (:jfrench)]

On a quick read it was unclear if jKey would meet all of the treeherder interaction requirements. Also unsure about shortcuts.js atm.

I am still pursuing mtrap and have working shortcuts for all functions, and now with reasonable responsiveness thanks to camd. A number of shortcuts like 'clear all' required additional angular digest cycles to work properly with mtrap. Subjectively, mousetrap shortcuts seem on the order of 1/10th to 1/4 second slower than the current implementation. It may be different on machines and OS other than mine of course (better or worse). Whether that will meet sheriff performance needs is unknown atm.

There also remains the task of excluding single character shortcuts in the targeted input fields add-related-bugs and classification-comment which are supposed to simultaneously accept shortcuts that use modifiers (eg. 'ctrl+enter', 'ctrl+shift+u' vs. 'c' 'r' 'space' etc). I have to dig around more to see if/how that is done in mtrap or if it is possible.

Comment 4

[Jonathan French (:jfrench)]

I have the work completed and tested which suppresses single character shortcuts in text/input areas. I'm trying to sort out another focus problem in the add-related-bug field specific to Firefox with the new interaction. It leaves the input field up after save. Chrome does not, and works fine.

I may end up opening another bug for the mousetrap refactor, and make this bug a dependent.

Comment 5

[Jonathan French (:jfrench)]

I've opened PR https://github.com/mozilla/treeherder-ui/pull/338 against bug 1121631 for camd's review.

Comment 6

[Jonathan French (:jfrench)]

Marking fixed per above merge. Will re-test on stage/prod.

Comment 7

[Jonathan French (:jfrench)]

There was a speed issue with toggle 'unclassified' which resulted in PR338 needing to be reverted. I'm still looking at that issue. Marking this dependent bug reopened for now.

Comment 8

[Jonathan French (:jfrench)]

I think I have the toggle unclassified speed issue sorted, and have opened PR https://github.com/mozilla/treeherder-ui/pull/368 against bug 1121631 for camd's review.

I also re-tested the above branch with Save while the comments field is focused for this bug request, and it's still working fine.

Comment 9

[Jonathan French (:jfrench)]

Camd has kindly pushed PR368 to dev, I've re-tested this there and it supports the ctrl+enter save workflow as expected. I'm holding off on marking this fixed until we hear final sign-off on bug 1121631 from Sheriffs.

Comment 10

[Jonathan French (:jfrench)]

So we have the mousetrap stuff landed and working in production. Save ('ctrl+enter') while the classification comments field is focused is working, but the related bug field during focus isn't. I believe it was working fine during my branch work, but other work I think may have touched the related bugs stuff in the interim.

So 1121631 we will likely be able to close shortly as fixed, and we can leave this bug open for continued work to get the related bug field working with ctrl+enter, for save.

Comment 11

[Jonathan French (:jfrench)]

Attachment: treeherder-ui-PR#385

Supplemental support for save when focused in add-related-bug, provided in this PR. Please see above for review and status.

Comment 12

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder-ui/commit/95dc701b9f22893e57d1be7ac314eff54960f1bb
Bug 1118342 - Support multi-key shortcuts when focused in add-related-bug

https://github.com/mozilla/treeherder-ui/commit/982e147fd2b3b063e83208c632cb6c18c1770328
Merge pull request #385 from tojonmz/related-bug-1118342

Bug 1118342 - Support ctrl+enter save when focused in add-related-bug

Comment 13

[Jonathan French (:jfrench)]

Marking fixed per above merge. I will verify on the next push to stage/prod.

Comment 14

[Jonathan French (:jfrench)]

Verified fixed on stage and prod.

Comment 15

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/3de745434b031b50519a6cd5396239ef8dfb1b5e
Bug 1118342 - Support multi-key shortcuts when focused in add-related-bug

https://github.com/mozilla/treeherder/commit/7aaafdae1f92141353a42e477f1ae86693f0cc33
Merge pull request #385 from tojonmz/related-bug-1118342

Bug 1118342 - Support ctrl+enter save when focused in add-related-bug