new Angular eg-grid doesn't include right click Actions menu

[Andrea Neiman]

3.2.1

Looking at some Acquisitions grids that were ported to new Angular for 3.2.

Actions are no longer available from a right-click in the grid, and this should be restored.

[Bill Erickson (berick)]

Patches pushed:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1803787-ang-grid-context-menu

There are two commits:

1. Adds support for <eg-grid-toolbar-action [separator]="true".../> for displaying separators in the grid actions drop-down.

2. Adds context menu display for grids matching the content of the toolbar actions, supporting separators.

Note the menu display uses an ng-bootstrap popover, which has no baked-in concept of displaying at the mouse action. Instead, the menu appears below the grid item upon which the right-click occurred. In most cases, this will be very close to the right-click, except when a grid column is very wide. I opted for this to reduce code (since we already import bootstrap), but if the display is irksome, more research may be needed and/or we may have to roll our own context menu.

[John Amundson (jamundson)]

I took a quick look at this patch today.

I'll start by saying the right-click menu looks really nice. The menu seems to appear below the selected row, midway through the width of the "hovered" column, but I didn't find this awkward. The little arrow is a nice touch. The couple actions I tested both worked from the right-click menu.

Having said that, I did have one issue. When I had multiple rows selected and I right-clicked in the highlighted area, only the row I right-clicked over stayed highlighted. I expected the rows to stay highlighted and for the action to apply to all selected rows.

[Bill Erickson (berick)]

Thanks, John. I'll take a look at the multi-row issue.

[Bill Erickson (berick)]

Working branch rebased to master with merge repairs. Added another commit to address the row selection issues raised by John.

When right-clicking on a row, if the row was not already selected, then that row (and only that row) is selected. If the row was already selected (and potentially others along with it), the row selection is left unchanged and the context menu action will be performed across multiple rows (where supported).

To test:

1. navigate to admin -> acq admin -> cancel reasons.
2. Create 2 new dummy cancel reasons
3. Control-click to muli-select the 2 new cancel reasons.
4. right click on either of the cancel reasons and note the selection is retained.
5. chose the delete action and note both rows deleted after refresh.

[Bill Erickson (berick)]

Note that after rebase, the working branch only merges cleanly to master / 3.3. If we want to back-port, I can create a 3.2 compatible branch.

[Andrea Neiman (aneiman)]

Thanks Bill! I'm +1 for backporting if it's not too much of a hassle.

[Bill Erickson (berick)]

Thanks for the input, Andrea. I'll push a 3.2 branch.

[Bill Erickson (berick)]

Here's a 3.2-compatible branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1803787-ang-grid-context-menu-3.2

[Jane Sandberg (sandbergja)]

Thanks for the two branches, Bill!

A few quick observations:

* It looks like the user/berick/lp1803787-ang-grid-context-menu branch no longer applies cleanly to master
* It looks like the right-click menu doesn't honor the disableOnRows callback
* I didn't have a chance to check how i18n works with this popover, but I didn't see any i18n attributes. Will the same translation appear in both the popover and the Actions menu?

Thanks!

[Bill Erickson (berick)]

Thanks, Jane!

I'm rebasing the branch now. I'll add support for disableOnRows while I'm in there.

Regarding i18n, the popover uses the same labels as the grid actions menu, which in turn are provided as i18n-ified strings by the caller. {{action.label}} for example will display the translated value.

[Jane Sandberg (sandbergja)]

Thanks for the i18n explanation, Bill. That sounds excellent.

I did some basic keyboard navigation testing on this, too. A few notes from that:

* The entries in the context menu are <a> tags without href attributes, which are not keyboard focusable. Could you please add a href attribute or change them into buttons, or do something else to make them focusable?
* Once menu items are focusable, as soon as the context menu opens, the first menu item should receive focus. (See the keyboard nav section under "Menu" in the WAI-Aria authoring practices: https://www.w3.org/TR/wai-aria-practices/#menu)
* On Linux in Firefox and Chrome, pressing Shift + F10 opens the browser's default context menu, rather than the eg-grid one. My laptop keyboard doesn't have a menu button, so I didn't have a chance to test with that. I also don't know what keyboard shortcuts a Mac user might expect. This seems less important, since keyboard users can still access the Actions menu, but it might be nice to allow a slick keyboard shortcut to open up the context menu too.

[Bill Erickson (berick)]

Force-pushed a master compat branch back to

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1803787-ang-grid-context-menu

* Moves toolbar action menu entries into a standalone component which can be used by both the popover and the dropdown menu.
* Adds support for disableOnRows for the popover

Thanks, Jane. The move from <a> to <button> was incidentally done by the refactoring. I have not addressed the other 2 items from your previous list.

[Bill Erickson (berick)]

Also note the rebased code required quite a few changes, so the path to a 3.2 back-port is getting thornier.

[Jane Sandberg (sandbergja)]

Thanks for the refactor, Bill.

I took a stab at adding the Shift+F10 functionality here: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/sandbergja/lp1803787-ang-grid-context-menu

The branch is called user/sandbergja/lp1803787-ang-grid-context-menu

The problem with my branch is that when you press Enter to select an Action from the context menu, it also triggers the onRowActivate event, which causes problems. So, in the majority of the admin interfaces, tabbing to "Edit Selected Rows" and pressing enter causes the edit modal to attempt to open twice, causing problems. Something fun to play around with, though!

Testing notes from my commit:
1) Open an eg2 grid interface (Server Administration -> Authority
Thesaurus is a good one).
2) Use the tab key to set focus onto the checkbox for one of the rows.
3) Note that pressing Shift + F10 opens the browser's context menu.
4) Apply this commit
5) Repeat steps 1+2
6) Press Shift + F10.
7) Note that the context menu opens, and that you can use Tab and
Shift+Tab to move through the various actions.
8) Note that you can press the Esc key to exit the context menu

I was also thinking that it would be nice for the sandbox to include an example of the action separators, but that doesn't seem essential to me.

If 3.2 backporting would be very difficult, I think that it is okay not to backport.

[Bill Erickson (berick)]

Thanks, Jane.

I have rebased my working branch to master and cherry-picked / signed-off on your shift+f10 branch.

Regarding the errant onRowActivate() calls, I added a tabindex to the actions menu buttons, which seems to be enough to convince the browser to focus the buttons on tab and not propagate the Enter to the grid itself.

I also pushed another commit to migrate some grid handler functions to click handlers instead of anonymous function handlers (functionality that was gained via the master rebase) to quiet some console deprecation messages.

Also some lint repairs.

[Bill Erickson (berick)]

Setting 3.2 target to WONTFIX since we're getting steadily further away from backport-ability.

[Jane Sandberg (sandbergja)]

Looks very good to me, Bill. I pushed a small follow-up commit here, as well as sign offs to all your recent commits: user/sandbergja/lp1803787-ang-grid-context-menu-follow-up

[Bill Erickson (berick)]

Thanks, Jane.

I've signed off on your latest commit, rebased to master, and pushed back to my working branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1803787-ang-grid-context-menu

I stopped short of merging, though, because there are still a few commits of mine in the branch that still need a sign off.

[Jane Sandberg (sandbergja)]

I added those missing signoffs, and pushed to master and rel_3_3. Thanks so much for your work on this, Bill. And thanks, Andrea, for the original report and John, for the testing.