Skip to content

[DREAM-974] Show the table row menu as a Primer ActionMenu #26571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
myabc wants to merge 2 commits into from
+674 −81
Draft

[DREAM-974] Show the table row menu as a Primer ActionMenu #26571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40fcb9a
[DREAM-974] Show the table row menu as a Primer ActionMenu
myabc Nov 3, 2026
fa233be
[DREAM-974] Nest the move actions under a Move submenu
myabc Nov 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 31 additions & 35 deletions ...s/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-entry.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ import { WorkPackageResource } from 'core-app/features/hal/resources/work-packag
import { WorkPackageViewOrderService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-order.service';
import { WorkPackageViewHierarchiesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-hierarchy.service';
import { WorkPackageContextMenuHelperService } from 'core-app/features/work-packages/components/wp-table/context-menu-helper/wp-context-menu-helper.service';
import { OPContextMenuService } from 'core-app/shared/components/op-context-menu/op-context-menu.service';
import { OpActionMenuService } from 'core-app/shared/components/op-context-menu/action-menu/op-action-menu.service';
import { WorkPackageTableContextMenu } from 'core-app/shared/components/op-context-menu/wp-context-menu/wp-table-context-menu.directive';
import { registerContextMenu } from 'core-app/shared/components/op-context-menu/testing/register-context-menu';
import { buildTable, TableHarness, type TableHarnessOptions } from '../../testing/table-harness';
import { wpOccurrenceKey } from '../../rendered-occurrence-ledger';
import { headerRowKey, totalsRowKey } from '../../row-keys';
Expand All @@ -61,7 +60,7 @@ describe('Context menu entry', () => {
menuTargets.push(workPackages.map((wp) => wp.id!));
return [];
});
opened = vi.spyOn(harness.injector.get(OPContextMenuService), 'show');
opened = vi.spyOn(harness.injector.get(OpActionMenuService), 'show').mockImplementation(() => undefined);
};

beforeEach(() => mount());
Expand Down Expand Up @@ -137,12 +136,12 @@ describe('Context menu entry', () => {
});

describe('Move actions', () => {
interface MenuItem { linkText?:string; isHeader?:boolean; disabled?:boolean; onClick?:() => boolean }
interface MenuItem { linkText?:string; disabled?:boolean; children?:MenuItem[]; onClick?:() => boolean }

const menuItems = ():MenuItem[] => (opened.mock.calls.at(-1)![0] as unknown as { locals:{ items:MenuItem[] } }).locals.items;
const moveLabels = ['Move to top', 'Move up', 'Move down', 'Move to bottom'];
const moveItems = () => menuItems().filter((item) => !item.isHeader && moveLabels.includes(item.linkText ?? ''));
const hasMoveHeader = () => menuItems().some((item) => item.isHeader && item.linkText === 'Move');
const moveMenu = () => menuItems().find((item) => item.children && item.linkText === 'Move');
const moveItems = () => moveMenu()?.children ?? [];
const hasMoveSubmenu = () => moveMenu() !== undefined;

const dragAndDropTable = { contextMenuEnabled: true, dragAndDropEnabled: true };

Expand All @@ -165,7 +164,7 @@ describe('Context menu entry', () => {
it('offers the move group for one work package', () => {
openFromKeyboard('2');

expect(hasMoveHeader()).toBe(true);
expect(hasMoveSubmenu()).toBe(true);
expect(moveItems().map((item) => item.linkText)).toEqual(['Move to top', 'Move up', 'Move down', 'Move to bottom']);
expect(moveItems().map((item) => item.disabled)).toEqual([false, false, false, false]);
});
Expand Down Expand Up @@ -198,7 +197,7 @@ describe('Context menu entry', () => {

openFromKeyboard('2');

expect(hasMoveHeader()).toBe(false);
expect(hasMoveSubmenu()).toBe(false);
expect(moveItems()).toEqual([]);
});

Expand All @@ -208,7 +207,7 @@ describe('Context menu entry', () => {

openFromKeyboard('2');

expect(hasMoveHeader()).toBe(false);
expect(hasMoveSubmenu()).toBe(false);
});

it('offers no move group when the table has drag and drop disabled', async () => {
Expand All @@ -217,7 +216,7 @@ describe('Context menu entry', () => {

openFromKeyboard('2');

expect(hasMoveHeader()).toBe(false);
expect(hasMoveSubmenu()).toBe(false);
expect(moveItems()).toEqual([]);
});

Expand Down Expand Up @@ -313,7 +312,7 @@ describe('Context menu entry from the actions column button', () => {
menuTargets.push(workPackages.map((wp) => wp.id!));
return [];
});
opened = vi.spyOn(harness.injector.get(OPContextMenuService), 'show');
opened = vi.spyOn(harness.injector.get(OpActionMenuService), 'show').mockImplementation(() => undefined);
});

afterEach(() => harness.destroy());
Expand Down Expand Up @@ -366,7 +365,7 @@ describe('Context menu entry on the inline-create row', () => {
inlineCreateRow = harness.addInlineCreateRow();
harness.click('2');

opened = vi.spyOn(harness.injector.get(OPContextMenuService), 'show');
opened = vi.spyOn(harness.injector.get(OpActionMenuService), 'show').mockImplementation(() => undefined);
});

afterEach(() => harness.destroy());
Expand Down Expand Up @@ -439,7 +438,7 @@ describe('Context menu keyboard chords', () => {
menuTargets.push(workPackages.map((wp) => wp.id!));
return [];
});
opened = vi.spyOn(harness.injector.get(OPContextMenuService), 'show');
opened = vi.spyOn(harness.injector.get(OpActionMenuService), 'show').mockImplementation(() => undefined);
};

const focusKey = (key:string) => harness.table.keyboardNavigation
Expand Down Expand Up @@ -714,10 +713,12 @@ describe('Context menu keyboard chords', () => {
});
});

const MENU = 'op-action-menu';
const MENU_ITEM = `${MENU} [role="menuitem"]`;

describe('Context menu focus return', () => {
let harness:TableHarness;
let menu:OPContextMenuService;
let unregister:() => void;
let menu:OpActionMenuService;
let action:() => void;
let page:HTMLElement;

Expand All @@ -728,7 +729,6 @@ describe('Context menu focus return', () => {
workPackages: [{ id: '1' }, { id: '2' }, { id: '3' }],
columns: ['id', 'subject', 'status'],
productionMarkup: true,
providers: [OPContextMenuService],
...options,
configuration: { contextMenuEnabled: true, ...options.configuration },
});
Expand All @@ -738,8 +738,7 @@ describe('Context menu focus return', () => {
page.id = 'wrapper';
page.textContent = 'Blank page';
document.body.append(page);
menu = harness.injector.get(OPContextMenuService);
unregister = registerContextMenu(menu);
menu = harness.injector.get(OpActionMenuService);
action = () => undefined;
vi.spyOn(harness.injector.get(WorkPackageContextMenuHelperService), 'getPermittedActions')
.mockReturnValue([{ key: 'test-action', text: 'Test action', link: '#' }]);
Expand All @@ -754,7 +753,7 @@ describe('Context menu focus return', () => {
target.focus();
fireEvent.keyDown(target, { key: 'F10', shiftKey: true });
return waitFor(() => {
const item = document.querySelector<HTMLElement>('.op-context-menu--overlay .menu-item')!;
const item = document.querySelector<HTMLElement>(MENU_ITEM)!;
expect(item).toHaveFocus();
return item;
});
Expand All @@ -766,7 +765,7 @@ describe('Context menu focus return', () => {
};

const clickAction = async () => {
const item = await waitFor(() => within(document.querySelector<HTMLElement>('.op-context-menu--overlay')!)
const item = await waitFor(() => within(document.querySelector<HTMLElement>(MENU)!)
.getByRole('menuitem', { name: 'Test action' }));
fireEvent.click(item);
};
Expand All @@ -776,7 +775,6 @@ describe('Context menu focus return', () => {
await nextFrame();
await nextFrame();
menu.close();
unregister();
page.remove();
vi.restoreAllMocks();
await harness.destroy();
Expand All @@ -801,7 +799,7 @@ describe('Context menu focus return', () => {
expect(link).toHaveFocus();

await userEvent.keyboard('{Enter}');
await waitFor(() => expect(document.querySelector('.op-context-menu--overlay .menu-item')).toHaveFocus());
await waitFor(() => expect(document.querySelector(MENU_ITEM)).toHaveFocus());
await userEvent.keyboard('{Escape}');

expect(menu.active).toBeNull();
Expand Down Expand Up @@ -851,30 +849,28 @@ describe('Context menu focus return', () => {
await nextFrame();
await nextFrame();
await nextFrame();
expect(document.querySelector('.op-context-menu--overlay .menu-item')).toBeNull();
expect(document.querySelector(MENU_ITEM)).toBeNull();
expect(cellOf('2', 1)).toHaveFocus();
expect(harness.selection.getSelectedWorkPackageIds()).toEqual(['2']);
});

it('a click outside closes a right-click menu without pulling focus into the table', async () => {
fireEvent.contextMenu(cellOf('2', 1));
await waitFor(() => expect(document.querySelector('.op-context-menu--overlay .menu-item')).toHaveFocus());
await waitFor(() => expect(document.querySelector(MENU_ITEM)).toHaveFocus());

fireEvent.click(page);
fireEvent.pointerDown(page);

expect(menu.active).toBeNull();
expect(document.body).toHaveFocus();
});

it('a scroll closes a keyboard-opened menu without moving focus or scrolling', async () => {
await openOnCell('2', 1);
const scrolled = vi.spyOn(Element.prototype, 'scrollIntoView');
it('a scroll keeps a keyboard-opened menu open with focus inside it', async () => {
const item = await openOnCell('2', 1);

fireEvent.scroll(page);

expect(menu.active).toBeNull();
expect(document.body).toHaveFocus();
expect(scrolled).not.toHaveBeenCalled();
expect(menu.active).not.toBeNull();
expect(item).toHaveFocus();
});

it('origin row removed by the action: focus lands on the reconciled neighbor', async () => {
Expand Down Expand Up @@ -943,9 +939,9 @@ describe('Context menu focus return', () => {
fireEvent.keyDown(cellOf('2', 1), chord);
fireEvent.keyDown(cellOf('2', 1), { ...chord, repeat: true });

await waitFor(() => expect(document.querySelector('.op-context-menu--overlay .menu-item')).toHaveFocus());
await waitFor(() => expect(document.querySelector(MENU_ITEM)).toHaveFocus());
expect(shown).toHaveBeenCalledTimes(1);
expect(document.querySelectorAll('.op-context-menu--overlay .dropdown')).toHaveLength(1);
expect(document.querySelectorAll('op-action-menu')).toHaveLength(1);
expect(errors).toEqual([]);
} finally {
window.removeEventListener('error', collect);
Expand Down Expand Up @@ -992,7 +988,7 @@ describe('Context menu focus return', () => {

it('still swallows the companion of a keyboard open that replaces an open menu', async () => {
fireEvent.contextMenu(cellOf('1', 1));
await waitFor(() => expect(document.querySelector('.op-context-menu--overlay .menu-item')).toHaveFocus());
await waitFor(() => expect(document.querySelector(MENU_ITEM)).toHaveFocus());

fireEvent.keyDown(harness.row('2'), { key: 'F10', shiftKey: true });

Expand Down
6 changes: 3 additions & 3 deletions ...ures/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import { Injector } from '@angular/core';
import { HAL_NEW_RESOURCE_ID } from 'core-app/features/hal/helpers/is-new-resource';
import { activeFieldContainerClassName } from 'core-app/shared/components/fields/edit/edit-form/edit-form';
import { OPContextMenuService } from 'core-app/shared/components/op-context-menu/op-context-menu.service';
import { OpActionMenuService } from 'core-app/shared/components/op-context-menu/action-menu/op-action-menu.service';
import { WorkPackageTableContextMenu } from 'core-app/shared/components/op-context-menu/wp-context-menu/wp-table-context-menu.directive';
import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator';
import { onDestroySafely } from 'core-app/shared/helpers/angular/owned-ui-cleanup';
Expand Down Expand Up @@ -56,7 +56,7 @@ export function forgetKeyboardMenuOpen(table:WorkPackageTable):void {

export abstract class ContextMenuHandler implements TableEventHandler {
// Injections
@LazyInject() public opContextMenu:OPContextMenuService;
@LazyInject() public actionMenu:OpActionMenuService;

@LazyInject() readonly selectionGestures:WorkPackageViewSelectionGesturesService;

Expand Down Expand Up @@ -119,6 +119,6 @@ export abstract class ContextMenuHandler implements TableEventHandler {
});

const handler = new WorkPackageTableContextMenu(this.injector, workPackageId, evt.target as HTMLElement, positionArgs, table);
this.opContextMenu.show(handler, evt);
this.actionMenu.show(handler, evt);
}
}
17 changes: 7 additions & 10 deletions .../components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ import { WorkPackageViewHierarchiesService } from 'core-app/features/work-packag
import { WorkPackageViewFocusService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-focus.service';
import { FocusHelperService } from 'core-app/shared/directives/focus/focus-helper';
import { WorkPackageContextMenuHelperService } from 'core-app/features/work-packages/components/wp-table/context-menu-helper/wp-context-menu-helper.service';
import { OPContextMenuService } from 'core-app/shared/components/op-context-menu/op-context-menu.service';
import { OpActionMenuService } from 'core-app/shared/components/op-context-menu/action-menu/op-action-menu.service';
import { OpContextMenuHandler } from 'core-app/shared/components/op-context-menu/op-context-menu-handler';
import { registerContextMenu } from 'core-app/shared/components/op-context-menu/testing/register-context-menu';
import { QueryColumn } from 'core-app/features/work-packages/components/wp-query/query-column';
import { WorkPackageTableConfiguration } from 'core-app/features/work-packages/components/wp-table/wp-table-configuration';
import { internalContextMenuColumn, internalSortColumn } from '../../../builders/internal-sort-columns';
Expand Down Expand Up @@ -1021,7 +1020,7 @@ describe('KeyboardNavigationTransformer', () => {
const table = await mount({ configuration: { contextMenuEnabled: true } });
vi.spyOn(table.injector.get(WorkPackageContextMenuHelperService), 'getPermittedActions').mockReturnValue([]);
let menu:OpContextMenuHandler|undefined;
vi.spyOn(table.injector.get(OPContextMenuService), 'show').mockImplementation((handler) => { menu = handler; });
vi.spyOn(table.injector.get(OpActionMenuService), 'show').mockImplementation((handler) => { menu = handler; });
const subjectCell = table.row('1').querySelector<HTMLTableCellElement>(':scope > .wp-table--cell-td.subject')!;
const field = subjectCell.querySelector<HTMLElement>('.inline-edit--display-field')!;

Expand Down Expand Up @@ -1573,7 +1572,7 @@ describe('KeyboardNavigationTransformer', () => {
])('opens the context menu on %s only for the BCF list', async (_chord, init) => {
const table = await mountConsumer();
vi.spyOn(table.injector.get(WorkPackageContextMenuHelperService), 'getPermittedActions').mockReturnValue([]);
const show = vi.spyOn(table.injector.get(OPContextMenuService), 'show').mockImplementation(() => undefined);
const show = vi.spyOn(table.injector.get(OpActionMenuService), 'show').mockImplementation(() => undefined);
table.row('1').focus();

fireEvent.keyDown(table.row('1'), init);
Expand All @@ -1594,7 +1593,7 @@ describe('KeyboardNavigationTransformer', () => {
describe('two consumer tables on one page', () => {
const mountPair = async (options:Partial<TableHarnessOptions> = {}) => {
const states = new States();
const first = await mount({ states, providers: [OPContextMenuService], configuration: consumerConfiguration('bcf'), ...options });
const first = await mount({ states, configuration: consumerConfiguration('bcf'), ...options });
const second = await mount({ states, configuration: consumerConfiguration('bcf'), ...options });
return [first, second] as const;
};
Expand Down Expand Up @@ -1633,18 +1632,17 @@ describe('KeyboardNavigationTransformer', () => {
const page = document.createElement('div');
page.id = 'wrapper';
document.body.append(page);
const unregister = registerContextMenu(first.injector.get(OPContextMenuService));
vi.spyOn(first.injector.get(WorkPackageContextMenuHelperService), 'getPermittedActions')
.mockReturnValue([{ key: 'test-action', text: 'Test action', link: '#' }]);
try {
first.click('1');
second.click('2');
first.row('1').focus();
fireEvent.keyDown(first.row('1'), { key: 'F10', shiftKey: true });
await waitFor(() => expect(document.querySelector('.op-context-menu--overlay .menu-item')).toHaveFocus());
await waitFor(() => expect(document.querySelector('op-action-menu [role="menuitem"]')).toHaveFocus());

fireEvent.keyDown(document.activeElement!, { key: 'Escape' });
await waitFor(() => expect(document.querySelector('.op-context-menu--overlay .menu-item')).toBeNull());
await waitFor(() => expect(document.querySelector('op-action-menu [role="menuitem"]')).toBeNull());

expect(selectedIds(first)).toEqual(['1']);
expect(selectedIds(second)).toEqual(['2']);
Expand All @@ -1654,8 +1652,7 @@ describe('KeyboardNavigationTransformer', () => {
expect(selectedIds(first)).toEqual([]);
expect(selectedIds(second)).toEqual([]);
} finally {
first.injector.get(OPContextMenuService).close();
unregister();
first.injector.get(OpActionMenuService).close();
page.remove();
}
});
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ import { WorkPackageViewSortByService } from 'core-app/features/work-packages/ro
import { WorkPackageViewTimelineService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-timeline.service';
import { HalResourceEditingService } from 'core-app/shared/components/fields/edit/services/hal-resource-editing.service';
import { OPContextMenuService } from 'core-app/shared/components/op-context-menu/op-context-menu.service';
import { OpActionMenuService } from 'core-app/shared/components/op-context-menu/action-menu/op-action-menu.service';
import { OpTableActionsService } from 'core-app/features/work-packages/components/wp-table/table-actions/table-actions.service';
import { OpContextMenuTableAction } from 'core-app/features/work-packages/components/wp-table/table-actions/actions/context-menu-table-action';
import { FocusHelperService } from 'core-app/shared/directives/focus/focus-helper';
Expand Down Expand Up @@ -690,6 +691,7 @@ export function harnessProviders(dragService:FakeDragAndDropService, options:Tab
{ provide: WorkPackageRelationsService, useValue: relationsStub(options.relations ?? []) },
{ provide: WorkPackageContextMenuHelperService, useValue: { getPermittedActions: () => [] } },
{ provide: OPContextMenuService, useValue: { close: () => undefined, show: () => undefined } },
{ provide: OpActionMenuService, useClass: OpActionMenuService },
{
provide: ConfigurationService,
useValue: { requireDoubleClickForInlineEdit: () => !!options.requireDoubleClickForInlineEdit },
Expand Down
Loading
Loading