From 64b5a70471d048d65f3bf3490acd8d2efe76ba07 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 19:42:39 +0100 Subject: [PATCH 1/8] [DREAM-894] Extract selection announcement engine Extracts the orchestrator's announcement policy into a shared SelectionAnnouncements class in core-common, so other selection consumers can reuse the gesture rules and the render baseline. Moves the navigation/selection speak rules and the baseline tracking out of renderSelection. The orchestrator now delegates to the engine and drops its own selectionMessage helper. Its spec is unchanged. https://community.openproject.org/wp/DREAM-894 --- .../common/selection-announcements.spec.ts | 126 ++++++++++++++++++ .../src/common/selection-announcements.ts | 69 ++++++++++ .../sortable-lists/selection-orchestrator.ts | 46 ++----- 3 files changed, 208 insertions(+), 33 deletions(-) create mode 100644 frontend/src/common/selection-announcements.spec.ts create mode 100644 frontend/src/common/selection-announcements.ts diff --git a/frontend/src/common/selection-announcements.spec.ts b/frontend/src/common/selection-announcements.spec.ts new file mode 100644 index 000000000000..d34118e6cfc3 --- /dev/null +++ b/frontend/src/common/selection-announcements.spec.ts @@ -0,0 +1,126 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { LiveRegionElement } from '@primer/live-region-element'; +import { type MockInstance } from 'vitest'; +import { SelectionAnnouncements } from './selection-announcements'; + +describe('SelectionAnnouncements', () => { + let announceSpy:MockInstance; + let announcements:SelectionAnnouncements; + + const keys = (...values:string[]) => new Set(values); + + beforeEach(() => { + document.body.append(document.createElement('live-region')); + announceSpy = vi.spyOn(LiveRegionElement.prototype, 'announce'); + window.I18n.store({ + en: { + test: { + selection: { + selected: { one: '1 item selected.', other: '%{count} items selected.' }, + cleared: 'Selection cleared.', + all_selected: { other: 'All %{count} items selected.' }, + }, + }, + }, + }); + announcements = new SelectionAnnouncements(() => 'test.selection'); + }); + + afterEach(() => { + document.querySelector('live-region')?.remove(); + announceSpy.mockRestore(); + }); + + it('selection gesture speaks on a size change', () => { + expect(announcements.commit('selection', keys('a', 'b'))).toBe(true); + + expect(announceSpy).toHaveBeenCalledTimes(1); + expect(announceSpy).toHaveBeenCalledWith('2 items selected.', { politeness: 'polite' }); + }); + + it('selection gesture speaks on a same-count replacement', () => { + announcements.rebase(keys('a')); + + announcements.commit('selection', keys('b')); + + expect(announceSpy).toHaveBeenCalledTimes(1); + }); + + it('selection gesture stays silent when membership is unchanged', () => { + announcements.rebase(keys('a', 'b')); + + expect(announcements.commit('selection', keys('a', 'b'))).toBe(false); + + expect(announceSpy).not.toHaveBeenCalled(); + }); + + it('navigation gesture speaks only when a batch collapsed', () => { + announcements.rebase(keys('a', 'b')); + announcements.commit('navigation', keys('a')); + expect(announceSpy).toHaveBeenCalledTimes(1); + + announcements.commit('navigation', keys('b')); + expect(announceSpy).toHaveBeenCalledTimes(1); + + announcements.rebase(keys()); + announcements.commit('navigation', keys('a')); + expect(announceSpy).toHaveBeenCalledTimes(1); + }); + + it('speaks cleared when the selection empties', () => { + announcements.rebase(keys('a', 'b')); + + announcements.commit('selection', keys()); + + expect(announceSpy).toHaveBeenCalledWith('Selection cleared.', { politeness: 'polite' }); + }); + + it('rebase is silent and moves the baseline', () => { + announcements.rebase(keys('a')); + + announcements.commit('selection', keys('a')); + + expect(announceSpy).not.toHaveBeenCalled(); + expect(announcements.baseline).toEqual(keys('a')); + }); + + it('commit against an explicit previous ignores the baseline', () => { + announcements.rebase(keys('a')); + + announcements.commit('selection', keys('a'), keys()); + + expect(announceSpy).toHaveBeenCalledTimes(1); + }); + + it('speak always announces with the given count', () => { + announcements.speak('all_selected', 7); + + expect(announceSpy).toHaveBeenCalledWith('All 7 items selected.', { politeness: 'polite' }); + }); +}); diff --git a/frontend/src/common/selection-announcements.ts b/frontend/src/common/selection-announcements.ts new file mode 100644 index 000000000000..6541f4872d20 --- /dev/null +++ b/frontend/src/common/selection-announcements.ts @@ -0,0 +1,69 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { announce } from '@primer/live-region-element'; +import type { SelectionKey } from 'core-common/batch-selection'; + +export type SelectionGesture = 'navigation'|'selection'; + +export class SelectionAnnouncements { + private lastKeys:ReadonlySet = new Set(); + + constructor(private readonly scope:() => string) {} + + get baseline():ReadonlySet { + return this.lastKeys; + } + + rebase(keys:ReadonlySet):void { + this.lastKeys = keys; + } + + /** + * Re-bases to `current` and announces `selected`/`cleared` with `count: current.size` + * when the gesture rule says so: `navigation` speaks only when it collapsed a batch, + * `selection` on any membership change. Returns whether it spoke. + */ + commit(gesture:SelectionGesture, current:ReadonlySet, previous:ReadonlySet = this.lastKeys):boolean { + this.lastKeys = current; + + const changed = gesture === 'navigation' + ? previous.size > 1 && current.size !== previous.size + : current.size !== previous.size || [...current].some((key) => !previous.has(key)); + + if (changed) { + this.speak(current.size === 0 ? 'cleared' : 'selected', current.size); + } + + return changed; + } + + /** Always announces `${scope}.${messageKey}` with `{ count }`. */ + speak(messageKey:string, count:number):void { + void announce(I18n.t(`${this.scope()}.${messageKey}`, { count }), { politeness: 'polite' }); + } +} diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts index 0d00a0ff6ca5..7e2b81720076 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts @@ -25,8 +25,7 @@ // See COPYRIGHT and LICENSE files for more details. //++ -import { BatchSelection, type SelectionAnchor, type SelectionKey } from 'core-common/batch-selection'; -import { announce } from '@primer/live-region-element'; +import { BatchSelection, type SelectionAnchor } from 'core-common/batch-selection'; import { resolveItemId, resolveItemType } from './list-dom'; import { applySelectionPresentation, @@ -42,6 +41,7 @@ import { } from './selection'; import { closestInteractiveElement } from 'core-common/interactive-element-helper'; import { isApplePlatform } from 'core-common/platform'; +import { SelectionAnnouncements, type SelectionGesture } from 'core-common/selection-announcements'; import { clearSelectionOnEscape } from 'core-common/selection-escape'; import { isSelectAllShortcut } from 'core-common/selection-shortcuts'; @@ -75,9 +75,7 @@ type ScopeMutation = 'none'|'replace-if-unselected'|'replace'; export class SelectionOrchestrator { private readonly selection = new BatchSelection(); - // Last painted membership, not last announced: a silent navigation render - // in between would leave a stale baseline and make the next no-op speak. - private lastRenderedKeys:ReadonlySet = new Set(); + private readonly announcements = new SelectionAnnouncements(() => this.host.announcementScope); constructor(private readonly host:SelectionHost) {} @@ -140,7 +138,7 @@ export class SelectionOrchestrator { this.selection.clear(); this.syncSelectionPresentation(); - this.lastRenderedKeys = this.selection.keys; + this.announcements.rebase(this.selection.keys); } // The platform's one multi-select modifier: ⌘ on Apple platforms, Ctrl @@ -436,47 +434,29 @@ export class SelectionOrchestrator { private renderRangeRestart(candidate:SelectionCandidate):void { this.selection.replace({ type: candidate.type, id: candidate.id }, candidate.listKey); this.syncSelectionPresentation(); - this.lastRenderedKeys = this.selection.keys; + this.announcements.rebase(this.selection.keys); this.announceSelection('range_restarted'); } - // Announcement follows the gesture class, not the count: `navigation` (a - // plain click) speaks only when it collapsed a batch, since the details - // pane it opens is its own feedback for the card itself; `selection` - // speaks on any membership change. - private renderSelection(kind:'navigation'|'selection', fullRepair = false):void { - const previous = this.lastRenderedKeys; + private renderSelection(kind:SelectionGesture, fullRepair = false):void { + const previous = this.announcements.baseline; this.syncSelectionPresentation(fullRepair); - - const current = this.selection.keys; - this.lastRenderedKeys = current; - - const changed = kind === 'navigation' - ? previous.size > 1 && current.size !== previous.size - : current.size !== previous.size || [...current].some((id) => !previous.has(id)); - - if (changed) { - this.announceSelection(current.size === 0 ? 'cleared' : 'selected'); - } + this.announcements.commit(kind, this.selection.keys, previous); } private syncSelectionPresentation(fullRepair = false):void { const current = this.selection.keys; + const { baseline } = this.announcements; const changedKeys = fullRepair ? undefined : new Set([ - ...[...this.lastRenderedKeys].filter((key) => !current.has(key)), - ...[...current].filter((key) => !this.lastRenderedKeys.has(key)), + ...[...baseline].filter((key) => !current.has(key)), + ...[...current].filter((key) => !baseline.has(key)), ]); applySelectionPresentation(this.host.rootElement, current, this.host.descriptionId, changedKeys); } private announceSelection(key:'selected'|'cleared'|'not_selectable'|'range_unavailable'|'range_blocked'|'range_restarted'):void { - void announce(this.selectionMessage(key), { politeness: 'polite' }); - } - - // The consumer's vocabulary: Backlogs says "work package", not "item". - private selectionMessage(key:string):string { - return I18n.t(`${this.host.announcementScope}.${key}`, { count: this.selection.size }); + this.announcements.speak(key, this.selection.size); } // Repaints whether or not prune dropped anything: a morph can strip or @@ -514,7 +494,7 @@ export class SelectionOrchestrator { teardown():void { this.selection.clear(); - this.lastRenderedKeys = new Set(); + this.announcements.rebase(new Set()); this.clearPresentation(); } } From a2003e2723a756fd9c63b77cda8afa74d21851eb Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 19:48:45 +0100 Subject: [PATCH 2/8] [DREAM-894] Leave Ctrl/Cmd+Shift+A unhandled Ctrl/Cmd+Shift+A is not Select All. The shared predicate isSelectAllShortcut now returns false when Shift is held, so the shortcut falls through to the browser instead of selecting every row (OP-20202). This applies to every caller of the shared predicate: the work package table, the card view, and Backlogs. https://community.openproject.org/wp/DREAM-894 --- frontend/src/common/selection-shortcuts.spec.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frontend/src/common/selection-shortcuts.spec.ts b/frontend/src/common/selection-shortcuts.spec.ts index d4deaddf7b29..3b09d3b979b1 100644 --- a/frontend/src/common/selection-shortcuts.spec.ts +++ b/frontend/src/common/selection-shortcuts.spec.ts @@ -49,6 +49,12 @@ describe('isSelectAllShortcut', () => { }, ); + it('rejects Shift (Ctrl/Cmd+Shift+A is not select all)', () => { + expect(isSelectAllShortcut(new KeyboardEvent('keydown', { key: 'A', code: 'KeyA', ctrlKey: true, shiftKey: true }), false)).toBe(false); + expect(isSelectAllShortcut(new KeyboardEvent('keydown', { key: 'A', code: 'KeyA', metaKey: true, shiftKey: true }), true)).toBe(false); + expect(isSelectAllShortcut(new KeyboardEvent('keydown', { key: 'Ф', code: 'KeyA', ctrlKey: true, shiftKey: true }), false)).toBe(false); + }); + it('rejects AltGraph', () => { const event = new KeyboardEvent('keydown', { key: 'a', ctrlKey: true }); vi.spyOn(event, 'getModifierState').mockImplementation((key) => key === 'AltGraph'); From 97c3a47cc7a5f1541a02c512b9f02cc9dfa91bc9 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 19:51:13 +0100 Subject: [PATCH 3/8] [DREAM-894] Allow Select All without an anchor Adds a null anchor mode to selectAll that selects every selectable occurrence and clears any existing anchor, so a following Shift gesture falls back to a plain replace. Omitting the argument keeps the first-selectable fallback. Adds a selectionKeys getter for callers that compare membership across a gesture. https://community.openproject.org/wp/DREAM-894 --- .../wp-view-selection.service.spec.ts | 27 +++++++++++++++++++ .../wp-view-selection.service.ts | 17 +++++++----- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/frontend/src/app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service.spec.ts b/frontend/src/app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service.spec.ts index 9437e1068413..4870572df804 100644 --- a/frontend/src/app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service.spec.ts +++ b/frontend/src/app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service.spec.ts @@ -147,6 +147,33 @@ describe('WorkPackageViewSelectionService', () => { expect(selection.getSelectedWorkPackageIds()).toEqual(['1', '2']); }); + it('selectAll with a null anchor selects every selectable occurrence and leaves no anchor', () => { + selection.selectAll(rows, null); + expect(selection.getSelectedWorkPackageIds()).toEqual(['1', '2', '3', '4']); + selection.rangeTo(rows[2], rows); + expect(selection.getSelectedWorkPackageIds()).toEqual(['3']); + }); + + it('selectAll with a null anchor clears an existing anchor', () => { + selection.replaceOccurrence(rows[0]); + selection.selectAll(rows, null); + selection.rangeTo(rows[1], rows); + expect(selection.getSelectedWorkPackageIds()).toEqual(['2']); + }); + + it('selectAll with no anchor argument still falls back to the first selectable', () => { + selection.selectAll(rows); + selection.rangeTo(rows[1], rows); + expect(selection.getSelectedWorkPackageIds()).toEqual(['1', '2']); + }); + + it('selectAll with a null anchor on a page without selectable rows does nothing', () => { + const header:RenderedWorkPackage = { workPackageId: null, classIdentifier: 'group', hidden: false }; + selection.replaceOccurrence(rows[0]); + selection.selectAll([header], null); + expect(selection.getSelectedWorkPackageIds()).toEqual(['1']); + }); + it('ignores group headers and missing gesture occurrences', () => { const header:RenderedWorkPackage = { workPackageId: null, classIdentifier: 'group', hidden: false }; selection.replaceOccurrence(rows[1]); diff --git a/frontend/src/app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service.ts b/frontend/src/app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service.ts index b4195cab7847..48a05af64cd5 100644 --- a/frontend/src/app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service.ts +++ b/frontend/src/app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service.ts @@ -34,7 +34,7 @@ import { WorkPackageViewBaseService } from 'core-app/features/work-packages/rout import { QueryResource } from 'core-app/features/hal/resources/query-resource'; import { WorkPackageCollectionResource } from 'core-app/features/hal/resources/wp-collection-resource'; -import { BatchSelection, SelectionItem, selectionKey } from 'core-common/batch-selection'; +import { BatchSelection, SelectionItem, SelectionKey, selectionKey } from 'core-common/batch-selection'; import { anchoredOccurrence, occurrenceRangeIds, @@ -87,6 +87,7 @@ export class WorkPackageViewSelectionService extends WorkPackageViewBaseService< } public isSelected(id:string):boolean { return this.model.has(this.item(id)); } + public get selectionKeys():ReadonlySet { return this.model.keys; } public get selectionCount():number { return this.model.size; } public get isEmpty():boolean { return this.model.size === 0; } public get hasSelectionState():boolean { return this.model.size > 0 || this.model.anchor !== null; } @@ -160,13 +161,15 @@ export class WorkPackageViewSelectionService extends WorkPackageViewBaseService< this.publish(); } - public selectAll(rows:RenderedWorkPackage[], requestedAnchor?:RenderedWorkPackage):void { + /** `requestedAnchor === null`: no anchor (any existing one is cleared). `undefined`: legacy first-selectable fallback. */ + public selectAll(rows:RenderedWorkPackage[], requestedAnchor?:RenderedWorkPackage|null):void { const selectable = selectableOccurrences(rows); - const anchor = selectAllAnchor(selectable, requestedAnchor); - if (!anchor) return; - this.model.selectAll(selectable.map((candidate) => this.item(candidate.workPackageId!)), { - ...this.item(anchor.workPackageId!), listKey: 'view', occurrenceKey: anchor.classIdentifier, - }); + if (selectable.length === 0) return; + const anchor = requestedAnchor === null ? null : selectAllAnchor(selectable, requestedAnchor)!; + this.model.selectAll( + selectable.map((candidate) => this.item(candidate.workPackageId!)), + anchor && { ...this.item(anchor.workPackageId!), listKey: 'view', occurrenceKey: anchor.classIdentifier }, + ); this.publish(); this.opContextMenu.close(); } From 599a08c2081395f9a7256ba24667a108ab9c1371 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 19:55:40 +0100 Subject: [PATCH 4/8] [DREAM-894] Announce table selection changes Adds TableSelectionFeedback, created by the selection transformer only for tables with keyboard navigation enabled. Gesture sites (row click, context menu, drag collapse and Escape) run their mutation through selectionGesture, which announces via the shared SelectionAnnouncements engine. Capability-off tables run the mutation unchanged. The feedback records the membership each gesture commits as owned by the table; any other publication with a different membership drops that ownership. Pruning after a render announces only while the membership is still gesture-owned, so route, reload and programmatic selections are pruned silently. Adds the js.work_packages.selection locale keys. The card view is covered by a zero-announce test in its click handler spec; the shared selection services stay unhooked. https://community.openproject.org/wp/DREAM-894 --- config/locales/js-en.yml | 8 + .../event-handler/click-handler.spec.ts | 38 ++- .../context-menu/context-menu-handler.ts | 5 +- .../handlers/row/click-handler.ts | 11 +- .../handlers/row/wp-state-links-handler.ts | 5 +- .../state/drag-and-drop-transformer.ts | 5 +- .../handlers/state/selection-feedback.spec.ts | 277 ++++++++++++++++++ .../handlers/state/selection-feedback.ts | 88 ++++++ .../handlers/state/selection-transformer.ts | 14 +- .../components/wp-fast-table/wp-fast-table.ts | 3 + 10 files changed, 445 insertions(+), 9 deletions(-) create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-feedback.spec.ts create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-feedback.ts diff --git a/config/locales/js-en.yml b/config/locales/js-en.yml index c71c96f8c981..8c7c163eb415 100644 --- a/config/locales/js-en.yml +++ b/config/locales/js-en.yml @@ -1158,6 +1158,14 @@ en: scheduling: is_parent: "The dates of this work package are automatically deduced from its children. Activate 'Manual scheduling' to set the dates." is_switched_from_manual_to_automatic: "The dates of this work package may need to be recalculated after switching from manual to automatic scheduling due to relationships with other work packages." + selection: + all_selected: + one: "All %{count} work package on this page selected." + other: "All %{count} work packages on this page selected." + cleared: "Selection cleared." + selected: + one: "%{count} work package selected." + other: "%{count} work packages selected." sharing: show_all_users: "Show all users with whom the work package has been shared with" title: "Share work package" diff --git a/frontend/src/app/features/work-packages/components/wp-card-view/event-handler/click-handler.spec.ts b/frontend/src/app/features/work-packages/components/wp-card-view/event-handler/click-handler.spec.ts index 535a2a0b51f6..8d9825d53079 100644 --- a/frontend/src/app/features/work-packages/components/wp-card-view/event-handler/click-handler.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-card-view/event-handler/click-handler.spec.ts @@ -29,10 +29,12 @@ import { usePlatform } from 'core-common/testing/platform'; import { ElementRef, EventEmitter, Injector } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { fireEvent } from '@testing-library/dom'; +import { LiveRegionElement } from '@primer/live-region-element'; +import { type MockInstance } from 'vitest'; import { States } from 'core-app/core/states/states.service'; import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource'; import { IsolatedQuerySpace } from 'core-app/features/work-packages/directives/query-space/isolated-query-space'; -import { registerWorkPackageSelectAll } from 'core-app/features/work-packages/routing/wp-view-base/event-handling/wp-selection-keyboard'; +import { registerWorkPackageDeselectAll, registerWorkPackageSelectAll } from 'core-app/features/work-packages/routing/wp-view-base/event-handling/wp-selection-keyboard'; import { WorkPackageViewFocusService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-focus.service'; import { WorkPackageViewSelectionGesturesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection-gestures.service'; import { WorkPackageViewSelectionService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service'; @@ -122,6 +124,40 @@ describe('CardClickHandler', () => { expect(itemClicked.map((event) => event.workPackageId)).toEqual(['A', 'C', 'E', 'C']); }); + describe('with a live region on the page', () => { + let announce:MockInstance; + let unregisterDeselectAll:() => void; + + beforeEach(() => { + document.body.append(document.createElement('live-region')); + announce = vi.spyOn(LiveRegionElement.prototype, 'announce'); + unregisterDeselectAll = registerWorkPackageDeselectAll({ + root, + hasState: () => selection.hasSelectionState, + clear: () => selection.reset(), + }); + }); + + afterEach(() => { + unregisterDeselectAll(); + announce.mockRestore(); + document.querySelector('live-region')?.remove(); + }); + + it('never announces selection changes', () => { + click('A'); + click('C', { ctrlKey: true }); + click('E', { shiftKey: true }); + fireEvent.keyDown(cardSurface('E'), { key: 'a', ctrlKey: true }); + expect(selected()).toEqual(['A', 'B', 'C', 'D', 'E']); + + fireEvent.keyDown(cardSurface('A'), { key: 'Escape' }); + + expect(selected()).toEqual([]); + expect(announce).not.toHaveBeenCalled(); + }); + }); + it('keeps the final member deselected after its focus update', () => { click('C'); click('C', { ctrlKey: true }); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts index f6585dd6a4b3..4c1447dae486 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts @@ -33,6 +33,7 @@ import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorato import { tableRowClassName } from '../../builders/rows/single-row-builder'; import { WorkPackageTable } from '../../wp-fast-table'; import { TableEventComponent, TableEventHandler } from '../table-handler-registry'; +import { selectionGesture } from '../state/selection-feedback'; import { PositionArgs } from 'core-app/shared/components/op-context-menu/wp-context-menu/wp-view-context-menu.directive'; import { EventType } from 'core-app/features/work-packages/routing/wp-view-base/event-handling/event-handler-registry'; import { WorkPackageViewSelectionGesturesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection-gestures.service'; @@ -66,7 +67,9 @@ export abstract class ContextMenuHandler implements TableEventHandler { protected openContextMenu(table:WorkPackageTable, evt:Event, row:HTMLElement, positionArgs:PositionArgs = {}):void { const workPackageId = row.dataset.workPackageId!; - this.selectionGestures.handleContextMenu(workPackageId, table.renderedRows, row.dataset.classIdentifier); + selectionGesture(table, 'navigation', () => { + this.selectionGestures.handleContextMenu(workPackageId, table.renderedRows, row.dataset.classIdentifier); + }); const handler = new WorkPackageTableContextMenu(this.injector, workPackageId, evt.target as HTMLElement, positionArgs, table); this.opContextMenu.show(handler, evt); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/row/click-handler.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/row/click-handler.ts index 0f90ea7f683f..8347c357ba6d 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/row/click-handler.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/row/click-handler.ts @@ -37,6 +37,7 @@ import { activeFieldClassName } from 'core-app/shared/components/fields/edit/edi import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; import { debugLog } from 'core-app/shared/helpers/debug_output'; import { TableEventComponent, TableEventHandler } from '../table-handler-registry'; +import { selectionGesture } from '../state/selection-feedback'; import { tableRowClassName } from '../../builders/rows/single-row-builder'; import { KeepTabService } from '../../../wp-single-view-tabs/keep-tab/keep-tab.service'; import { EventType } from 'core-app/features/work-packages/routing/wp-view-base/event-handling/event-handler-registry'; @@ -112,10 +113,14 @@ export function activateRow( ):void { const { injector } = view.workPackageTable; - injector.get(WorkPackageViewSelectionGesturesService) - .handleClick(wpId, view.workPackageTable.renderedRows, modifiers, element.dataset.classIdentifier); + const modified = [modifiers.ctrlKey, modifiers.metaKey, modifiers.shiftKey].some(Boolean); - if (!(modifiers.ctrlKey || modifiers.metaKey || modifiers.shiftKey)) { + selectionGesture(view.workPackageTable, modified ? 'selection' : 'navigation', () => { + injector.get(WorkPackageViewSelectionGesturesService) + .handleClick(wpId, view.workPackageTable.renderedRows, modifiers, element.dataset.classIdentifier); + }); + + if (!modified) { view.itemClicked.emit({ workPackageId: wpId, double: false }); } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/row/wp-state-links-handler.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/row/wp-state-links-handler.ts index 29ad62d4bfc1..b850c768dbd3 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/row/wp-state-links-handler.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/row/wp-state-links-handler.ts @@ -35,6 +35,7 @@ import { KeepTabService } from '../../../wp-single-view-tabs/keep-tab/keep-tab.s import { tableRowClassName } from '../../builders/rows/single-row-builder'; import { uiStateLinkClass } from '../../builders/ui-state-link-builder'; import { TableEventComponent, TableEventHandler } from '../table-handler-registry'; +import { selectionGesture } from '../state/selection-feedback'; import { EventType } from 'core-app/features/work-packages/routing/wp-view-base/event-handling/event-handler-registry'; export class WorkPackageStateLinksHandler implements TableEventHandler { @@ -94,7 +95,9 @@ export class WorkPackageStateLinksHandler implements TableEventHandler { if (!row) { return true; } this.wpTableFocus.updateFocus(workPackageId, false, false); - this.selectionGestures.replace(workPackageId, view.workPackageTable.renderedRows, row.dataset.classIdentifier); + selectionGesture(view.workPackageTable, 'navigation', () => { + this.selectionGestures.replace(workPackageId, view.workPackageTable.renderedRows, row.dataset.classIdentifier); + }); view.stateLinkClicked.emit({ workPackageId, requestedState: state }); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/drag-and-drop-transformer.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/drag-and-drop-transformer.ts index 69748a9c9a93..48735677b82f 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/drag-and-drop-transformer.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/drag-and-drop-transformer.ts @@ -45,6 +45,7 @@ import { isInsideCollapsedGroup, locateTableRow } from 'core-app/features/work-p import { collapsedGroupClass } from 'core-app/features/work-packages/components/wp-fast-table/helpers/wp-table-hierarchy-helpers'; import { reorderById, type Edge } from 'core-common/drag-and-drop/reorder'; import { WorkPackageTable } from '../../wp-fast-table'; +import { selectionGesture } from './selection-feedback'; import { firstValueFrom } from 'rxjs'; export class DragAndDropTransformer { @@ -145,7 +146,9 @@ export class DragAndDropTransformer { if (this.table.destroyed) return; const wpId = row.dataset.workPackageId; if (wpId) { - this.selectionGestures.collapseTo(wpId, this.table.renderedRows, row.dataset.classIdentifier); + selectionGesture(this.table, 'navigation', () => { + this.selectionGestures.collapseTo(wpId, this.table.renderedRows, row.dataset.classIdentifier); + }); } } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-feedback.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-feedback.spec.ts new file mode 100644 index 000000000000..b88adc30ea0d --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-feedback.spec.ts @@ -0,0 +1,277 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { fireEvent } from '@testing-library/dom'; +import { LiveRegionElement } from '@primer/live-region-element'; +import { type MockInstance } from 'vitest'; +import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource'; +import { WorkPackageContextMenuHelperService } from 'core-app/features/work-packages/components/wp-table/context-menu-helper/wp-context-menu-helper.service'; +import { OpTableActionsService } from 'core-app/features/work-packages/components/wp-table/table-actions/table-actions.service'; +import { OpDetailsTableAction } from 'core-app/features/work-packages/components/wp-table/table-actions/actions/details-table-action'; +import { buildTable, TableHarness } from '../../testing/table-harness'; + +describe('Table selection feedback', () => { + let harness:TableHarness; + let announceSpy:MockInstance; + + const detailsActions = () => { + const actions = new OpTableActionsService(); + actions.setActions((injector, workPackage) => new OpDetailsTableAction(injector, workPackage)); + return actions; + }; + + const mount = async (keyboardNavigationEnabled:boolean, actionsColumnEnabled = false) => { + harness = buildTable({ + configuration: { keyboardNavigationEnabled, contextMenuEnabled: true, actionsColumnEnabled }, + productionMarkup: true, + workPackages: [{ id: '1' }, { id: '2' }, { id: '3' }, { id: '4' }], + providers: actionsColumnEnabled ? [{ provide: OpTableActionsService, useFactory: detailsActions }] : [], + }); + await harness.render(); + vi.spyOn(harness.injector.get(WorkPackageContextMenuHelperService), 'getPermittedActions') + .mockImplementation((_workPackages:WorkPackageResource[]) => []); + }; + + const spoken = () => announceSpy.mock.calls.map(([message]) => message); + const openDetails = (workPackageId:string) => fireEvent.click( + harness.row(workPackageId).querySelector('a[data-wp-state="split"]')!, + ); + const escape = () => fireEvent.keyDown(document.body, { key: 'Escape' }); + + beforeEach(() => { + document.body.append(document.createElement('live-region')); + announceSpy = vi.spyOn(LiveRegionElement.prototype, 'announce'); + window.I18n.store({ + en: { + js: { + work_packages: { + selection: { + all_selected: { + one: 'All %{count} work package on this page selected.', + other: 'All %{count} work packages on this page selected.', + }, + cleared: 'Selection cleared.', + selected: { one: '%{count} work package selected.', other: '%{count} work packages selected.' }, + }, + }, + }, + }, + }); + }); + + afterEach(async () => { + await harness.destroy(); + document.querySelector('live-region')?.remove(); + announceSpy.mockRestore(); + }); + + describe('on a capability-on table', () => { + beforeEach(() => mount(true)); + + it('plain click on an unselected row is silent', () => { + harness.click('1'); + harness.click('2'); + + expect(announceSpy).not.toHaveBeenCalled(); + }); + + it('plain click collapsing a batch announces the new count', () => { + harness.click('1'); + harness.click('3', { shiftKey: true }); + announceSpy.mockClear(); + + harness.click('2'); + + expect(spoken()).toEqual(['1 work package selected.']); + }); + + it('Ctrl-click toggle announces', () => { + harness.click('1'); + harness.click('2', { ctrlKey: true }); + harness.click('2', { ctrlKey: true }); + + expect(spoken()).toEqual(['2 work packages selected.', '1 work package selected.']); + }); + + it('Shift-click range announces', () => { + harness.click('1'); + harness.click('3', { shiftKey: true }); + + expect(spoken()).toEqual(['3 work packages selected.']); + }); + + it('Shift-click same-count replacement announces', () => { + harness.click('2'); + harness.click('1', { shiftKey: true }); + announceSpy.mockClear(); + + harness.click('3', { shiftKey: true }); + + expect(harness.selection.getSelectedWorkPackageIds()).toEqual(['2', '3']); + expect(spoken()).toEqual(['2 work packages selected.']); + }); + + it('Escape that clears membership announces "Selection cleared."', () => { + harness.click('1'); + harness.click('2', { ctrlKey: true }); + announceSpy.mockClear(); + + escape(); + + expect(spoken()).toEqual(['Selection cleared.']); + }); + + it('Escape that only drops an anchor is silent', () => { + harness.click('1'); + harness.click('1', { ctrlKey: true }); + announceSpy.mockClear(); + expect(harness.selection.hasSelectionState).toBe(true); + + escape(); + + expect(announceSpy).not.toHaveBeenCalled(); + expect(harness.selection.hasSelectionState).toBe(false); + }); + + it('right-click on an unselected row inside a batch collapse announces', () => { + harness.click('1'); + harness.click('2', { shiftKey: true }); + announceSpy.mockClear(); + + fireEvent.contextMenu(harness.row('3')); + + expect(spoken()).toEqual(['1 work package selected.']); + }); + + it('drag collapse announces', () => { + harness.click('1'); + harness.click('2', { shiftKey: true }); + announceSpy.mockClear(); + + harness.dragStart('3'); + + expect(spoken()).toEqual(['1 work package selected.']); + }); + + it('pruning after a re-render that drops a selected row announces the new count', async () => { + harness.click('1'); + harness.click('3', { shiftKey: true }); + announceSpy.mockClear(); + + await harness.render([{ id: '1' }, { id: '2' }]); + + expect(harness.selection.getSelectedWorkPackageIds()).toEqual(['1', '2']); + expect(spoken()).toEqual(['2 work packages selected.']); + }); + + it('pruning a selection set by the route on the first render is silent', async () => { + harness.selection.initializeSelection(['99']); + + await harness.render(); + + expect(harness.selection.isEmpty).toBe(true); + expect(announceSpy).not.toHaveBeenCalled(); + }); + + it('pruning after a programmatic update is silent', async () => { + harness.click('1'); + harness.click('3', { shiftKey: true }); + harness.selection.update({ selected: { 2: true, 3: true } }); + announceSpy.mockClear(); + + await harness.render([{ id: '1' }, { id: '2' }]); + + expect(harness.selection.getSelectedWorkPackageIds()).toEqual(['2']); + expect(announceSpy).not.toHaveBeenCalled(); + }); + + it('a gesture after a programmatic update announces normally', () => { + harness.selection.update({ selected: { 2: true } }); + + harness.click('3', { ctrlKey: true }); + + expect(spoken()).toEqual(['2 work packages selected.']); + }); + }); + + describe('split-details link on a capability-on table', () => { + beforeEach(() => mount(true, true)); + + it('collapsing a batch announces the new count once', () => { + harness.click('1'); + harness.click('2', { shiftKey: true }); + announceSpy.mockClear(); + + openDetails('3'); + + expect(harness.selection.getSelectedWorkPackageIds()).toEqual(['3']); + expect(spoken()).toEqual(['1 work package selected.']); + }); + + it('moving a single selection is silent', () => { + harness.click('1'); + + openDetails('2'); + + expect(harness.selection.getSelectedWorkPackageIds()).toEqual(['2']); + expect(announceSpy).not.toHaveBeenCalled(); + }); + }); + + describe('split-details link on a capability-off table', () => { + beforeEach(() => mount(false, true)); + + it('announces nothing when collapsing a batch', () => { + harness.click('1'); + harness.click('2', { shiftKey: true }); + + openDetails('3'); + + expect(harness.selection.getSelectedWorkPackageIds()).toEqual(['3']); + expect(announceSpy).not.toHaveBeenCalled(); + }); + }); + + describe('on a capability-off table', () => { + beforeEach(() => mount(false)); + + it('announces nothing across the same script', async () => { + harness.click('1'); + harness.click('3', { shiftKey: true }); + harness.click('2', { ctrlKey: true }); + harness.click('2'); + harness.click('1', { shiftKey: true }); + fireEvent.contextMenu(harness.row('3')); + harness.dragStart('2'); + escape(); + await harness.render([{ id: '1' }]); + + expect(harness.table.selectionFeedback).toBeNull(); + expect(announceSpy).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-feedback.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-feedback.ts new file mode 100644 index 000000000000..da245f73f99e --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-feedback.ts @@ -0,0 +1,88 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { DestroyRef } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; +import { SelectionAnnouncements, type SelectionGesture } from 'core-common/selection-announcements'; +import type { SelectionKey } from 'core-common/batch-selection'; +import { WorkPackageViewSelectionService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service'; +import type { WorkPackageTable } from '../../wp-fast-table'; + +export class TableSelectionFeedback { + private readonly announcements = new SelectionAnnouncements(() => 'js.work_packages.selection'); + + private gestureOwned:ReadonlySet|null = null; + + constructor(private readonly selection:WorkPackageViewSelectionService, destroyRef:DestroyRef) { + this.selection.live$() + .pipe(takeUntilDestroyed(destroyRef)) + .subscribe(() => { + if (this.gestureOwned && !sameKeys(this.gestureOwned, this.selection.selectionKeys)) { + this.gestureOwned = null; + } + }); + } + + gesture(kind:SelectionGesture, mutate:() => void):void { + const before = this.selection.selectionKeys; + mutate(); + this.announcements.commit(kind, this.selection.selectionKeys, before); + this.gestureOwned = this.selection.selectionKeys; + } + + /** Runs `mutate`, re-bases, and always speaks `all_selected` with the new count. */ + selectAll(mutate:() => void):void { + mutate(); + this.announcements.rebase(this.selection.selectionKeys); + this.announcements.speak('all_selected', this.selection.selectionCount); + this.gestureOwned = this.selection.selectionKeys; + } + + /** Speaks what `retain` pruned only from a membership this table's last gesture produced; otherwise re-bases silently. */ + prune(retain:() => void):void { + if (this.gestureOwned) { + this.gesture('selection', retain); + return; + } + + retain(); + this.announcements.rebase(this.selection.selectionKeys); + } +} + +function sameKeys(a:ReadonlySet, b:ReadonlySet):boolean { + return a.size === b.size && [...a].every((key) => b.has(key)); +} + +/** Runs `mutate` through the table's feedback when it has one, plainly otherwise. */ +export function selectionGesture(table:WorkPackageTable, kind:SelectionGesture, mutate:() => void):void { + if (table.selectionFeedback) { + table.selectionFeedback.gesture(kind, mutate); + } else { + mutate(); + } +} diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts index 6f94c165b7b6..dcc4970271a5 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts @@ -37,6 +37,7 @@ import { tableRowClassName } from '../../builders/rows/single-row-builder'; import { paintRowSelection } from '../../builders/rows/row-selection-painter'; import { locateTableRow, scrollTableRowIntoView } from '../../helpers/wp-table-row-helpers'; import { WorkPackageTable } from '../../wp-fast-table'; +import { selectionGesture, TableSelectionFeedback } from './selection-feedback'; import { registerWorkPackageDeselectAll, registerWorkPackageSelectAll, @@ -55,13 +56,22 @@ export class SelectionTransformer { public readonly table:WorkPackageTable) { const destroyRef = table.destroyRef; + if (table.configuration.keyboardNavigationEnabled) { + table.selectionFeedback = new TableSelectionFeedback(this.wpTableSelection, destroyRef); + } + this.querySpace.tableRendered.values$() .pipe( takeUntilDestroyed(destroyRef), ) .subscribe((rows) => { if (table.ledger.generation > 0) { - this.wpTableSelection.retainRendered(rows); + const retain = () => this.wpTableSelection.retainRendered(rows); + if (table.selectionFeedback) { + table.selectionFeedback.prune(retain); + } else { + retain(); + } } if (!table.configuration.keyboardNavigationEnabled) { this.wpTableFocus.ifShouldFocus((wpId:string) => { @@ -97,7 +107,7 @@ export class SelectionTransformer { const unregisterDeselectAll = registerWorkPackageDeselectAll({ root: table.tableAndTimelineContainer, hasState: () => this.wpTableSelection.hasSelectionState, - clear: () => this.wpTableSelection.reset(), + clear: () => selectionGesture(table, 'selection', () => this.wpTableSelection.reset()), }); onDestroySafely(destroyRef, unregisterSelectAll); onDestroySafely(destroyRef, unregisterDeselectAll); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table.ts index d3e650687257..722f9da28084 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table.ts @@ -49,6 +49,7 @@ import { type OccurrenceKey, RenderedOccurrenceLedger } from './rendered-occurre import { WorkPackageTableEditingContext } from './wp-table-editing'; import type { WorkPackageTableRow } from './wp-table.interfaces'; import type { KeyboardNavigation } from './handlers/state/keyboard/keyboard-navigation-transformer'; +import type { TableSelectionFeedback } from './handlers/state/selection-feedback'; import { TableRowPositions, type CapturedRowPositions } from './semantics/table-row-positions'; import { paintSectionExposure, TreegridSemantics } from './semantics/treegrid-semantics'; @@ -100,6 +101,8 @@ export class WorkPackageTable { public keyboardNavigation:KeyboardNavigation|null = null; + public selectionFeedback:TableSelectionFeedback|null = null; + public readonly semantics:TreegridSemantics|null = this.configuration.keyboardNavigationEnabled ? new TreegridSemantics(this) : null; public readonly rowPositions:TableRowPositions|null = this.configuration.keyboardNavigationEnabled ? new TableRowPositions(this) : null; From 36f0dcdb111358d39786642fe5af5b7eb3887091 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 20:04:20 +0100 Subject: [PATCH 5/8] [DREAM-894] Select table rows with the keyboard Adds the Shift+Space, Shift+ArrowUp/Down and Ctrl/Cmd+A chords to the capability-on work package table. A new KeyboardSelection collaborator resolves the eligible rendered occurrence at the roving position and runs toggle, range and select-all through the announcement feedback. KeyboardNavigationTransformer dispatches to it on the roving stop before navigation. Shift+Arrow moves focus first and extends the range only when focus landed on a selectable row, so group headers and sums rows leave the range unchanged. Select All anchors the occurrence under focus, or none from a structural row. The legacy capture-phase select-all registration is now skipped for capability-on tables, so Ctrl/Cmd+A is handled exactly once. https://community.openproject.org/wp/DREAM-894 --- .../keyboard-navigation-transformer.spec.ts | 74 +- .../keyboard-navigation-transformer.ts | 51 +- .../state/keyboard/keyboard-selection.spec.ts | 633 ++++++++++++++++++ .../state/keyboard/keyboard-selection.ts | 108 +++ .../handlers/state/selection-transformer.ts | 17 +- 5 files changed, 868 insertions(+), 15 deletions(-) create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-selection.spec.ts create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-selection.ts diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts index aa742dde44b7..008075204e26 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts @@ -27,6 +27,7 @@ import { fireEvent, waitFor, within } from '@testing-library/dom'; import { userEvent } from 'vitest/browser'; +import { usePlatform } from 'core-common/testing/platform'; import { States } from 'core-app/core/states/states.service'; import { DisplayFieldService } from 'core-app/shared/components/fields/display/display-field.service'; import { TextDisplayField } from 'core-app/shared/components/fields/display/field-types/text-display-field.module'; @@ -69,6 +70,7 @@ const grouped:Partial = { }; describe('KeyboardNavigationTransformer', () => { + usePlatform(); const harnesses:TableHarness[] = []; const mount = async (options:Partial = {}, beforeRender?:(table:TableHarness) => void) => { @@ -352,15 +354,16 @@ describe('KeyboardNavigationTransformer', () => { }); it.each([ - ['Alt', { altKey: true }], - ['composition', { isComposing: true }], - ['Ctrl', { ctrlKey: true }], - ['Shift', { shiftKey: true }], - ])('keys with %s are not handled', async (_name, init) => { + ['Alt', 'ArrowDown', { altKey: true }], + ['composition', 'ArrowDown', { isComposing: true }], + ['Ctrl', 'ArrowDown', { ctrlKey: true }], + ['Shift', 'PageDown', { shiftKey: true }], + ['Shift', 'End', { shiftKey: true }], + ])('keys with %s (%s) are not handled', async (_name, key, init) => { const table = await mount(); table.row('2').focus(); - expect(pressActive('ArrowDown', init)).toBe(true); + expect(pressActive(key, init)).toBe(true); expect(table.row('2')).toHaveFocus(); expect(navigation(table).position?.key).toBe(wpOccurrenceKey('2')); @@ -979,4 +982,63 @@ describe('KeyboardNavigationTransformer', () => { } }); }); + + describe('selection chords', () => { + it('fire only on the roving stop', async () => { + const table = await mount({}, (harness) => { + harness.injector.get(DisplayFieldService).addFieldType(AnchorDisplayField, 'text', ['String']); + }); + const subjectCell = table.row('1').querySelector('td.subject')!; + subjectCell.focus(); + const link = within(subjectCell).getByRole('link'); + + expect(press(link, ' ', { shiftKey: true })).toBe(true); + expect(press(link, 'ArrowDown', { shiftKey: true })).toBe(true); + + expect(selectedIds(table)).toEqual([]); + expect(subjectCell).toHaveFocus(); + }); + + it('leave Shift+ArrowLeft and Shift+ArrowRight unconsumed and in place', async () => { + const table = await mount(); + table.row('1').focus(); + + expect(pressActive('ArrowRight', { shiftKey: true })).toBe(true); + expect(pressActive('ArrowLeft', { shiftKey: true })).toBe(true); + + expect(table.row('1')).toHaveFocus(); + expect(navigation(table).position).toEqual({ key: wpOccurrenceKey('1'), cell: null, desiredColumn: null }); + }); + + it('leave Tab handling unchanged with Shift held', async () => { + const table = await mount(); + table.row('2').focus(); + + expect(pressActive('Tab', { shiftKey: true })).toBe(true); + + expect(table.row('2')).toHaveAttribute('tabindex', '0'); + expect(selectedIds(table)).toEqual([]); + }); + + it('handle Ctrl+A once on a capability-on table', async () => { + const table = await mount(); + const selectAll = vi.spyOn(table.selection, 'selectAll'); + table.row('1').focus(); + + expect(pressActive('a', { code: 'KeyA', ctrlKey: true })).toBe(false); + + expect(selectAll).toHaveBeenCalledTimes(1); + expect(selectedIds(table)).toEqual(['1', '2', '3']); + }); + + it('still select all through the legacy registration on a capability-off table', async () => { + const table = await mount({ configuration: { keyboardNavigationEnabled: false } }); + const selectAll = vi.spyOn(table.selection, 'selectAll'); + + expect(press(table.row('1'), 'a', { code: 'KeyA', ctrlKey: true })).toBe(false); + + expect(selectAll).toHaveBeenCalledTimes(1); + expect(selectedIds(table)).toEqual(['1', '2', '3']); + }); + }); }); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.ts index 1a1496ba470d..238058701d2b 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.ts @@ -35,6 +35,8 @@ import { WorkPackageViewFocusService } from 'core-app/features/work-packages/rou import { displayClassName, editableClassName } from 'core-app/shared/components/fields/display/display-field-renderer'; import { activeFieldContainerClassName } from 'core-app/shared/components/fields/edit/edit-form/edit-form'; import { onDestroySafely } from 'core-app/shared/helpers/angular/owned-ui-cleanup'; +import { isApplePlatform } from 'core-common/platform'; +import { WorkPackageViewSelectionService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service'; import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; import type { WorkPackageTable } from '../../../wp-fast-table'; import { wpOccurrenceKey } from '../../../rendered-occurrence-ledger'; @@ -48,6 +50,8 @@ import { import { buildNavigableRows, headerRowKey, headerRowOf, totalsRowKey, totalsRowOf, } from './navigable-rows'; +import { selectableOccurrences } from 'core-app/features/work-packages/routing/wp-view-base/view-services/rendered-occurrences'; +import { KeyboardSelection, selectionChord } from './keyboard-selection'; import { insideGrid, navigableCellClassName, navigableCells, paintRovingTabindex } from './roving-tabindex'; export interface KeyboardNavigation { @@ -101,6 +105,8 @@ export class KeyboardNavigationTransformer implements KeyboardNavigation { @LazyInject() public wpTableFocus:WorkPackageViewFocusService; + @LazyInject() public wpTableSelection:WorkPackageViewSelectionService; + private current:FocusPosition|null = null; private userPosition:FocusPosition|null = null; @@ -115,6 +121,8 @@ export class KeyboardNavigationTransformer implements KeyboardNavigation { private rendered:RenderedRows = { rows: [], groupBy: null }; + private selecting!:KeyboardSelection; + public onCellEnter = (position:FocusPosition):boolean => { const field = this.elementAt(position)?.querySelector(`.${displayClassName}.${editableClassName}`); return !!field && activateTableField(this.table, field); @@ -131,6 +139,7 @@ export class KeyboardNavigationTransformer implements KeyboardNavigation { const { destroyRef, scrollContainer } = table; table.keyboardNavigation = this; + this.selecting = new KeyboardSelection(table, this.wpTableSelection); const removedFromTabOrder = [scrollContainer, table.tableAndTimelineContainer] .filter((container, index, containers) => containers.indexOf(container) === index && !container.hasAttribute('tabindex')); removedFromTabOrder.forEach((container) => container.setAttribute('tabindex', '-1')); @@ -416,7 +425,11 @@ export class KeyboardNavigationTransformer implements KeyboardNavigation { return; } - if (!position || this.ignored(event) || event.target !== this.elementAt(position)) { + if (!position || event.target !== this.elementAt(position)) { + return; + } + + if (this.handleSelectionChord(position, event) || this.ignored(event)) { return; } @@ -429,6 +442,42 @@ export class KeyboardNavigationTransformer implements KeyboardNavigation { } } + private handleSelectionChord(position:FocusPosition, event:KeyboardEvent):boolean { + const chord = selectionChord(event, isApplePlatform()); + switch (chord) { + case null: + return false; + case 'toggle': + if (this.selecting.occurrenceAt(position)) { + event.preventDefault(); + if (!event.repeat) { + this.selecting.toggle(position); + } + } + return true; + case 'selectAll': + if (selectableOccurrences(this.table.renderedRows).length > 0) { + event.preventDefault(); + if (!event.repeat) { + this.selecting.selectAll(position); + } + } + return true; + default: + event.preventDefault(); + this.extendSelection(position, event); + return true; + } + } + + private extendSelection(position:FocusPosition, event:KeyboardEvent):void { + this.navigate(position, event); + const arrived = this.current; + if (arrived && (arrived.key !== position.key || arrived.cell !== position.cell)) { + this.selecting.extendTo(arrived); + } + } + private ignored(event:KeyboardEvent):boolean { const command = event.ctrlKey || event.metaKey; return event.isComposing diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-selection.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-selection.spec.ts new file mode 100644 index 000000000000..669c867d2dfd --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-selection.spec.ts @@ -0,0 +1,633 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { createEvent, fireEvent, waitFor } from '@testing-library/dom'; +import { LiveRegionElement } from '@primer/live-region-element'; +import { type MockInstance } from 'vitest'; +import { usePlatform } from 'core-common/testing/platform'; +import { WorkPackageViewHierarchiesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-hierarchy.service'; +import { buildTable, TableHarness, TableHarnessOptions } from '../../../testing/table-harness'; +import { wpOccurrenceKey } from '../../../rendered-occurrence-ledger'; +import { headerRowKey, totalsRowKey } from './navigable-rows'; +import { selectionChord } from './keyboard-selection'; + +const newStatus = { href: '/api/v3/statuses/1' }; +const inProgressStatus = { href: '/api/v3/statuses/2' }; + +const flat:Partial = { + workPackages: [{ id: '1' }, { id: '2' }, { id: '3' }, { id: '4' }, { id: '5' }], +}; + +const groupedWorkPackages = [ + { id: '1', attributes: { status: newStatus } }, + { id: '2', attributes: { status: newStatus } }, + { id: '3', attributes: { status: inProgressStatus } }, + { id: '4', attributes: { status: inProgressStatus } }, +]; + +const grouped:Partial = { + workPackages: groupedWorkPackages, + groups: [ + { value: 'New', href: newStatus.href, count: 2 }, + { value: 'In progress', href: inProgressStatus.href, count: 2 }, + ], +}; + +const groupedWithSums:Partial = { + workPackages: groupedWorkPackages, + groups: [ + { value: 'New', href: newStatus.href, count: 2, sums: { storyPoints: 13 } }, + { value: 'In progress', href: inProgressStatus.href, count: 2, sums: { storyPoints: 21 } }, + ], +}; + +describe('KeyboardSelection', () => { + const harnesses:TableHarness[] = []; + const pretendPlatform = usePlatform(); + let announceSpy:MockInstance; + + const mount = async (options:Partial = flat) => { + const table = buildTable({ + productionMarkup: true, + ...options, + configuration: { keyboardNavigationEnabled: true, ...options.configuration }, + } as TableHarnessOptions); + harnesses.push(table); + await table.render(); + return table; + }; + + const navigation = (table:TableHarness) => table.table.keyboardNavigation!; + + const focusKey = (table:TableHarness, key:string) => navigation(table) + .focusPosition({ key, cell: null, desiredColumn: null }); + + const focusRow = (table:TableHarness, id:string) => focusKey(table, wpOccurrenceKey(id)); + + const focusElement = (table:TableHarness, element:HTMLElement) => navigation(table) + .focusPosition(navigation(table).positionOfCell(element)!); + + const press = (key:string, init:KeyboardEventInit = {}) => fireEvent.keyDown(document.activeElement!, { key, ...init }); + + const shiftSpace = (init:KeyboardEventInit = {}) => press(' ', { code: 'Space', shiftKey: true, ...init }); + const shiftDown = (init:KeyboardEventInit = {}) => press('ArrowDown', { shiftKey: true, ...init }); + const shiftUp = (init:KeyboardEventInit = {}) => press('ArrowUp', { shiftKey: true, ...init }); + const ctrlA = (init:KeyboardEventInit = {}) => press('a', { code: 'KeyA', ctrlKey: true, ...init }); + + // Stubbed rather than passed as `modifierAltGraph`: not every engine maps + // the init member onto getModifierState. + const withAltGraph = (event:T):T => { + vi.spyOn(event as unknown as KeyboardEvent, 'getModifierState').mockImplementation((key) => key === 'AltGraph'); + return event; + }; + + const selected = (table:TableHarness) => table.selection.getSelectedWorkPackageIds().sort(); + const spoken = () => announceSpy.mock.calls.map(([message]) => message); + + beforeEach(() => { + document.body.append(document.createElement('live-region')); + announceSpy = vi.spyOn(LiveRegionElement.prototype, 'announce'); + window.I18n.store({ + en: { + js: { + work_packages: { + selection: { + all_selected: { + one: 'All %{count} work package on this page selected.', + other: 'All %{count} work packages on this page selected.', + }, + cleared: 'Selection cleared.', + selected: { one: '%{count} work package selected.', other: '%{count} work packages selected.' }, + }, + }, + }, + }, + }); + }); + + afterEach(async () => { + await Promise.all(harnesses.splice(0).map((table) => table.destroy())); + document.querySelector('live-region')?.remove(); + announceSpy.mockRestore(); + }); + + describe('selectionChord', () => { + const chord = (init:KeyboardEventInit & { key:string }, apple = false) => selectionChord(new KeyboardEvent('keydown', init), apple); + + it('recognises the three chords and nothing else', () => { + expect(chord({ key: ' ', shiftKey: true })).toBe('toggle'); + expect(chord({ key: 'ArrowUp', shiftKey: true })).toBe('extend'); + expect(chord({ key: 'ArrowDown', shiftKey: true })).toBe('extend'); + expect(chord({ key: 'a', ctrlKey: true })).toBe('selectAll'); + expect(chord({ key: 'a', metaKey: true }, true)).toBe('selectAll'); + expect(chord({ key: 'a', ctrlKey: true }, true)).toBeNull(); + expect(chord({ key: 'a', ctrlKey: true, shiftKey: true })).toBeNull(); + expect(chord({ key: ' ' })).toBeNull(); + expect(chord({ key: 'ArrowDown' })).toBeNull(); + expect(chord({ key: 'ArrowLeft', shiftKey: true })).toBeNull(); + expect(chord({ key: 'ArrowDown', shiftKey: true, ctrlKey: true })).toBeNull(); + expect(chord({ key: ' ', shiftKey: true, metaKey: true })).toBeNull(); + expect(chord({ key: ' ', shiftKey: true, altKey: true })).toBeNull(); + expect(chord({ key: ' ', shiftKey: true, isComposing: true })).toBeNull(); + expect(selectionChord(withAltGraph(new KeyboardEvent('keydown', { key: 'ArrowDown', shiftKey: true })), false)).toBeNull(); + }); + }); + + describe('Shift+Space', () => { + it('on a row toggles it and anchors it', async () => { + const table = await mount(); + focusRow(table, '2'); + + expect(shiftSpace()).toBe(false); + expect(selected(table)).toEqual(['2']); + + shiftDown(); + shiftDown(); + expect(selected(table)).toEqual(['2', '3', '4']); + }); + + it('on a cell toggles its row', async () => { + const table = await mount(); + focusElement(table, table.row('2').querySelector('td.subject')!); + + expect(shiftSpace()).toBe(false); + + expect(selected(table)).toEqual(['2']); + }); + + it('deselect keeps the anchor', async () => { + const table = await mount(); + focusRow(table, '2'); + shiftSpace(); + shiftSpace(); + expect(selected(table)).toEqual([]); + + shiftDown(); + + expect(selected(table)).toEqual(['2', '3']); + }); + + it('on a group header is not consumed', async () => { + const table = await mount(grouped); + focusElement(table, table.groupHeader(0)); + + expect(shiftSpace()).toBe(true); + + expect(selected(table)).toEqual([]); + }); + + it('held toggles once', async () => { + const table = await mount(); + focusRow(table, '1'); + + expect(shiftSpace()).toBe(false); + expect(shiftSpace({ repeat: true })).toBe(false); + + expect(selected(table)).toEqual(['1']); + }); + + it('announces the count', async () => { + const table = await mount(); + focusRow(table, '1'); + + shiftSpace(); + + expect(spoken()).toEqual(['1 work package selected.']); + }); + }); + + describe('Shift+Arrow', () => { + it('Down twice from an anchored row selects three', async () => { + const table = await mount(); + table.click('1'); + focusRow(table, '1'); + + expect(shiftDown()).toBe(false); + shiftDown(); + + expect(selected(table)).toEqual(['1', '2', '3']); + expect(table.row('3')).toHaveFocus(); + }); + + it('Up after extending down shrinks', async () => { + const table = await mount(); + table.click('1'); + focusRow(table, '1'); + shiftDown(); + shiftDown(); + + shiftUp(); + + expect(selected(table)).toEqual(['1', '2']); + }); + + it('reverses past the anchor', async () => { + const table = await mount(); + table.click('3'); + focusRow(table, '3'); + shiftDown(); + shiftUp(); + shiftUp(); + shiftUp(); + + expect(selected(table)).toEqual(['1', '2', '3']); + }); + + it('range includes the hidden children of a collapsed parent', async () => { + const table = await mount({ + workPackages: [{ id: '1', children: [{ id: '2' }] }, { id: '2', ancestors: [{ id: '1' }] }, { id: '3' }], + showHierarchies: true, + }); + table.injector.get(WorkPackageViewHierarchiesService).collapse('1'); + await waitFor(() => expect(table.row('2')).not.toBeVisible()); + table.click('1'); + focusRow(table, '1'); + + shiftDown(); + + expect(table.row('3')).toHaveFocus(); + expect(selected(table)).toEqual(['1', '2', '3']); + }); + + it('range includes both occurrences of a work package once', async () => { + const table = buildTable({ + productionMarkup: true, + workPackages: [{ id: '1' }, { id: '2' }, { id: '3' }], + relations: [{ from: '1', to: '2', type: 'follows', reverseType: 'precedes' }], + columns: ['id', 'subject', { id: 'relationsOfTypeFollows', relationType: 'follows' }], + configuration: { keyboardNavigationEnabled: true }, + }); + harnesses.push(table); + table.expand('1', 'relationsOfTypeFollows'); + await table.render(); + table.click('1'); + focusRow(table, '1'); + + shiftDown(); + shiftDown(); + + expect(table.row('2')).toHaveFocus(); + expect(selected(table)).toEqual(['1', '2']); + }); + + describe('onto structural rows', () => { + it('Down onto a group header moves focus but leaves the range unchanged', async () => { + const table = await mount(grouped); + table.click('2'); + focusRow(table, '2'); + + expect(shiftDown()).toBe(false); + + expect(table.groupHeader(1)).toHaveFocus(); + expect(selected(table)).toEqual(['2']); + }); + + it('continuing onto the next eligible row extends to it', async () => { + const table = await mount(grouped); + table.click('2'); + focusRow(table, '2'); + shiftDown(); + + shiftDown(); + + expect(table.row('3')).toHaveFocus(); + expect(selected(table)).toEqual(['2', '3']); + }); + + it('consecutive structural stops leave the range unchanged', async () => { + const table = await mount(groupedWithSums); + table.click('2'); + focusRow(table, '2'); + + shiftDown(); + expect(table.row('2').nextElementSibling).toHaveFocus(); + expect(selected(table)).toEqual(['2']); + + shiftDown(); + expect(table.groupHeader(1)).toHaveFocus(); + expect(selected(table)).toEqual(['2']); + }); + }); + + describe('without an anchor', () => { + it('first Down replaces the selection with the row reached and anchors it', async () => { + const table = await mount(); + table.click('1'); + table.click('4', { ctrlKey: true }); + fireEvent.keyDown(document.body, { key: 'Escape' }); + focusRow(table, '2'); + + shiftDown(); + expect(selected(table)).toEqual(['3']); + + shiftDown(); + expect(selected(table)).toEqual(['3', '4']); + }); + + it('first Up replaces the selection with the row reached and anchors it', async () => { + const table = await mount(); + focusRow(table, '3'); + + shiftUp(); + expect(selected(table)).toEqual(['2']); + + shiftUp(); + expect(selected(table)).toEqual(['1', '2']); + }); + + it('Select All from the column header then Down collapses to the first eligible row', async () => { + const table = await mount(); + focusKey(table, headerRowKey); + ctrlA(); + expect(selected(table)).toEqual(['1', '2', '3', '4', '5']); + + shiftDown(); + + expect(table.row('1')).toHaveFocus(); + expect(selected(table)).toEqual(['1']); + }); + }); + + describe('at the edges', () => { + it('Up on the first body row with the header absent changes nothing but is consumed', async () => { + const table = await mount({ ...flat, productionMarkup: false }); + table.click('1'); + focusRow(table, '1'); + + announceSpy.mockClear(); + + expect(shiftUp()).toBe(false); + + expect(table.row('1')).toHaveFocus(); + expect(selected(table)).toEqual(['1']); + expect(announceSpy).not.toHaveBeenCalled(); + }); + + it('held Down keeps extending', async () => { + const table = await mount(); + table.click('1'); + focusRow(table, '1'); + + shiftDown(); + shiftDown({ repeat: true }); + shiftDown({ repeat: true }); + + expect(selected(table)).toEqual(['1', '2', '3', '4']); + }); + }); + + it('same-count replacement announces', async () => { + const table = await mount(); + table.click('2'); + table.click('1', { shiftKey: true }); + focusRow(table, '2'); + announceSpy.mockClear(); + + shiftDown(); + + expect(selected(table)).toEqual(['2', '3']); + expect(spoken()).toEqual(['2 work packages selected.']); + }); + }); + + describe('Select All', () => { + it('Ctrl+A on a row selects the page and anchors that occurrence', async () => { + const table = await mount(); + focusRow(table, '3'); + + expect(ctrlA()).toBe(false); + expect(selected(table)).toEqual(['1', '2', '3', '4', '5']); + + table.click('5', { shiftKey: true }); + expect(selected(table)).toEqual(['3', '4', '5']); + }); + + it('Ctrl+A on the header, a group header, sums and totals selects the page with no anchor', async () => { + const table = await mount(groupedWithSums); + const structural = [ + () => focusKey(table, headerRowKey), + () => focusElement(table, table.groupHeader(0)), + () => focusElement(table, table.row('2').nextElementSibling as HTMLElement), + () => focusKey(table, totalsRowKey), + ]; + + structural.forEach((focus) => { + table.selection.reset(); + focus(); + + expect(ctrlA()).toBe(false); + + expect(selected(table)).toEqual(['1', '2', '3', '4']); + table.click('4', { shiftKey: true }); + expect(selected(table)).toEqual(['4']); + }); + }); + + it('Ctrl+A from a structural row clears an existing anchor', async () => { + const table = await mount(); + table.click('3'); + focusKey(table, headerRowKey); + + ctrlA(); + table.click('5', { shiftKey: true }); + + expect(selected(table)).toEqual(['5']); + }); + + it('Ctrl+A dedups a work package rendered twice', async () => { + const table = buildTable({ + productionMarkup: true, + workPackages: [{ id: '1' }, { id: '2' }, { id: '3' }], + relations: [{ from: '1', to: '2', type: 'follows', reverseType: 'precedes' }], + columns: ['id', 'subject', { id: 'relationsOfTypeFollows', relationType: 'follows' }], + configuration: { keyboardNavigationEnabled: true }, + }); + harnesses.push(table); + table.expand('1', 'relationsOfTypeFollows'); + await table.render(); + focusRow(table, '1'); + + ctrlA(); + + expect(selected(table)).toEqual(['1', '2', '3']); + }); + + it('Ctrl+A on a page with only group headers is not consumed', async () => { + const table = await mount(grouped); + focusElement(table, table.groupHeader(0)); + vi.spyOn(table.table, 'renderedRows', 'get') + .mockReturnValue(table.querySpace.tableRendered.value!.filter((row) => row.workPackageId === null)); + + expect(ctrlA()).toBe(true); + + expect(selected(table)).toEqual([]); + }); + + it('Ctrl+Shift+A is not consumed', async () => { + const table = await mount(); + focusRow(table, '1'); + + expect(ctrlA({ shiftKey: true })).toBe(true); + + expect(selected(table)).toEqual([]); + }); + + it('Ctrl+A with AltGraph or during composition is not consumed', async () => { + const table = await mount(); + focusRow(table, '1'); + + const target = document.activeElement!; + expect(fireEvent(target, withAltGraph(createEvent.keyDown(target, { key: 'a', code: 'KeyA', ctrlKey: true })))).toBe(true); + expect(ctrlA({ isComposing: true })).toBe(true); + + expect(selected(table)).toEqual([]); + }); + + it('Ctrl+A on a non-Latin layout selects all', async () => { + const table = await mount(); + focusRow(table, '1'); + + expect(press('ф', { code: 'KeyA', ctrlKey: true })).toBe(false); + + expect(selected(table)).toEqual(['1', '2', '3', '4', '5']); + }); + + it('Meta+A on Apple selects all and Ctrl+A on Apple does not', async () => { + pretendPlatform('macOS'); + const table = await mount(); + focusRow(table, '1'); + + expect(ctrlA()).toBe(true); + expect(selected(table)).toEqual([]); + + expect(press('a', { code: 'KeyA', metaKey: true })).toBe(false); + expect(selected(table)).toEqual(['1', '2', '3', '4', '5']); + }); + + it('announces the page count', async () => { + const table = await mount(); + focusRow(table, '1'); + + ctrlA(); + + expect(spoken()).toEqual(['All 5 work packages on this page selected.']); + }); + + it('held announces once', async () => { + const table = await mount(); + focusRow(table, '1'); + + ctrlA(); + ctrlA({ repeat: true }); + ctrlA({ repeat: true }); + + expect(spoken()).toEqual(['All 5 work packages on this page selected.']); + }); + + it('announces again when everything is already selected', async () => { + const table = await mount(); + focusRow(table, '1'); + ctrlA(); + announceSpy.mockClear(); + + ctrlA(); + + expect(spoken()).toEqual(['All 5 work packages on this page selected.']); + }); + + it('does not select the draft of an open create row', async () => { + const table = await mount(); + table.addInlineCreateRow(); + focusRow(table, '1'); + + ctrlA(); + + expect(selected(table)).toEqual(['1', '2', '3', '4', '5']); + }); + }); + + describe('inside the create row', () => { + it('Shift+Space in an input is untouched', async () => { + const table = await mount(); + const row = table.addInlineCreateRow(); + const input = document.createElement('input'); + row.append(input); + input.focus(); + + expect(shiftSpace()).toBe(true); + + expect(selected(table)).toEqual([]); + }); + }); + + describe('pointer and keyboard parity', () => { + it('pointer then keyboard then pointer matches pointer only', async () => { + const keyboard = await mount(); + keyboard.click('1'); + focusRow(keyboard, '1'); + shiftDown(); + shiftDown(); + keyboard.click('5', { shiftKey: true }); + + const pointer = await mount(); + pointer.click('1'); + pointer.click('3', { shiftKey: true }); + pointer.click('5', { shiftKey: true }); + + expect(selected(keyboard)).toEqual(selected(pointer)); + keyboard.click('2', { shiftKey: true }); + pointer.click('2', { shiftKey: true }); + expect(selected(keyboard)).toEqual(selected(pointer)); + }); + + it('an Escape-cleared anchor behaves like a Shift-click on the same row', async () => { + const escape = () => fireEvent.keyDown(document.body, { key: 'Escape' }); + + const keyboard = await mount(); + keyboard.click('1'); + keyboard.click('4', { ctrlKey: true }); + escape(); + focusRow(keyboard, '2'); + shiftDown(); + const afterKeyboard = selected(keyboard); + keyboard.click('5', { shiftKey: true }); + const nextAfterKeyboard = selected(keyboard); + await keyboard.destroy(); + + const pointer = await mount(); + pointer.click('1'); + pointer.click('4', { ctrlKey: true }); + escape(); + pointer.click('3', { shiftKey: true }); + const afterPointer = selected(pointer); + pointer.click('5', { shiftKey: true }); + + expect(afterKeyboard).toEqual(afterPointer); + expect(nextAfterKeyboard).toEqual(selected(pointer)); + }); + }); +}); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-selection.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-selection.ts new file mode 100644 index 000000000000..dfbb8d7f7dff --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-selection.ts @@ -0,0 +1,108 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { isSelectAllShortcut } from 'core-common/selection-shortcuts'; +import { WorkPackageViewSelectionService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service'; +import { isSelectableOccurrence, selectableOccurrences } from 'core-app/features/work-packages/routing/wp-view-base/view-services/rendered-occurrences'; +import type { WorkPackageTable } from '../../../wp-fast-table'; +import { selectionGesture } from '../selection-feedback'; +import type { FocusPosition } from './focus-position'; + +export type SelectionChord = 'toggle'|'extend'|'selectAll'; + +export function selectionChord(event:KeyboardEvent, applePlatform:boolean):SelectionChord|null { + if (event.isComposing || event.altKey || event.getModifierState('AltGraph')) { + return null; + } + + if (isSelectAllShortcut(event, applePlatform)) { + return 'selectAll'; + } + + if (!event.shiftKey || event.ctrlKey || event.metaKey) { + return null; + } + + switch (event.key) { + case ' ': + return 'toggle'; + case 'ArrowUp': + case 'ArrowDown': + return 'extend'; + default: + return null; + } +} + +export class KeyboardSelection { + constructor( + private readonly table:WorkPackageTable, + private readonly selection:WorkPackageViewSelectionService, + ) {} + + occurrenceAt(position:FocusPosition):RenderedWorkPackage|null { + const classIdentifier = this.table.ledger.byKey(position.key)?.classIdentifier; + if (classIdentifier === undefined) { + return null; + } + + const rendered = this.table.renderedRows.find((row) => row.classIdentifier === classIdentifier); + return rendered && isSelectableOccurrence(rendered) ? rendered : null; + } + + toggle(position:FocusPosition):boolean { + const occurrence = this.occurrenceAt(position); + if (!occurrence) { + return false; + } + + selectionGesture(this.table, 'selection', () => this.selection.toggleOccurrence(occurrence)); + return true; + } + + extendTo(arrived:FocusPosition):void { + const occurrence = this.occurrenceAt(arrived); + if (occurrence) { + selectionGesture(this.table, 'selection', () => this.selection.rangeTo(occurrence, this.table.renderedRows)); + } + } + + selectAll(position:FocusPosition):boolean { + const rows = this.table.renderedRows; + if (selectableOccurrences(rows).length === 0) { + return false; + } + + const select = () => this.selection.selectAll(rows, this.occurrenceAt(position)); + if (this.table.selectionFeedback) { + this.table.selectionFeedback.selectAll(select); + } else { + select(); + } + return true; + } +} diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts index dcc4970271a5..5c1f3ae26917 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts @@ -97,19 +97,20 @@ export class SelectionTransformer { ) .subscribe(() => this.paintRows()); - const unregisterSelectAll = registerWorkPackageSelectAll({ - root: table.tableAndTimelineContainer, - focusSelector: '.wp-table--row', - occurrenceSelector: '.wp-table--row[data-work-package-id][data-class-identifier]', - rendered: () => table.renderedRows, - selectAll: (rows, anchor) => this.wpTableSelection.selectAll(rows, anchor), - }); + if (!table.configuration.keyboardNavigationEnabled) { + onDestroySafely(destroyRef, registerWorkPackageSelectAll({ + root: table.tableAndTimelineContainer, + focusSelector: '.wp-table--row', + occurrenceSelector: '.wp-table--row[data-work-package-id][data-class-identifier]', + rendered: () => table.renderedRows, + selectAll: (rows, anchor) => this.wpTableSelection.selectAll(rows, anchor), + })); + } const unregisterDeselectAll = registerWorkPackageDeselectAll({ root: table.tableAndTimelineContainer, hasState: () => this.wpTableSelection.hasSelectionState, clear: () => selectionGesture(table, 'selection', () => this.wpTableSelection.reset()), }); - onDestroySafely(destroyRef, unregisterSelectAll); onDestroySafely(destroyRef, unregisterDeselectAll); } From 87902c52d78accc2711c594b5afe2ee903c82a91 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 20:09:00 +0100 Subject: [PATCH 6/8] [DREAM-894] Open row menus with Shift+F10 and Menu Opens the work package row context menu from the keyboard with Shift+F10 and the Menu (ContextMenu) key, in addition to the existing Shift+Alt+F10. Cell focus resolves to its row, and group headers, the column header and totals stay inert. Repeat keydowns of a held chord are swallowed, so holding the key opens the menu once. Browsers fire a companion contextmenu event after these keys. The shared ContextMenuHandler records each keyboard open per table with its row and swallows the first contextmenu on that row within 500 ms, so a keyboard invocation opens the menu exactly once. A repeat keydown on the same row refreshes the record, so the companion after a long hold is still swallowed. A pointerdown on the table or the table's teardown forgets the record, and a right-click on another row, or after the window, opens normally. Inside an editable control (input, textarea, select, contenteditable other than contenteditable="false", or an active edit field) both the chords and right-click are left to the browser, so the native menu shows instead of the row menu. Both handlers share one predicate for this. The work package id link keeps its native link menu for the chords as it already does for right-click. The new chords, the editor refusal and the id link exemption apply only to tables with keyboard navigation. Other tables keep their previous behaviour: only Shift+Alt+F10 opens the row menu, also on the id link and inside an editor, and right-click inside an editor opens the row menu. https://community.openproject.org/wp/DREAM-894 --- .../context-menu/context-menu-entry.spec.ts | 362 ++++++++++++++++++ .../context-menu/context-menu-handler.ts | 39 ++ .../context-menu-keyboard-handler.ts | 31 +- .../context-menu-rightclick-handler.ts | 21 +- 4 files changed, 447 insertions(+), 6 deletions(-) diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-entry.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-entry.spec.ts index 09fba13adc03..0c4827c183bf 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-entry.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-entry.spec.ts @@ -31,6 +31,9 @@ import { WorkPackageContextMenuHelperService } from 'core-app/features/work-pack import { OPContextMenuService } from 'core-app/shared/components/op-context-menu/op-context-menu.service'; import { WorkPackageTableContextMenu } from 'core-app/shared/components/op-context-menu/wp-context-menu/wp-table-context-menu.directive'; import { buildTable, TableHarness, type TableHarnessOptions } from '../../testing/table-harness'; +import { wpOccurrenceKey } from '../../rendered-occurrence-ledger'; +import { headerRowKey, totalsRowKey } from '../state/keyboard/navigable-rows'; +import { uiStateLinkClass } from '../../builders/ui-state-link-builder'; describe('Context menu entry', () => { let harness:TableHarness; @@ -260,3 +263,362 @@ describe('Context menu entry on the inline-create row', () => { expect(harness.selection.getSelectedWorkPackageIds()).toEqual(['2']); }); }); + +describe('Context menu keyboard chords', () => { + let harness:TableHarness; + let menuTargets:string[][]; + let opened:ReturnType; + let clock:number; + + const newStatus = { href: '/api/v3/statuses/1' }; + const inProgressStatus = { href: '/api/v3/statuses/2' }; + + const chords:Record = { + 'Shift+F10': { key: 'F10', shiftKey: true }, + Menu: { key: 'ContextMenu' }, + 'Shift+Alt+F10': { key: 'F10', shiftKey: true, altKey: true }, + }; + + const mount = async (options:Partial = {}) => { + harness = buildTable({ + workPackages: [{ id: '1' }, { id: '2' }, { id: '3' }], + productionMarkup: true, + ...options, + configuration: { contextMenuEnabled: true, keyboardNavigationEnabled: true, ...options.configuration }, + }); + await harness.render(); + + menuTargets = []; + vi.spyOn(harness.injector.get(WorkPackageContextMenuHelperService), 'getPermittedActions') + .mockImplementation((workPackages:WorkPackageResource[]) => { + menuTargets.push(workPackages.map((wp) => wp.id!)); + return []; + }); + opened = vi.spyOn(harness.injector.get(OPContextMenuService), 'show'); + }; + + const focusKey = (key:string) => harness.table.keyboardNavigation! + .focusPosition({ key, cell: null, desiredColumn: null }); + + const press = (chord:string, element:HTMLElement) => fireEvent.keyDown(element, chords[chord]); + + beforeEach(() => { + clock = 1000; + vi.spyOn(performance, 'now').mockImplementation(() => clock); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + await harness.destroy(); + }); + + describe.each(Object.keys(chords))('%s', (chord) => { + beforeEach(() => mount()); + + it('opens once for the focused row', () => { + focusKey(wpOccurrenceKey('2')); + + expect(press(chord, harness.row('2'))).toBe(false); + + expect(opened).toHaveBeenCalledTimes(1); + expect(menuTargets).toEqual([['2']]); + }); + + it('opens for the row of the focused cell', () => { + focusKey(wpOccurrenceKey('2')); + const cell = harness.row('2').querySelector('td.subject')!; + cell.focus(); + + press(chord, cell); + + expect(opened).toHaveBeenCalledTimes(1); + expect(menuTargets).toEqual([['2']]); + }); + + it('swallows a held-key repeat without opening again', () => { + focusKey(wpOccurrenceKey('2')); + press(chord, harness.row('2')); + + expect(fireEvent.keyDown(harness.row('2'), { ...chords[chord], repeat: true })).toBe(false); + + expect(opened).toHaveBeenCalledTimes(1); + }); + + it('targets the selection when the row is selected', () => { + harness.click('1'); + harness.click('2', { shiftKey: true }); + + press(chord, harness.row('2')); + + expect(menuTargets).toEqual([['1', '2']]); + }); + + it('replaces the selection when the row is not selected', () => { + harness.click('1'); + + press(chord, harness.row('3')); + + expect(harness.selection.getSelectedWorkPackageIds()).toEqual(['3']); + expect(menuTargets).toEqual([['3']]); + }); + }); + + describe('companion contextmenu', () => { + beforeEach(() => mount()); + + it('is swallowed after a keyboard open', () => { + press('Shift+F10', harness.row('1')); + + expect(fireEvent.contextMenu(harness.row('1'))).toBe(false); + + expect(opened).toHaveBeenCalledTimes(1); + }); + + it('does not suppress a right-click on another row within the window', () => { + press('Shift+F10', harness.row('1')); + + fireEvent.contextMenu(harness.row('3')); + + expect(opened).toHaveBeenCalledTimes(2); + expect(menuTargets).toEqual([['1'], ['3']]); + }); + + it('does not suppress a right-click on the same row after pointer input', () => { + press('Shift+F10', harness.row('1')); + fireEvent.pointerDown(harness.row('1')); + + fireEvent.contextMenu(harness.row('1')); + + expect(opened).toHaveBeenCalledTimes(2); + }); + + it('leaves the chords and the following contextmenu native on a focused id link', () => { + const link = document.createElement('a'); + link.className = uiStateLinkClass; + link.href = '#'; + harness.row('1').querySelector('td.subject')!.append(link); + link.focus(); + + Object.keys(chords).forEach((chord) => expect(press(chord, link)).toBe(true)); + + expect(fireEvent.contextMenu(link)).toBe(true); + expect(opened).not.toHaveBeenCalled(); + }); + + it('is still swallowed at 499 ms', () => { + press('Shift+F10', harness.row('1')); + clock += 499; + + expect(fireEvent.contextMenu(harness.row('1'))).toBe(false); + + expect(opened).toHaveBeenCalledTimes(1); + }); + + it('does not suppress a genuine right-click at 501 ms', () => { + press('Shift+F10', harness.row('1')); + clock += 501; + + fireEvent.contextMenu(harness.row('1')); + + expect(opened).toHaveBeenCalledTimes(2); + }); + + it('is still swallowed after a chord held past the window', () => { + press('Shift+F10', harness.row('1')); + clock += 400; + fireEvent.keyDown(harness.row('1'), { ...chords['Shift+F10'], repeat: true }); + clock += 400; + fireEvent.keyDown(harness.row('1'), { ...chords['Shift+F10'], repeat: true }); + clock += 300; + + expect(fireEvent.contextMenu(harness.row('1'))).toBe(false); + + expect(opened).toHaveBeenCalledTimes(1); + }); + + it('is consumed only once', () => { + press('Menu', harness.row('1')); + fireEvent.contextMenu(harness.row('1')); + + fireEvent.contextMenu(harness.row('1')); + + expect(opened).toHaveBeenCalledTimes(2); + }); + + it('opens a bare contextmenu on a focused row and on a focused cell once each', () => { + focusKey(wpOccurrenceKey('1')); + fireEvent.contextMenu(harness.row('1')); + expect(opened).toHaveBeenCalledTimes(1); + + const cell = harness.row('2').querySelector('td.subject')!; + cell.focus(); + fireEvent.contextMenu(cell); + expect(opened).toHaveBeenCalledTimes(2); + }); + }); + + describe('structural rows', () => { + beforeEach(() => mount({ + workPackages: [ + { id: '1', attributes: { status: newStatus } }, + { id: '2', attributes: { status: inProgressStatus } }, + ], + groups: [ + { value: 'New', href: newStatus.href, count: 1, sums: { storyPoints: 13 } }, + { value: 'In progress', href: inProgressStatus.href, count: 1, sums: { storyPoints: 21 } }, + ], + })); + + it('do nothing on a group sums row', () => { + const sumsRow = harness.table.tableAndTimelineContainer.querySelector('.wp-table--sums-row')!; + + Object.keys(chords).forEach((chord) => expect(press(chord, sumsRow)).toBe(true)); + + expect(opened).not.toHaveBeenCalled(); + }); + + it('do nothing on a group header, the column header and totals', () => { + const targets = [ + () => harness.groupHeader(0), + () => { focusKey(headerRowKey); return harness.headerCells()[0]; }, + () => { focusKey(totalsRowKey); return harness.totalsRow(); }, + ]; + + targets.forEach((target) => { + const element = target(); + Object.keys(chords).forEach((chord) => press(chord, element)); + }); + + expect(opened).not.toHaveBeenCalled(); + }); + }); + + describe('inside an inline editor on a saved row', () => { + beforeEach(() => mount({ editing: {} })); + + const openSubjectEditor = () => { + fireEvent.click(harness.row('1').querySelector('td.subject .inline-edit--display-field')!); + return within(harness.row('1')).findByRole('textbox'); + }; + + it('leaves the chords to the editor', async () => { + const input = await openSubjectEditor(); + + Object.keys(chords).forEach((chord) => expect(press(chord, input)).toBe(true)); + + expect(opened).not.toHaveBeenCalled(); + }); + + it('leaves the native menu alone for the contextmenu that follows', async () => { + const input = await openSubjectEditor(); + + expect(fireEvent.contextMenu(input)).toBe(true); + + expect(opened).not.toHaveBeenCalled(); + }); + + it('still opens on right-click of a display cell', async () => { + await openSubjectEditor(); + + fireEvent.contextMenu(harness.row('2').querySelector('td.subject')!); + + expect(opened).toHaveBeenCalledTimes(1); + expect(menuTargets).toEqual([['2']]); + }); + }); + + describe('on a contenteditable="false" element', () => { + beforeEach(() => mount()); + + it('opens the row menu', () => { + const element = document.createElement('span'); + element.contentEditable = 'false'; + element.tabIndex = -1; + harness.row('1').querySelector('td.subject')!.append(element); + element.focus(); + + expect(press('Shift+F10', element)).toBe(false); + expect(fireEvent.contextMenu(harness.row('2').querySelector('td.subject')!.appendChild(element))).toBe(false); + + expect(menuTargets).toEqual([['1'], ['2']]); + }); + }); + + describe('on a capability-off table', () => { + beforeEach(() => mount({ editing: {}, configuration: { keyboardNavigationEnabled: false } })); + + const openSubjectEditor = () => { + fireEvent.click(harness.row('1').querySelector('td.subject .inline-edit--display-field')!); + return within(harness.row('1')).findByRole('textbox'); + }; + + it('leaves Shift+F10 and Menu unhandled', () => { + harness.row('2').focus(); + + expect(press('Shift+F10', harness.row('2'))).toBe(true); + expect(press('Menu', harness.row('2'))).toBe(true); + + expect(opened).not.toHaveBeenCalled(); + }); + + it('opens on Shift+Alt+F10', () => { + harness.row('2').focus(); + + expect(press('Shift+Alt+F10', harness.row('2'))).toBe(false); + + expect(menuTargets).toEqual([['2']]); + }); + + it('opens the row menu on Shift+Alt+F10 on the id link', () => { + const link = document.createElement('a'); + link.className = uiStateLinkClass; + link.href = '#'; + harness.row('1').querySelector('td.subject')!.append(link); + link.focus(); + + expect(press('Shift+Alt+F10', link)).toBe(false); + + expect(menuTargets).toEqual([['1']]); + }); + + it('opens the row menu on Shift+Alt+F10 inside an open editor', async () => { + const input = await openSubjectEditor(); + + expect(press('Shift+Alt+F10', input)).toBe(false); + + expect(menuTargets).toEqual([['1']]); + }); + + it('opens the row menu on right-click inside an open editor', async () => { + const input = await openSubjectEditor(); + + expect(fireEvent.contextMenu(input)).toBe(false); + + expect(menuTargets).toEqual([['1']]); + }); + }); + + describe('with the context menu disabled', () => { + beforeEach(() => mount({ configuration: { contextMenuEnabled: false } })); + + it('leaves all three chords inert', () => { + Object.keys(chords).forEach((chord) => press(chord, harness.row('1'))); + + expect(opened).not.toHaveBeenCalled(); + }); + }); + + describe('on the inline-create row', () => { + beforeEach(async () => { + await mount({ configuration: { inlineCreateEnabled: true } }); + }); + + it('is refused', () => { + const inlineCreateRow = harness.addInlineCreateRow(); + + Object.keys(chords).forEach((chord) => press(chord, inlineCreateRow)); + + expect(opened).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts index 4c1447dae486..78311bf6e97f 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts @@ -27,9 +27,11 @@ 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 { 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'; import { tableRowClassName } from '../../builders/rows/single-row-builder'; import { WorkPackageTable } from '../../wp-fast-table'; import { TableEventComponent, TableEventHandler } from '../table-handler-registry'; @@ -38,6 +40,12 @@ import { PositionArgs } from 'core-app/shared/components/op-context-menu/wp-cont import { EventType } from 'core-app/features/work-packages/routing/wp-view-base/event-handling/event-handler-registry'; import { WorkPackageViewSelectionGesturesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection-gestures.service'; +/** Window in which a `contextmenu` following a keyboard-opened menu on the same table is its companion, not a new request. */ +export const companionContextMenuWindowMs = 500; + +const keyboardOpens = new WeakMap(); +const forgetOnTeardown = new WeakSet(); + export abstract class ContextMenuHandler implements TableEventHandler { // Injections @LazyInject() public opContextMenu:OPContextMenuService; @@ -65,6 +73,37 @@ export abstract class ContextMenuHandler implements TableEventHandler { return row?.dataset.workPackageId === HAL_NEW_RESOURCE_ID; } + protected isEditing(target:HTMLElement):boolean { + return !!target.closest(`input, textarea, select, [contenteditable]:not([contenteditable="false"]), .${activeFieldContainerClassName}`); + } + + protected noteKeyboardOpen(table:WorkPackageTable, row:HTMLElement):void { + if (!forgetOnTeardown.has(table)) { + forgetOnTeardown.add(table); + onDestroySafely(table.destroyRef, () => keyboardOpens.delete(table)); + } + keyboardOpens.set(table, { row, at: performance.now() }); + } + + protected refreshKeyboardOpen(table:WorkPackageTable, row:HTMLElement):void { + const open = keyboardOpens.get(table); + if (open?.row === row) { + open.at = performance.now(); + } + } + + protected forgetKeyboardOpen(table:WorkPackageTable):void { + keyboardOpens.delete(table); + } + + /** True once for the first `contextmenu` on `target` within the window after a keyboard open of its row; clears the record. */ + protected consumeCompanion(table:WorkPackageTable, target:HTMLElement):boolean { + const open = keyboardOpens.get(table); + keyboardOpens.delete(table); + + return !!open && performance.now() - open.at <= companionContextMenuWindowMs && open.row.contains(target); + } + protected openContextMenu(table:WorkPackageTable, evt:Event, row:HTMLElement, positionArgs:PositionArgs = {}):void { const workPackageId = row.dataset.workPackageId!; selectionGesture(table, 'navigation', () => { diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-keyboard-handler.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-keyboard-handler.ts index c9087a37304c..340ed516531d 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-keyboard-handler.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-keyboard-handler.ts @@ -27,6 +27,8 @@ import { Injector } from '@angular/core'; import { TableEventComponent } from 'core-app/features/work-packages/components/wp-fast-table/handlers/table-handler-registry'; +import { uiStateLinkClass } from '../../builders/ui-state-link-builder'; +import { keyboardNavigationEnabled } from '../state/keyboard/roving-tabindex'; import { ContextMenuHandler } from './context-menu-handler'; import { EventType } from 'core-app/features/work-packages/routing/wp-view-base/event-handling/event-handler-registry'; @@ -44,25 +46,34 @@ export class ContextMenuKeyboardHandler extends ContextMenuHandler { } public handleEvent(component:TableEventComponent, evt:KeyboardEvent):boolean { - if (!component.workPackageTable.configuration.contextMenuEnabled) { + const { configuration } = component.workPackageTable; + if (!configuration.contextMenuEnabled) { return false; } const target = evt.target as HTMLElement; + const keyboardNavigation = keyboardNavigationEnabled(configuration); - if (!(evt.key === 'F10' && evt.shiftKey && evt.altKey)) { + if (!this.isMenuChord(evt, keyboardNavigation) + || (keyboardNavigation && (this.isEditing(target) || target.closest(`.${uiStateLinkClass}`)))) { return true; } - const row = target.closest(this.SELECTOR)!; + const row = target.closest(this.SELECTOR); - if (!row.dataset.workPackageId || this.isUnsavedRow(row)) { + if (!row?.dataset.workPackageId || this.isUnsavedRow(row)) { return true; } evt.preventDefault(); evt.stopPropagation(); + if (evt.repeat) { + this.refreshKeyboardOpen(component.workPackageTable, row); + return false; + } + + this.noteKeyboardOpen(component.workPackageTable, row); super.openContextMenu( component.workPackageTable, evt, @@ -73,4 +84,16 @@ export class ContextMenuKeyboardHandler extends ContextMenuHandler { return false; } + + private isMenuChord(evt:KeyboardEvent, keyboardNavigation:boolean):boolean { + if (!keyboardNavigation) { + return evt.key === 'F10' && evt.shiftKey && evt.altKey; + } + + if (evt.key === 'ContextMenu') { + return true; + } + + return evt.key === 'F10' && evt.shiftKey && !evt.ctrlKey && !evt.metaKey; + } } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-rightclick-handler.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-rightclick-handler.ts index c6dc6541aece..212b15d58a7b 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-rightclick-handler.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-rightclick-handler.ts @@ -31,6 +31,7 @@ import { TableEventComponent } from 'core-app/features/work-packages/components/ import { tableRowClassName } from '../../builders/rows/single-row-builder'; import { timelineCellClassName } from '../../builders/timeline/timeline-row-builder'; import { uiStateLinkClass } from '../../builders/ui-state-link-builder'; +import { keyboardNavigationEnabled } from '../state/keyboard/roving-tabindex'; import { ContextMenuHandler } from './context-menu-handler'; import { EventType } from 'core-app/features/work-packages/routing/wp-view-base/event-handling/event-handler-registry'; @@ -39,8 +40,8 @@ export class ContextMenuRightClickHandler extends ContextMenuHandler { super(injector); } - public get EVENT():EventType { - return 'contextmenu'; // N.B.: contextmenu is not supported by Safari on iOS. + public get EVENT():EventType[] { + return ['contextmenu', 'pointerdown']; // N.B.: contextmenu is not supported by Safari on iOS. } public get SELECTOR() { @@ -55,8 +56,18 @@ export class ContextMenuRightClickHandler extends ContextMenuHandler { if (!view.workPackageTable.configuration.contextMenuEnabled) { return false; } + + if (evt.type === 'pointerdown') { + this.forgetKeyboardOpen(view.workPackageTable); + return true; + } + const target = evt.target as HTMLElement; + if (keyboardNavigationEnabled(view.workPackageTable.configuration) && this.isEditing(target)) { + return true; + } + // We want to keep the original context menu on hrefs // (currently, this is only the id if (target.closest(`.${uiStateLinkClass}`)) { @@ -64,6 +75,12 @@ export class ContextMenuRightClickHandler extends ContextMenuHandler { return true; } + if (this.consumeCompanion(view.workPackageTable, target)) { + evt.preventDefault(); + evt.stopPropagation(); + return false; + } + // Locate the row from event const element = target.closest(this.SELECTOR); From e2d957270d56529251aea1544fa5e8ed951fd667 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 20:27:52 +0100 Subject: [PATCH 7/8] [DREAM-894] Return focus to the table after menus Focus in the body-portal context menu marks focus as having left the table, so neither recovery nor returnFocus restored it after a menu was dismissed with Escape or closed by an action. OPContextMenuService.close now carries a reason (escape, action, outside or scroll) through to the handler's onClose. The row and column context menus capture their origin position at open and return focus through it only for Escape and action closes; a click outside or a scroll closes the menu as before without moving focus. Tables without keyboard navigation keep the previous behaviour. Adds returnFocusFromMenu to KeyboardNavigation: it runs the returnFocus logic with its userPosition guard while ignoring the left-table flag, and clears the flag and claims ownership for a pending render only when focus was actually returned. It never takes focus from a control outside the table and is a no-op once the table is destroyed. Lets Escape cancel a context menu that is still opening, so the keypress closes it instead of being refused; a click outside during the opening stays refused. The context menu overlay prevents the browser's own menu for any contextmenu inside it, so a keyboard companion contextmenu that lands on the focused menu item does not show the native menu over ours. Closing a row menu for any reason forgets the table's keyboard-open record, so a contextmenu on the same row right after the close opens the menu again instead of being swallowed as a companion. The record is noted after the menu is shown, so closing a menu it replaces does not forget it. https://community.openproject.org/wp/DREAM-894 --- .../context-menu/context-menu-entry.spec.ts | 316 ++++++++++++++++++ .../context-menu/context-menu-handler.ts | 6 +- .../context-menu-keyboard-handler.ts | 2 +- .../keyboard-navigation-transformer.spec.ts | 75 +++++ .../keyboard-navigation-transformer.ts | 29 +- .../op-columns-context-menu.directive.spec.ts | 100 ++++++ .../op-columns-context-menu.directive.ts | 14 + .../op-context-menu-handler.ts | 5 +- .../op-context-menu.component.ts | 2 +- .../op-context-menu.service.spec.ts | 175 ++++++++++ .../op-context-menu.service.ts | 15 +- .../op-context-menu/op-context-menu.types.ts | 2 + .../testing/register-context-menu.ts | 48 +++ .../wp-table-context-menu.directive.ts | 17 + .../table/keyboard_navigation_spec.rb | 16 +- 15 files changed, 806 insertions(+), 16 deletions(-) create mode 100644 frontend/src/app/shared/components/op-context-menu/op-context-menu.service.spec.ts create mode 100644 frontend/src/app/shared/components/op-context-menu/testing/register-context-menu.ts diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-entry.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-entry.spec.ts index 0c4827c183bf..17e53345c898 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-entry.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-entry.spec.ts @@ -26,13 +26,17 @@ //++ import { fireEvent, waitFor, within } from '@testing-library/dom'; +import { States } from 'core-app/core/states/states.service'; import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource'; +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 { 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 '../state/keyboard/navigable-rows'; +import { navigableCells } from '../state/keyboard/roving-tabindex'; import { uiStateLinkClass } from '../../builders/ui-state-link-builder'; describe('Context menu entry', () => { @@ -622,3 +626,315 @@ describe('Context menu keyboard chords', () => { }); }); }); + +describe('Context menu focus return', () => { + let harness:TableHarness; + let menu:OPContextMenuService; + let unregister:() => void; + let action:() => void; + let page:HTMLElement; + + const nextFrame = () => new Promise((resolve) => { requestAnimationFrame(() => resolve()); }); + + const mount = async (options:Partial = {}) => { + harness = buildTable({ + workPackages: [{ id: '1' }, { id: '2' }, { id: '3' }], + columns: ['id', 'subject', 'status'], + productionMarkup: true, + providers: [OPContextMenuService], + ...options, + configuration: { contextMenuEnabled: true, keyboardNavigationEnabled: true, ...options.configuration }, + }); + await harness.render(); + + page = document.createElement('div'); + page.id = 'wrapper'; + page.textContent = 'Blank page'; + document.body.append(page); + menu = harness.injector.get(OPContextMenuService); + unregister = registerContextMenu(menu); + action = () => undefined; + vi.spyOn(harness.injector.get(WorkPackageContextMenuHelperService), 'getPermittedActions') + .mockReturnValue([{ key: 'test-action', text: 'Test action', link: '#' }]); + vi.spyOn(WorkPackageTableContextMenu.prototype, 'triggerContextMenuAction').mockImplementation(() => action()); + }; + + const cellOf = (workPackageId:string, index:number) => navigableCells(harness.row(workPackageId))[index]; + + const resource = (id:string) => harness.injector.get(States).workPackages.get(id).value!; + + const openFromKeyboard = async (target:HTMLElement) => { + target.focus(); + fireEvent.keyDown(target, { key: 'F10', shiftKey: true }); + return waitFor(() => { + const item = document.querySelector('.op-context-menu--overlay .menu-item')!; + expect(item).toHaveFocus(); + return item; + }); + }; + + const openOnCell = (workPackageId:string, index:number) => { + harness.table.keyboardNavigation!.focusPosition({ key: wpOccurrenceKey(workPackageId), cell: index, desiredColumn: index }); + return openFromKeyboard(cellOf(workPackageId, index)); + }; + + const clickAction = async () => { + const item = await waitFor(() => within(document.querySelector('.op-context-menu--overlay')!) + .getByRole('menuitem', { name: 'Test action' })); + fireEvent.click(item); + }; + + afterEach(async () => { + await nextFrame(); + await nextFrame(); + await nextFrame(); + menu.close(); + unregister(); + page.remove(); + vi.restoreAllMocks(); + await harness.destroy(); + }); + + describe('on a capability-on table', () => { + beforeEach(() => mount()); + + it('row menu: an action that re-renders the row returns focus to the origin cell, not the body', async () => { + action = () => harness.table.refreshRows(resource('2')); + await openOnCell('2', 2); + + await clickAction(); + + expect(menu.active).toBeNull(); + expect(cellOf('2', 2)).toHaveFocus(); + }); + + it('Escape close returns to a row origin', async () => { + harness.table.keyboardNavigation!.focusPosition({ key: wpOccurrenceKey('2'), cell: null, desiredColumn: null }); + await openFromKeyboard(harness.row('2')); + + fireEvent.keyDown(document.activeElement!, { key: 'Escape' }); + + expect(menu.active).toBeNull(); + expect(harness.row('2')).toHaveFocus(); + }); + + it('Escape close returns to a cell origin', async () => { + await openOnCell('2', 1); + + fireEvent.keyDown(document.activeElement!, { key: 'Escape' }); + + expect(cellOf('2', 1)).toHaveFocus(); + }); + + it('Escape immediately after opening closes the menu, returns focus and keeps the selection', async () => { + harness.click('2'); + harness.table.keyboardNavigation!.focusPosition({ key: wpOccurrenceKey('2'), cell: 1, desiredColumn: 1 }); + fireEvent.keyDown(cellOf('2', 1), { key: 'F10', shiftKey: true }); + + fireEvent.keyDown(cellOf('2', 1), { key: 'Escape' }); + + expect(menu.active).toBeNull(); + await nextFrame(); + await nextFrame(); + await nextFrame(); + expect(document.querySelector('.op-context-menu--overlay .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()); + + fireEvent.click(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'); + + fireEvent.scroll(page); + + expect(menu.active).toBeNull(); + expect(document.body).toHaveFocus(); + expect(scrolled).not.toHaveBeenCalled(); + }); + + it('origin row removed by the action: focus lands on the reconciled neighbour', async () => { + action = () => { void harness.render([{ id: '1' }, { id: '3' }]); }; + await openOnCell('2', 1); + + const rendered = harness.nextRender(); + await clickAction(); + await rendered; + + await waitFor(() => expect(harness.row('3')).toHaveFocus()); + }); + + it('a newer external owner keeps focus', async () => { + const input = document.createElement('input'); + document.body.append(input); + try { + action = () => input.focus(); + await openOnCell('2', 1); + + await clickAction(); + + expect(input).toHaveFocus(); + } finally { + input.remove(); + } + }); + + it('pointer moved to another row while the menu was open: no return, not even when a pending render commits', async () => { + await openOnCell('2', 1); + + fireEvent.mouseDown(cellOf('3', 1), { button: 0 }); + const rendered = harness.render(); + fireEvent.keyDown(document.activeElement!, { key: 'Escape' }); + await rendered; + + expect(cellOf('3', 1)).not.toHaveFocus(); + expect(cellOf('2', 1)).not.toHaveFocus(); + expect(document.body).toHaveFocus(); + }); + + it('teardown with a menu open: close after destroy does not throw', async () => { + await openOnCell('2', 1); + + await harness.destroy(); + + expect(() => menu.close(true)).not.toThrow(); + expect(menu.active).toBeNull(); + expect(document.body).toHaveFocus(); + }); + + describe.each([ + ['Shift+F10', { key: 'F10', shiftKey: true }], + ['Menu', { key: 'ContextMenu' }], + ['Shift+Alt+F10', { key: 'F10', shiftKey: true, altKey: true }], + ] as [string, KeyboardEventInit][])('holding %s', (_name, chord) => { + it('opens one menu without errors and moves focus into it', async () => { + const errors:unknown[] = []; + const collect = (event:ErrorEvent) => errors.push(event.error); + window.addEventListener('error', collect); + const shown = vi.spyOn(menu, 'show'); + try { + harness.table.keyboardNavigation!.focusPosition({ key: wpOccurrenceKey('2'), cell: 1, desiredColumn: 1 }); + cellOf('2', 1).focus(); + + 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()); + expect(shown).toHaveBeenCalledTimes(1); + expect(document.querySelectorAll('.op-context-menu--overlay .dropdown')).toHaveLength(1); + expect(errors).toEqual([]); + } finally { + window.removeEventListener('error', collect); + } + }); + }); + + describe('companion contextmenu after a keyboard-opened menu closed', () => { + let clock:number; + let shown:ReturnType; + + beforeEach(() => { + clock = 1000; + vi.spyOn(performance, 'now').mockImplementation(() => clock); + shown = vi.spyOn(menu, 'show'); + }); + + const rightClickWithinWindow = () => { + clock += 100; + return fireEvent.contextMenu(harness.row('2')); + }; + + it('opens a bare same-row contextmenu after an Escape close', async () => { + harness.table.keyboardNavigation!.focusPosition({ key: wpOccurrenceKey('2'), cell: null, desiredColumn: null }); + await openFromKeyboard(harness.row('2')); + fireEvent.keyDown(document.activeElement!, { key: 'Escape' }); + + expect(rightClickWithinWindow()).toBe(false); + + expect(shown).toHaveBeenCalledTimes(2); + expect(menu.active).not.toBeNull(); + }); + + it('opens a bare same-row contextmenu after an action close', async () => { + harness.table.keyboardNavigation!.focusPosition({ key: wpOccurrenceKey('2'), cell: null, desiredColumn: null }); + await openFromKeyboard(harness.row('2')); + await clickAction(); + + rightClickWithinWindow(); + + expect(shown).toHaveBeenCalledTimes(2); + expect(menu.active).not.toBeNull(); + }); + + 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()); + + fireEvent.keyDown(harness.row('2'), { key: 'F10', shiftKey: true }); + + expect(rightClickWithinWindow()).toBe(false); + expect(shown).toHaveBeenCalledTimes(2); + }); + }); + + it('Escape with a menu open closes only the menu; the selection survives', async () => { + harness.click('1'); + harness.click('2', { shiftKey: true }); + await openOnCell('2', 1); + + fireEvent.keyDown(document.activeElement!, { key: 'Escape' }); + + expect(menu.active).toBeNull(); + expect(harness.selection.getSelectedWorkPackageIds()).toEqual(['1', '2']); + expect(cellOf('2', 1)).toHaveFocus(); + }); + }); + + describe('with a hierarchy', () => { + beforeEach(() => mount({ + workPackages: [{ id: '1', children: [{ id: '2' }] }, { id: '2', ancestors: [{ id: '1' }] }, { id: '3' }], + showHierarchies: true, + })); + + it('origin hidden by a collapse: focus lands on the row hiding it', async () => { + action = () => harness.injector.get(WorkPackageViewHierarchiesService).collapse('1'); + await openOnCell('2', 1); + + await clickAction(); + + await waitFor(() => expect(harness.row('1')).toHaveFocus()); + }); + }); + + describe('on a capability-off table', () => { + beforeEach(() => mount({ productionMarkup: false, configuration: { keyboardNavigationEnabled: false } })); + + it('menu close behaves as before: an action leaves focus alone, Escape focuses the trigger', async () => { + const openByRightClick = () => { + fireEvent.contextMenu(harness.row('2')); + return waitFor(() => expect(document.querySelector('.op-context-menu--overlay .menu-item')).toHaveFocus()); + }; + await openByRightClick(); + await clickAction(); + + expect(document.body).toHaveFocus(); + + await openByRightClick(); + fireEvent.keyDown(document.activeElement!, { key: 'Escape' }); + + expect(harness.table.keyboardNavigation).toBeNull(); + expect(menu.active).toBeNull(); + expect(harness.row('2')).toHaveFocus(); + }); + }); +}); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts index 78311bf6e97f..e54e13ec47b6 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler.ts @@ -46,6 +46,10 @@ export const companionContextMenuWindowMs = 500; const keyboardOpens = new WeakMap(); const forgetOnTeardown = new WeakSet(); +export function forgetKeyboardMenuOpen(table:WorkPackageTable):void { + keyboardOpens.delete(table); +} + export abstract class ContextMenuHandler implements TableEventHandler { // Injections @LazyInject() public opContextMenu:OPContextMenuService; @@ -93,7 +97,7 @@ export abstract class ContextMenuHandler implements TableEventHandler { } protected forgetKeyboardOpen(table:WorkPackageTable):void { - keyboardOpens.delete(table); + forgetKeyboardMenuOpen(table); } /** True once for the first `contextmenu` on `target` within the window after a keyboard open of its row; clears the record. */ diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-keyboard-handler.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-keyboard-handler.ts index 340ed516531d..541d233a1f92 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-keyboard-handler.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-keyboard-handler.ts @@ -73,7 +73,6 @@ export class ContextMenuKeyboardHandler extends ContextMenuHandler { return false; } - this.noteKeyboardOpen(component.workPackageTable, row); super.openContextMenu( component.workPackageTable, evt, @@ -81,6 +80,7 @@ export class ContextMenuKeyboardHandler extends ContextMenuHandler { // Set position args to open at element { placement: 'bottom-start', reference: target } ); + this.noteKeyboardOpen(component.workPackageTable, row); return false; } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts index 008075204e26..46341ac3134e 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts @@ -571,6 +571,81 @@ describe('KeyboardNavigationTransformer', () => { expect(input).toHaveFocus(); }); + describe('returnFocusFromMenu', () => { + const origin = { key: wpOccurrenceKey('2'), cell: 1, desiredColumn: 1 }; + + const visitMenu = (table:TableHarness) => { + const menuItem = document.createElement('button'); + document.body.append(menuItem); + cell(table.row('2'), 1).focus(); + menuItem.focus(); + return menuItem; + }; + + it('focuses the origin again after focus visited the menu portal', async () => { + const table = await mount(); + visitMenu(table).remove(); + + navigation(table).returnFocus(origin); + expect(document.body).toHaveFocus(); + + navigation(table).returnFocusFromMenu(origin); + expect(cell(table.row('2'), 1)).toHaveFocus(); + }); + + it('restores the origin after a full render the menu action started', async () => { + const table = await mount(); + const menuItem = visitMenu(table); + const rendered = table.nextRender(); + table.table.redrawTable(); + menuItem.remove(); + + navigation(table).returnFocusFromMenu(origin); + await rendered; + + expect(cell(table.row('2'), 1)).toHaveFocus(); + }); + + it('leaves focus on a control outside the table', async () => { + const table = await mount(); + const menuItem = visitMenu(table); + const input = document.createElement('input'); + document.body.append(input); + try { + menuItem.remove(); + input.focus(); + + navigation(table).returnFocusFromMenu(origin); + + expect(input).toHaveFocus(); + } finally { + input.remove(); + } + }); + + it('keeps focus outside the table for later returns when the user moved elsewhere meanwhile', async () => { + const table = await mount(); + visitMenu(table).remove(); + const moved = { key: wpOccurrenceKey('3'), cell: 1, desiredColumn: 1 }; + navigation(table).recordPosition(moved); + + navigation(table).returnFocusFromMenu(origin); + navigation(table).returnFocus(moved); + + expect(document.body).toHaveFocus(); + }); + + it('does nothing after the table is destroyed', async () => { + const table = await mount(); + const keyboardNavigation = navigation(table); + visitMenu(table).remove(); + await table.destroy(); + + expect(() => keyboardNavigation.returnFocusFromMenu(origin)).not.toThrow(); + expect(document.body).toHaveFocus(); + }); + }); + it('consumes Home on the first cell without moving', async () => { const table = await mount(); cell(table.row('1'), 0).focus(); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.ts index 238058701d2b..3f75453e5c63 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.ts @@ -62,6 +62,8 @@ export interface KeyboardNavigation { recordPosition(position:FocusPosition):void; /** Focuses the origin again, or the row hiding it, unless the user has since moved elsewhere, an editor is open or focus rests on a live control. */ returnFocus(origin:FocusPosition):void; + /** Returns focus to `origin` after a menu closed, though focus visited the menu portal; same userPosition guard as returnFocus, and never takes focus from a control outside the table. */ + returnFocusFromMenu(origin:FocusPosition):void; /** The position of the navigable cell or row enclosing `cell`, or null when it is not a rendered occurrence. */ positionOfCell(cell:HTMLElement):FocusPosition|null; /** The row or cell element at the position, or null when it is not rendered. */ @@ -221,17 +223,38 @@ export class KeyboardNavigationTransformer implements KeyboardNavigation { } public returnFocus(origin:FocusPosition):void { + this.restoreFocus(origin); + } + + public returnFocusFromMenu(origin:FocusPosition):void { + if (this.table.destroyed || !this.focusAtRest()) { + return; + } + + const focusLeftTable = this.focusLeftTable; + this.focusLeftTable = false; + if (this.restoreFocus(origin)) { + this.owned = true; + } else { + this.focusLeftTable = focusLeftTable; + } + } + + private restoreFocus(origin:FocusPosition):boolean { const user = this.userPosition; if (user && (user.key !== origin.key || user.cell !== origin.cell)) { - return; + return false; } const rows = buildNavigableRows(this.table); const originIndex = rows.findIndex((row) => row.key === origin.key); const target = originIndex === -1 ? this.current : reconcilePosition(rows, origin, originIndex); - if (target && this.mayRestoreFocus()) { - this.focusPosition(target); + if (!target || !this.mayRestoreFocus()) { + return false; } + + this.focusPosition(target); + return true; } public positionOfCell(cell:Element):FocusPosition|null { diff --git a/frontend/src/app/shared/components/op-context-menu/handlers/op-columns-context-menu.directive.spec.ts b/frontend/src/app/shared/components/op-context-menu/handlers/op-columns-context-menu.directive.spec.ts index c862c3db4e12..ec91223a7cda 100644 --- a/frontend/src/app/shared/components/op-context-menu/handlers/op-columns-context-menu.directive.spec.ts +++ b/frontend/src/app/shared/components/op-context-menu/handlers/op-columns-context-menu.directive.spec.ts @@ -26,6 +26,7 @@ //++ import { ElementRef, Injector, runInInjectionContext } from '@angular/core'; +import { fireEvent, waitFor, within } from '@testing-library/dom'; import { I18nService } from 'core-app/core/i18n/i18n.service'; import { QueryColumn } from 'core-app/features/work-packages/components/wp-query/query-column'; import { WorkPackageViewColumnsService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-columns.service'; @@ -35,6 +36,9 @@ import { WorkPackageViewSortByService } from 'core-app/features/work-packages/ro import { OpModalService } from 'core-app/shared/components/modal/modal.service'; import { ConfirmDialogService } from 'core-app/shared/components/modals/confirm-dialog/confirm-dialog.service'; import { OPContextMenuService } from 'core-app/shared/components/op-context-menu/op-context-menu.service'; +import { registerContextMenu } from 'core-app/shared/components/op-context-menu/testing/register-context-menu'; +import { headerRowKey } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/navigable-rows'; +import { buildTable, TableHarness } from 'core-app/features/work-packages/components/wp-fast-table/testing/table-harness'; import { OpColumnsContextMenu } from './op-columns-context-menu.directive'; class TestColumnsMenu extends OpColumnsContextMenu { @@ -90,3 +94,99 @@ describe('OpColumnsContextMenu', () => { expect(menu.focusTarget).toBe(document.getElementById('trigger')); }); }); + +describe('OpColumnsContextMenu on a capability-on table', () => { + let harness:TableHarness; + let service:OPContextMenuService; + let unregister:() => void; + let page:HTMLElement; + + const nextFrame = () => new Promise((resolve) => { requestAnimationFrame(() => resolve()); }); + + beforeEach(async () => { + harness = buildTable({ + workPackages: [{ id: '1' }, { id: '2' }], + columns: ['id', 'subject', 'status'], + productionMarkup: true, + providers: [OPContextMenuService], + configuration: { columnMenuEnabled: true, keyboardNavigationEnabled: true }, + }); + await harness.render(); + page = document.createElement('div'); + page.id = 'wrapper'; + page.textContent = 'Blank page'; + document.body.append(page); + service = harness.injector.get(OPContextMenuService); + unregister = registerContextMenu(service); + }); + + afterEach(async () => { + await nextFrame(); + await nextFrame(); + await nextFrame(); + service.close(); + unregister(); + page.remove(); + await harness.destroy(); + }); + + const subjectHeader = () => harness.headerCells()[1]; + + const mountMenu = () => { + const injector = Injector.create({ + providers: [ + { provide: ElementRef, useValue: new ElementRef(subjectHeader()) }, + { provide: I18nService, useValue: { t: (key:string) => key } }, + { provide: OpModalService, useValue: {} }, + { provide: ConfirmDialogService, useValue: {} }, + ], + parent: harness.injector, + }); + const menu = runInInjectionContext(injector, () => new OpColumnsContextMenu()); + menu.column = harness.injector.get(WorkPackageViewColumnsService).getColumns().find((column) => column.id === 'subject')!; + menu.table = harness.table; + menu.ngAfterViewInit(); + return menu; + }; + + const openMenu = async () => { + mountMenu(); + harness.table.keyboardNavigation!.focusPosition({ key: headerRowKey, cell: 1, desiredColumn: 1 }); + fireEvent.click(subjectHeader()); + await waitFor(() => expect(document.querySelector('.op-context-menu--overlay .menu-item')).toHaveFocus()); + return within(document.querySelector('.op-context-menu--overlay')!); + }; + + it('column menu: an action (move column) returns focus to the header cell, not the body', async () => { + const overlay = await openMenu(); + + fireEvent.click(overlay.getByRole('menuitem', { name: 'js.work_packages.query.move_column_right' })); + + expect(service.active).toBeNull(); + expect(subjectHeader()).toHaveFocus(); + }); + + it('Escape immediately after opening closes the menu and returns focus to the header cell', async () => { + mountMenu(); + harness.table.keyboardNavigation!.focusPosition({ key: headerRowKey, cell: 1, desiredColumn: 1 }); + fireEvent.click(subjectHeader()); + + fireEvent.keyDown(subjectHeader(), { key: 'Escape' }); + + expect(service.active).toBeNull(); + await nextFrame(); + await nextFrame(); + await nextFrame(); + expect(document.querySelector('.op-context-menu--overlay .menu-item')).toBeNull(); + expect(subjectHeader()).toHaveFocus(); + }); + + it('a click outside closes the menu without returning focus to the header cell', async () => { + await openMenu(); + + fireEvent.click(page); + + expect(service.active).toBeNull(); + expect(document.body).toHaveFocus(); + }); +}); diff --git a/frontend/src/app/shared/components/op-context-menu/handlers/op-columns-context-menu.directive.ts b/frontend/src/app/shared/components/op-context-menu/handlers/op-columns-context-menu.directive.ts index f1c3b84f3bb4..6e03590fb73e 100644 --- a/frontend/src/app/shared/components/op-context-menu/handlers/op-columns-context-menu.directive.ts +++ b/frontend/src/app/shared/components/op-context-menu/handlers/op-columns-context-menu.directive.ts @@ -40,6 +40,8 @@ import { WpTableConfigurationModalComponent } from 'core-app/features/work-packa import { QUERY_SORT_BY_ASC, QUERY_SORT_BY_DESC } from 'core-app/features/hal/resources/query-sort-by-resource'; import { ConfirmDialogService } from 'core-app/shared/components/modals/confirm-dialog/confirm-dialog.service'; import { computePosition, ComputePositionReturn, flip, shift } from '@floating-ui/dom'; +import { FocusPosition } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position'; +import { OpContextMenuCloseReason } from 'core-app/shared/components/op-context-menu/op-context-menu.types'; @Directive({ selector: '[opColumnsContextMenu]', @@ -61,6 +63,8 @@ export class OpColumnsContextMenu extends OpContextMenuTrigger { // eslint-disable-next-line @angular-eslint/no-input-rename @Input('opColumnsContextMenu-table') public table:WorkPackageTable; + private origin:FocusPosition|null = null; + public text = { confirmDelete: { text: this.I18n.t('js.work_packages.table_configuration.sorting_mode.warning'), @@ -73,9 +77,19 @@ export class OpColumnsContextMenu extends OpContextMenuTrigger { return; } this.buildItems(); + this.origin = this.table.keyboardNavigation?.positionOfCell(this.element) ?? null; this.opContextMenu.show(this, evt); } + public override onClose(focus = true, reason?:OpContextMenuCloseReason):void { + if (this.origin && (reason === 'escape' || reason === 'action')) { + this.table.keyboardNavigation?.returnFocusFromMenu(this.origin); + return; + } + + super.onClose(focus); + } + public get locals() { return { showAnchorRight: this.column && this.column.id !== 'id', diff --git a/frontend/src/app/shared/components/op-context-menu/op-context-menu-handler.ts b/frontend/src/app/shared/components/op-context-menu/op-context-menu-handler.ts index 9ee168357f79..ad0553a0241d 100644 --- a/frontend/src/app/shared/components/op-context-menu/op-context-menu-handler.ts +++ b/frontend/src/app/shared/components/op-context-menu/op-context-menu-handler.ts @@ -27,7 +27,7 @@ import { computePosition, ComputePositionReturn, flip, Placement, shift } from '@floating-ui/dom'; import { OPContextMenuService } from 'core-app/shared/components/op-context-menu/op-context-menu.service'; -import { OpContextMenuItem } from 'core-app/shared/components/op-context-menu/op-context-menu.types'; +import { OpContextMenuCloseReason, OpContextMenuItem } from 'core-app/shared/components/op-context-menu/op-context-menu.types'; import { UntilDestroyedMixin } from 'core-app/shared/helpers/angular/until-destroyed.mixin'; /** @@ -49,8 +49,9 @@ export abstract class OpContextMenuHandler extends UntilDestroyedMixin { * Called when the service closes this context menu * * @param focus Focus on the trigger again + * @param _reason Why the service closed the menu */ - public onClose(focus = true) { + public onClose(focus = true, _reason?:OpContextMenuCloseReason) { if (focus) { this.afterFocusOn.focus(); } diff --git a/frontend/src/app/shared/components/op-context-menu/op-context-menu.component.ts b/frontend/src/app/shared/components/op-context-menu/op-context-menu.component.ts index 88de8261a1ac..20c4772162f2 100644 --- a/frontend/src/app/shared/components/op-context-menu/op-context-menu.component.ts +++ b/frontend/src/app/shared/components/op-context-menu/op-context-menu.component.ts @@ -59,7 +59,7 @@ export class OPContextMenuComponent { } if (item.onClick!(event)) { - this.locals.service!.close(); + this.locals.service!.close(false, 'action'); event.preventDefault(); event.stopPropagation(); return false; diff --git a/frontend/src/app/shared/components/op-context-menu/op-context-menu.service.spec.ts b/frontend/src/app/shared/components/op-context-menu/op-context-menu.service.spec.ts new file mode 100644 index 000000000000..5d1545a599c3 --- /dev/null +++ b/frontend/src/app/shared/components/op-context-menu/op-context-menu.service.spec.ts @@ -0,0 +1,175 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { TestBed } from '@angular/core/testing'; +import { fireEvent, waitFor } from '@testing-library/dom'; +import { computePosition, ComputePositionReturn } from '@floating-ui/dom'; +import { OpContextMenuHandler } from 'core-app/shared/components/op-context-menu/op-context-menu-handler'; +import { OPContextMenuService } from 'core-app/shared/components/op-context-menu/op-context-menu.service'; +import { registerContextMenu } from 'core-app/shared/components/op-context-menu/testing/register-context-menu'; + +class TriggerMenu extends OpContextMenuHandler { + constructor(service:OPContextMenuService, protected element:HTMLElement) { + super(service); + this.items = [{ linkText: 'Action', onClick: () => true }]; + } + + public computePosition(floating:HTMLElement):Promise { + return computePosition(this.element, floating); + } +} + +const nextFrame = () => new Promise((resolve) => { requestAnimationFrame(() => resolve()); }); + +describe('OPContextMenuService', () => { + let service:OPContextMenuService; + let wrapper:HTMLElement; + let trigger:HTMLButtonElement; + let unregister:() => void; + + beforeEach(() => { + wrapper = document.createElement('div'); + wrapper.id = 'wrapper'; + trigger = document.createElement('button'); + trigger.textContent = 'Trigger'; + wrapper.append(trigger); + document.body.append(wrapper); + + service = TestBed.inject(OPContextMenuService); + unregister = registerContextMenu(service); + }); + + afterEach(async () => { + await nextFrame(); + await nextFrame(); + await nextFrame(); + service.close(); + unregister(); + wrapper.remove(); + }); + + const open = () => { + const menu = new TriggerMenu(service, trigger); + trigger.focus(); + service.show(menu, new MouseEvent('click')); + return menu; + }; + + const overlay = () => document.querySelector('.op-context-menu--overlay')!; + + it('Escape immediately after opening closes the menu and returns focus', async () => { + const menu = open(); + const onOpen = vi.spyOn(menu, 'onOpen'); + + fireEvent.keyDown(window, { key: 'Escape' }); + + expect(service.active).toBeNull(); + expect(overlay()).not.toBeVisible(); + expect(overlay().querySelector('.dropdown')).toBeNull(); + await nextFrame(); + await nextFrame(); + await nextFrame(); + expect(onOpen).not.toHaveBeenCalled(); + expect(trigger).toHaveFocus(); + }); + + it('refuses a click outside the menu while it is opening', async () => { + open(); + + fireEvent.click(wrapper); + + expect(service.active).not.toBeNull(); + await waitFor(() => expect(overlay().querySelector('.menu-item')).toHaveFocus()); + + fireEvent.click(wrapper); + + expect(service.active).toBeNull(); + }); + + it('Escape on an open menu closes it and returns focus to the trigger', async () => { + open(); + await waitFor(() => expect(overlay().querySelector('.menu-item')).toHaveFocus()); + + fireEvent.keyDown(window, { key: 'Escape' }); + + expect(service.active).toBeNull(); + expect(trigger).toHaveFocus(); + }); + + it('prevents the native menu over an open menu', async () => { + const menu = open(); + const item = await waitFor(() => { + const element = overlay().querySelector('.menu-item')!; + expect(element).toHaveFocus(); + return element; + }); + + expect(fireEvent.contextMenu(item)).toBe(false); + + expect(service.active).toBe(menu); + }); + + describe('close reason', () => { + const openAndSpy = async () => { + const menu = open(); + await waitFor(() => expect(overlay().querySelector('.menu-item')).toHaveFocus()); + return vi.spyOn(menu, 'onClose'); + }; + + it('is escape for Escape', async () => { + const onClose = await openAndSpy(); + + fireEvent.keyDown(window, { key: 'Escape' }); + + expect(onClose).toHaveBeenCalledWith(true, 'escape'); + }); + + it('is action for a chosen item', async () => { + const onClose = await openAndSpy(); + + fireEvent.click(overlay().querySelector('.menu-item')!); + + expect(onClose).toHaveBeenCalledWith(false, 'action'); + }); + + it('is outside for a click outside the menu', async () => { + const onClose = await openAndSpy(); + + fireEvent.click(wrapper); + + expect(onClose).toHaveBeenCalledWith(false, 'outside'); + }); + + it('is scroll for a scroll outside the menu', async () => { + const onClose = await openAndSpy(); + + fireEvent.scroll(trigger); + + expect(onClose).toHaveBeenCalledWith(false, 'scroll'); + }); + }); +}); diff --git a/frontend/src/app/shared/components/op-context-menu/op-context-menu.service.ts b/frontend/src/app/shared/components/op-context-menu/op-context-menu.service.ts index 53d312568ca8..2ab6237b5128 100644 --- a/frontend/src/app/shared/components/op-context-menu/op-context-menu.service.ts +++ b/frontend/src/app/shared/components/op-context-menu/op-context-menu.service.ts @@ -29,6 +29,7 @@ import { ApplicationRef, Injectable, Injector, inject } from '@angular/core'; import { ComponentPortal, ComponentType, DomPortalOutlet } from '@angular/cdk/portal'; import { OpContextMenuHandler } from 'core-app/shared/components/op-context-menu/op-context-menu-handler'; import { + OpContextMenuCloseReason, OpContextMenuLocalsMap, OpContextMenuLocalsToken, } from 'core-app/shared/components/op-context-menu/op-context-menu.types'; @@ -60,6 +61,7 @@ export class OPContextMenuService { const hostElement = this.portalHostElement = document.createElement('div'); hostElement.classList.add('op-context-menu--overlay'); document.body.appendChild(hostElement); + hostElement.addEventListener('contextmenu', (evt) => evt.preventDefault()); this.bodyPortalHost = new DomPortalOutlet( hostElement, @@ -70,7 +72,10 @@ export class OPContextMenuService { // Listen to keyups on window to close context menus window.addEventListener('keydown', (evt) => { if (this.active && evt.key === 'Escape') { - this.close(true); + // Escape may cancel a pending open; a click outside may not, as the opening click itself bubbles to the wrapper. + this.openSeq += 1; + this.isOpening = false; + this.close(true, 'escape'); } return true; @@ -81,13 +86,13 @@ export class OPContextMenuService { // Listen to any click and close the active context menu wrapper.addEventListener('click', (evt:Event) => { if (this.active && !this.portalHostElement.contains(evt.target as Element)) { - this.close(); + this.close(false, 'outside'); } }); // Listen if it scrolles then close the active context menu wrapper.addEventListener('scroll', (evt:Event) => { if (this.active && !this.portalHostElement.contains(evt.target as Element)) { - this.close(); + this.close(false, 'scroll'); } }, true); } @@ -152,7 +157,7 @@ export class OPContextMenuService { /** * Closes all currently open context menus. */ - public close(focus = false):void { + public close(focus = false, reason?:OpContextMenuCloseReason):void { if (this.isOpening) { return; } @@ -160,7 +165,7 @@ export class OPContextMenuService { // Detach any component currently in the portal this.bodyPortalHost.detach(); this.portalHostElement.style.display = 'none'; - this.active?.onClose(focus); + this.active?.onClose(focus, reason); this.active = null; } diff --git a/frontend/src/app/shared/components/op-context-menu/op-context-menu.types.ts b/frontend/src/app/shared/components/op-context-menu/op-context-menu.types.ts index bad7841c321b..0297075a1034 100644 --- a/frontend/src/app/shared/components/op-context-menu/op-context-menu.types.ts +++ b/frontend/src/app/shared/components/op-context-menu/op-context-menu.types.ts @@ -28,6 +28,8 @@ import { InjectionToken } from '@angular/core'; import type { OPContextMenuService } from 'core-app/shared/components/op-context-menu/op-context-menu.service'; +export type OpContextMenuCloseReason = 'escape'|'action'|'outside'|'scroll'; + export const OpContextMenuLocalsToken = new InjectionToken('CONTEXT_MENU_LOCALS'); export interface OpContextMenuItem { diff --git a/frontend/src/app/shared/components/op-context-menu/testing/register-context-menu.ts b/frontend/src/app/shared/components/op-context-menu/testing/register-context-menu.ts new file mode 100644 index 000000000000..9e5ba623dec9 --- /dev/null +++ b/frontend/src/app/shared/components/op-context-menu/testing/register-context-menu.ts @@ -0,0 +1,48 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { OPContextMenuService } from 'core-app/shared/components/op-context-menu/op-context-menu.service'; + +/** Registers the service's overlay and window listeners as the application does; the returned cleanup removes both. */ +export function registerContextMenu(service:OPContextMenuService):() => void { + const listeners:[string, EventListenerOrEventListenerObject][] = []; + const addEventListener = window.addEventListener.bind(window); + const spy = vi.spyOn(window, 'addEventListener').mockImplementation((type:string, listener:EventListenerOrEventListenerObject, options?:boolean|AddEventListenerOptions) => { + listeners.push([type, listener]); + addEventListener(type, listener, options); + }); + try { + service.register(); + } finally { + spy.mockRestore(); + } + + return () => { + listeners.forEach(([type, listener]) => window.removeEventListener(type, listener)); + document.querySelector('.op-context-menu--overlay')?.remove(); + }; +} diff --git a/frontend/src/app/shared/components/op-context-menu/wp-context-menu/wp-table-context-menu.directive.ts b/frontend/src/app/shared/components/op-context-menu/wp-context-menu/wp-table-context-menu.directive.ts index 69e280a71cbd..8dd0dc4fa1fb 100644 --- a/frontend/src/app/shared/components/op-context-menu/wp-context-menu/wp-table-context-menu.directive.ts +++ b/frontend/src/app/shared/components/op-context-menu/wp-context-menu/wp-table-context-menu.directive.ts @@ -32,16 +32,33 @@ import { PositionArgs, WorkPackageViewContextMenu } from 'core-app/shared/compon import { WorkPackageViewHierarchyIdentationService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-hierarchy-indentation.service'; import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; import { keyboardNavigationEnabled } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex'; +import { FocusPosition } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position'; +import { OpContextMenuCloseReason } from 'core-app/shared/components/op-context-menu/op-context-menu.types'; +import { forgetKeyboardMenuOpen } from 'core-app/features/work-packages/components/wp-fast-table/handlers/context-menu/context-menu-handler'; export class WorkPackageTableContextMenu extends WorkPackageViewContextMenu { @LazyInject() wpViewIndentation:WorkPackageViewHierarchyIdentationService; + private readonly origin:FocusPosition|null; + constructor(public injector:Injector, protected workPackageId:string, protected element:HTMLElement, additionalPositionArgs:PositionArgs, protected table:WorkPackageTable) { super(injector, workPackageId, element, additionalPositionArgs, true); + this.origin = table.keyboardNavigation?.positionOfCell(element) ?? null; + } + + public override onClose(focus = true, reason?:OpContextMenuCloseReason):void { + forgetKeyboardMenuOpen(this.table); + + if (this.origin && (reason === 'escape' || reason === 'action')) { + this.table.keyboardNavigation?.returnFocusFromMenu(this.origin); + return; + } + + super.onClose(focus); } protected override get afterFocusOn():HTMLElement { diff --git a/spec/features/work_packages/table/keyboard_navigation_spec.rb b/spec/features/work_packages/table/keyboard_navigation_spec.rb index 30febe59b2c3..f613f47d5263 100644 --- a/spec/features/work_packages/table/keyboard_navigation_spec.rb +++ b/spec/features/work_packages/table/keyboard_navigation_spec.rb @@ -78,9 +78,6 @@ end expect(page).to have_css(".generic-table--sort-header #subject.asc") wp_table.expect_work_package_order(sibling, parent, child) - - # The column menu drops focus to body after an action until DREAM-894. - wp_table.tab_into_table wp_table.expect_focus_on_column_header("Subject") wp_table.press_keys(:down) @@ -90,6 +87,19 @@ wp_table.expect_focus_outside_table end + it "closes a column menu with Escape and keeps the header focused" do + wp_table.tab_into_table + wp_table.press_keys(:right, :right, :up) + wp_table.expect_focus_on_column_header("Subject") + + wp_table.press_keys(:enter) + expect(page).to have_selector(:menuitem, focused: true) + + wp_table.press_keys(:escape) + expect(page).to have_no_css("#column-context-menu") + wp_table.expect_focus_on_column_header("Subject") + end + it "leaves the table forward with one Tab from the last row" do wp_table.tab_into_table wp_table.press_keys(:down, :down) From 2bf593be82f93c44e9686d981a49efbf1c655c9d Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 20:52:00 +0100 Subject: [PATCH 8/8] [DREAM-894] Cover keyboard selection end to end Adds Components::LiveRegion, a Capybara support component that reads Primer's polite live region through its shadow root and offers a synchronized announcement expectation. Adds a feature spec for the work package table covering Shift+Space toggling, Shift+Arrow ranges, Select All, Escape, Shift+F10 context menus with focus return, mixed pointer and keyboard gestures and the selection announcements, in flat, grouped and hierarchy tables. https://community.openproject.org/wp/DREAM-894 --- .../keyboard-navigation-transformer.spec.ts | 15 + .../table/keyboard_selection_spec.rb | 543 ++++++++++++++++++ spec/support/components/live_region.rb | 46 ++ .../work_packages/work_packages_table.rb | 7 + 4 files changed, 611 insertions(+) create mode 100644 spec/features/work_packages/table/keyboard_selection_spec.rb create mode 100644 spec/support/components/live_region.rb diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts index 46341ac3134e..e84165eb75dd 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/keyboard-navigation-transformer.spec.ts @@ -1106,6 +1106,21 @@ describe('KeyboardNavigationTransformer', () => { expect(selectedIds(table)).toEqual(['1', '2', '3']); }); + it.each([ + ['Ctrl+D', { code: 'KeyD', ctrlKey: true }], + ['Meta+D', { code: 'KeyD', metaKey: true }], + ])('leave %s to the browser on a capability-on table', async (_chord, init) => { + const table = await mount(); + table.row('1').focus(); + expect(pressActive(' ', { shiftKey: true })).toBe(false); + expect(selectedIds(table)).toEqual(['1']); + + expect(pressActive('d', init)).toBe(true); + + expect(selectedIds(table)).toEqual(['1']); + expect(table.row('1')).toHaveFocus(); + }); + it('still select all through the legacy registration on a capability-off table', async () => { const table = await mount({ configuration: { keyboardNavigationEnabled: false } }); const selectAll = vi.spyOn(table.selection, 'selectAll'); diff --git a/spec/features/work_packages/table/keyboard_selection_spec.rb b/spec/features/work_packages/table/keyboard_selection_spec.rb new file mode 100644 index 000000000000..e72c03f2143f --- /dev/null +++ b/spec/features/work_packages/table/keyboard_selection_spec.rb @@ -0,0 +1,543 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe "Work package table keyboard selection", :js do + let(:project) { create(:project) } + let(:user) do + create(:user, + member_with_permissions: { + project => %i[view_work_packages edit_work_packages add_work_packages delete_work_packages] + }) + end + + let(:wp_table) { Pages::WorkPackagesTable.new(project) } + let(:context_menu) { Components::WorkPackages::ContextMenu.new } + let(:destroy_modal) { Components::WorkPackages::DestroyModal.new(bulk_mode: true) } + let(:live_region) { Components::LiveRegion.new } + let(:filters) { Components::WorkPackages::Filters.new } + let(:select_all_chord) { [BrowserPlatform.multi_select_modifier(page), "a"] } + + before do + login_as(user) + end + + def expect_selection(*selected) + listed.each do |work_package| + expect(wp_table.treegrid) + .to have_selector(:row, work_package.subject, aria: { selected: selected.include?(work_package) }) + end + end + + context "in a flat table" do + let!(:alpha) { create(:work_package, project:, subject: "Alpha task") } + let!(:bravo) { create(:work_package, project:, subject: "Bravo task") } + let!(:charlie) { create(:work_package, project:, subject: "Charlie task") } + let!(:delta) { create(:work_package, project:, subject: "Delta task") } + let!(:echo) { create(:work_package, project:, subject: "Echo task") } + let(:listed) { [alpha, bravo, charlie, delta, echo] } + let!(:query) do + create(:query, user:, project:, column_names: %w[id subject], + sort_criteria: [%w[subject asc]], show_hierarchies: false) + end + + before do + wp_table.visit_query(query) + wp_table.expect_work_package_order(*listed) + wp_table.expect_inline_create_button + end + + # Backlogs counterpart: modules/backlogs/spec/features/work_packages/batch_selection_spec.rb:154 + # Divergence: the table toggles with Shift+Space, Backlogs with plain Space. + it "toggles with Shift+Space from row and cell focus, and deselecting re-bases the anchor" do + wp_table.focus_row_by_keyboard(bravo) + wp_table.press_keys(:space) + expect_selection + wp_table.expect_focus_on_row(bravo) + + wp_table.press_keys(%i[shift space]) + expect_selection(bravo) + live_region.expect_announcement("1 work package selected.") + + wp_table.press_keys(:right, :right) + wp_table.expect_focus_on_cell(bravo, :subject) + wp_table.press_keys(%i[shift space]) + expect_selection + live_region.expect_announcement("Selection cleared.") + + # Backlogs counterpart: modules/backlogs/spec/features/work_packages/batch_selection_spec.rb:98 + wp_table.press_keys(:down) + wp_table.press_keys(%i[shift space]) + wp_table.press_keys(%i[shift space]) + expect_selection + + wp_table.press_keys(%i[shift down]) + expect_selection(charlie, delta) + end + + # Backlogs counterpart: modules/backlogs/spec/features/work_packages/batch_selection_spec.rb:111,190 + it "extends, shrinks and reverses a range from a fixed anchor with Shift+ArrowDown and Shift+ArrowUp" do + wp_table.focus_row_by_keyboard(charlie) + wp_table.press_keys(%i[shift space]) + + wp_table.press_keys(%i[shift down], %i[shift down]) + wp_table.expect_focus_on_row(echo) + expect_selection(charlie, delta, echo) + + wp_table.press_keys(%i[shift up]) + expect_selection(charlie, delta) + + wp_table.press_keys(%i[shift up], %i[shift up], %i[shift up]) + wp_table.expect_focus_on_row(alpha) + expect_selection(alpha, bravo, charlie) + end + + # Backlogs counterpart: modules/backlogs/spec/features/work_packages/batch_selection_spec.rb:181 + it "selects every work package on the page with Ctrl/Cmd+A from a row and from the column header" do + wp_table.focus_row_by_keyboard(bravo) + wp_table.press_keys(select_all_chord) + expect_selection(*listed) + + wp_table.press_keys(:escape) + expect_selection + + wp_table.press_keys(:up, :up, :right) + wp_table.expect_focus_on_column_header("ID") + wp_table.press_keys(select_all_chord) + expect_selection(*listed) + end + + it "collapses to the first row when Shift+ArrowDown follows a header Select All" do + wp_table.tab_into_table + wp_table.press_keys(%i[control home]) + wp_table.press_keys(select_all_chord) + expect_selection(*listed) + + wp_table.press_keys(%i[shift down]) + + wp_table.expect_focus_on_row(alpha) + expect_selection(alpha) + end + + it "does not handle Ctrl/Cmd+Shift+A" do + wp_table.focus_row_by_keyboard(bravo) + wp_table.press_keys([BrowserPlatform.multi_select_modifier(page), :shift, "a"]) + + wp_table.expect_focus_on_row(bravo) + expect_selection + end + + # Backlogs counterpart: modules/backlogs/spec/features/work_packages/batch_selection_spec.rb:154,168 + it "clears with Escape from inside the table and with focus outside it" do + wp_table.focus_row_by_keyboard(bravo) + wp_table.press_keys(%i[shift space]) + expect_selection(bravo) + wp_table.press_keys(:escape) + expect_selection + + wp_table.press_keys(%i[shift space], %i[shift down]) + expect_selection(bravo, charlie) + find("li", text: "(1 - 5/5)").click + wp_table.press_keys(:escape) + expect_selection + end + + it "lets an inline editor consume its own Escape before the selection is cleared" do + wp_table.focus_row_by_keyboard(bravo) + wp_table.press_keys(%i[shift space]) + expect_selection(bravo) + + subject_field = wp_table.edit_field(bravo, :subject) + subject_field.activate! + subject_field.cancel_by_escape + subject_field.expect_inactive! + expect_selection(bravo) + + wp_table.press_keys(:escape) + expect_selection + end + + it "opens one context menu acting on the selection when Shift+F10 is pressed on a selected row" do + wp_table.focus_row_by_keyboard(bravo) + wp_table.press_keys(%i[shift space], %i[shift down]) + expect_selection(bravo, charlie) + + wp_table.press_keys(%i[shift f10]) + + context_menu.expect_open + expect(page).to have_selector(:menu, count: 1) + context_menu.choose("Bulk delete") + destroy_modal.expect_listed(bravo, charlie) + destroy_modal.cancel_deletion + end + + it "replaces the selection when Shift+F10 is pressed on an unselected row" do + wp_table.focus_row_by_keyboard(bravo) + wp_table.press_keys(%i[shift space], :down) + wp_table.expect_focus_on_row(charlie) + expect_selection(bravo) + + wp_table.press_keys(%i[shift f10]) + + context_menu.expect_open + expect_selection(charlie) + context_menu.choose("Delete") + destroy_modal.expect_listed(charlie) + destroy_modal.cancel_deletion + end + + it "returns focus to the originating cell when the menu is closed with Escape" do + wp_table.focus_cell_by_keyboard(delta, :subject) + + wp_table.press_keys(%i[shift f10]) + context_menu.expect_open + expect(page).to have_selector(:menuitem, focused: true) + wp_table.press_keys(:escape) + + context_menu.expect_closed + wp_table.expect_focus_on_cell(delta, :subject) + expect_selection(delta) + end + + it "opens the context menu with the Menu key and with Shift+Alt+F10" do + wp_table.focus_row_by_keyboard(bravo) + + wp_table.press_menu_key + context_menu.expect_open + expect(page).to have_selector(:menuitem, focused: true) + wp_table.press_keys(:escape) + context_menu.expect_closed + wp_table.expect_focus_on_row(bravo) + + wp_table.press_keys(%i[shift alt f10]) + context_menu.expect_open + expect(page).to have_selector(:menuitem, focused: true) + wp_table.press_keys(:escape) + context_menu.expect_closed + wp_table.expect_focus_on_row(bravo) + end + + it "keeps the selection and the header focus when sorting from the column menu" do + wp_table.focus_row_by_keyboard(bravo) + wp_table.press_keys(%i[shift space], %i[shift down]) + expect_selection(bravo, charlie) + + wp_table.press_keys(:right, :right, :up, :up, :up) + wp_table.expect_focus_on_column_header("Subject") + wp_table.press_keys(:enter) + within "#column-context-menu" do + click_on "Sort descending" + end + + wp_table.expect_work_package_order(echo, delta, charlie, bravo, alpha) + expect_selection(bravo, charlie) + wp_table.expect_focus_on_column_header("Subject") + end + + it "returns focus to the originating row when the menu is closed with Escape" do + wp_table.focus_row_by_keyboard(delta) + + wp_table.press_keys(%i[shift f10]) + context_menu.expect_open + expect(page).to have_selector(:menuitem, focused: true) + wp_table.press_keys(:escape) + + context_menu.expect_closed + wp_table.expect_focus_on_row(delta) + expect_selection(delta) + end + + # Backlogs counterpart: modules/backlogs/spec/features/work_packages/batch_selection_spec.rb:111 + it "mixes pointer and keyboard gestures on one selection and anchor" do + modifier = BrowserPlatform.multi_select_modifier(page) + wp_table.focus_row_by_keyboard(alpha) + wp_table.press_keys(%i[shift space], %i[shift down], %i[shift down]) + expect_selection(alpha, bravo, charlie) + + wp_table.row(echo).click(:shift) + expect_selection(*listed) + + wp_table.row(bravo).click(modifier) + expect_selection(alpha, charlie, delta, echo) + end + + it "announces the page scope on Select All, and when the selection is cleared" do + wp_table.focus_row_by_keyboard(bravo) + wp_table.press_keys(select_all_chord) + live_region.expect_announcement("All 5 work packages on this page selected.") + + wp_table.press_keys(:escape) + live_region.expect_announcement("Selection cleared.") + end + + it "announces each range change from a fixed anchor" do + wp_table.focus_row_by_keyboard(charlie) + wp_table.press_keys(%i[shift space], %i[shift down]) + expect_selection(charlie, delta) + live_region.expect_announcement("2 work packages selected.") + + wp_table.press_keys(%i[shift down]) + expect_selection(charlie, delta, echo) + live_region.expect_announcement("3 work packages selected.") + + wp_table.press_keys(:up, :up, :up, :up) + wp_table.expect_focus_on_row(alpha) + wp_table.press_keys(%i[shift down]) + expect_selection(bravo, charlie) + live_region.expect_announcement("2 work packages selected.") + end + + it "announces the remaining selection when a filter drops a selected work package" do + wp_table.focus_row_by_keyboard(alpha) + wp_table.press_keys(%i[shift space], %i[shift down]) + expect_selection(alpha, bravo) + live_region.expect_announcement("2 work packages selected.") + + filters.open + filters.add_filter_by("Subject", "doesn't contain", ["Bravo"]) + wp_table.expect_work_package_order(alpha, charlie, delta, echo) + + expect(wp_table.treegrid).to have_selector(:row, alpha.subject, aria: { selected: true }) + expect(wp_table.treegrid).to have_selector(:row, aria: { selected: true }, count: 1) + live_region.expect_announcement("1 work package selected.") + end + + it "announces the remaining selection when a plain click collapses a batch" do + wp_table.focus_row_by_keyboard(alpha) + wp_table.press_keys(%i[shift space], %i[shift down], %i[shift down]) + expect_selection(alpha, bravo, charlie) + live_region.expect_announcement("3 work packages selected.") + + wp_table.row(delta).click + expect_selection(delta) + live_region.expect_announcement("1 work package selected.") + end + + it "announces a same-count replacement afresh" do + wp_table.focus_row_by_keyboard(charlie) + wp_table.press_keys(%i[shift space], %i[shift down]) + expect_selection(charlie, delta) + live_region.expect_announcement("2 work packages selected.") + + wp_table.press_keys(:up) + wp_table.expect_focus_on_row(charlie) + expect_selection(charlie, delta) + + wp_table.press_keys(%i[shift up]) + expect_selection(bravo, charlie) + # The live region repeats an identical message with a trailing no-break space. + live_region.expect_announcement(/\A2 work packages selected\.[ \u00A0]\z/) + end + + # Backlogs counterpart: modules/backlogs/spec/features/work_packages/batch_selection_spec.rb:202 + # Divergence: Enter is click-like in the table and re-anchors, Backlogs passes it through. + it "selects only the focused row on Enter and re-anchors there" do + wp_table.focus_row_by_keyboard(bravo) + wp_table.press_keys(%i[shift space], %i[shift down]) + expect_selection(bravo, charlie) + + wp_table.press_keys(:enter) + expect_selection(charlie) + + wp_table.press_keys(%i[shift down]) + expect_selection(charlie, delta) + end + end + + context "in a grouped table" do + let(:one) { create(:category, project:, name: "Group one") } + let(:two) { create(:category, project:, name: "Group two") } + let!(:alpha) { create(:work_package, project:, category: one, subject: "Alpha task") } + let!(:bravo) { create(:work_package, project:, category: one, subject: "Bravo task") } + let!(:charlie) { create(:work_package, project:, category: two, subject: "Charlie task") } + let!(:delta) { create(:work_package, project:, category: two, subject: "Delta task") } + let(:listed) { [alpha, bravo, charlie, delta] } + let!(:query) do + create(:query, user:, project:, column_names: %w[id subject estimated_hours], + sort_criteria: [%w[subject asc]], group_by: "category", + show_hierarchies: false, display_sums: true) + end + + before do + wp_table.visit_query(query) + wp_table.expect_work_package_order(*listed) + wp_table.expect_inline_create_button + end + + it "passes Shift+ArrowDown through sums rows and group headers without changing the range" do + wp_table.focus_row_by_keyboard(alpha) + wp_table.press_keys(%i[shift space], %i[shift down]) + expect_selection(alpha, bravo) + + wp_table.press_keys(%i[shift down]) + expect(wp_table.treegrid).to have_selector(:row, /\bSum\b/, focused: true) + expect_selection(alpha, bravo) + + wp_table.press_keys(%i[shift down]) + expect(wp_table.treegrid).to have_selector(:row, "Group two", focused: true) + expect_selection(alpha, bravo) + + wp_table.press_keys(%i[shift down]) + expect_selection(alpha, bravo, charlie) + wp_table.expect_focus_on_row(charlie) + end + + it "does not open a context menu from a group header" do + wp_table.focus_row_by_keyboard(alpha) + wp_table.press_keys(:up) + expect(wp_table.treegrid).to have_selector(:row, "Group one", focused: true) + + wp_table.press_keys(%i[shift f10], :down) + + wp_table.expect_focus_on_row(alpha) + expect(page).to have_no_selector(:menu) + end + + it "does not consume Shift+Space on a group header and leaves the selection alone" do + wp_table.focus_row_by_keyboard(alpha) + wp_table.press_keys(%i[shift space], :up) + expect(wp_table.treegrid).to have_selector(:row, "Group one", focused: true) + + wp_table.press_keys(%i[shift space]) + + expect_selection(alpha) + end + + it "selects only work packages with Select All from a group header, without anchoring" do + wp_table.focus_row_by_keyboard(alpha) + wp_table.press_keys(:up) + expect(wp_table.treegrid).to have_selector(:row, "Group one", focused: true) + + wp_table.press_keys(select_all_chord) + expect_selection(*listed) + live_region.expect_announcement("All 4 work packages on this page selected.") + + wp_table.press_keys(%i[shift down]) + expect(wp_table.treegrid).to have_selector(:row, "Alpha task", focused: true) + expect_selection(alpha) + end + + it "leaves the selection alone on Shift+Space on a sums row" do + wp_table.focus_row_by_keyboard(alpha) + wp_table.press_keys(%i[shift space], %i[shift down]) + expect_selection(alpha, bravo) + + wp_table.press_keys(%i[shift down]) + expect(wp_table.treegrid).to have_selector(:row, /\bSum\b/, focused: true) + wp_table.press_keys(%i[shift space]) + + expect_selection(alpha, bravo) + end + end + + context "in an expanded hierarchy" do + let(:user) do + create(:user, + member_with_permissions: { + project => %i[view_work_packages edit_work_packages add_work_packages delete_work_packages manage_subtasks] + }) + end + let!(:parent) { create(:work_package, project:, subject: "Alpha parent") } + let!(:child) { create(:work_package, project:, parent:, subject: "Bravo child") } + let!(:sibling) { create(:work_package, project:, subject: "Charlie sibling") } + let!(:query) do + create(:query, user:, project:, column_names: %w[id subject], + sort_criteria: [%w[subject asc]], show_hierarchies: true) + end + + before do + wp_table.visit_query(query) + wp_table.expect_work_package_order(parent, child, sibling) + end + + it "keeps focus on the work package after a menu action re-renders the table" do + wp_table.focus_row_by_keyboard(child) + expect(wp_table.treegrid).to have_selector(:row, "Bravo child", aria: { level: 2 }) + + wp_table.press_keys(%i[shift f10]) + context_menu.expect_open + context_menu.choose("Outdent hierarchy") + + expect(wp_table.treegrid).to have_selector(:row, "Bravo child", aria: { level: 1 }) + wp_table.expect_focus_on_row(child) + end + end + + context "in a hierarchy with a collapsed parent" do + let!(:parent) { create(:work_package, project:, subject: "Alpha parent") } + let!(:child) { create(:work_package, project:, parent:, subject: "Bravo child") } + let!(:sibling) { create(:work_package, project:, subject: "Charlie sibling") } + let!(:other) { create(:work_package, project:, subject: "Delta other") } + let(:listed) { [parent, child, sibling, other] } + let!(:query) do + create(:query, user:, project:, column_names: %w[id subject], + sort_criteria: [%w[subject asc]], show_hierarchies: true) + end + + def expect_selection(*selected) + listed.each do |work_package| + expect(wp_table.treegrid) + .to have_css("tr[data-work-package-id='#{work_package.id}']" \ + "[aria-selected='#{selected.include?(work_package)}']", + visible: :all) + end + end + + before do + wp_table.visit_query(query) + wp_table.expect_work_package_order(parent, child, sibling, other) + collapse_parent + end + + def collapse_parent + find(:role, "button", accessible_name: /\AHierarchy level 1, expanded\./).click + expect(wp_table.treegrid).to have_no_css("tr[data-work-package-id='#{child.id}']") + end + + it "includes collapsed children in a Shift+ArrowDown range" do + wp_table.focus_row_by_keyboard(parent) + wp_table.press_keys(%i[shift space]) + expect_selection(parent) + + wp_table.press_keys(%i[shift down]) + + wp_table.expect_focus_on_row(sibling) + expect_selection(parent, child, sibling) + end + + it "includes collapsed children in Select All" do + wp_table.focus_row_by_keyboard(sibling) + wp_table.press_keys(select_all_chord) + + expect_selection(*listed) + live_region.expect_announcement("All 4 work packages on this page selected.") + end + end +end diff --git a/spec/support/components/live_region.rb b/spec/support/components/live_region.rb new file mode 100644 index 000000000000..09778e8eaf3f --- /dev/null +++ b/spec/support/components/live_region.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module Components + class LiveRegion + include Capybara::DSL + include Capybara::RSpecMatchers + include RSpec::Matchers + + def expect_announcement(text) + expect(polite_region).to have_css("#polite", text:) + end + + private + + def polite_region + find("live-region", visible: :all).shadow_root + end + end +end diff --git a/spec/support/pages/work_packages/work_packages_table.rb b/spec/support/pages/work_packages/work_packages_table.rb index 03605b0e78b9..8abc6f0a52d9 100644 --- a/spec/support/pages/work_packages/work_packages_table.rb +++ b/spec/support/pages/work_packages/work_packages_table.rb @@ -323,6 +323,13 @@ def press_keys(*keys) page.driver.browser.keyboard.type(*keys) end + # Ferrum maps no symbol to the Menu key, so dispatch it from its own key table. + def press_menu_key + key = Ferrum::Keyboard::KEYS.fetch("ContextMenu").transform_keys(&:to_sym) + page.driver.browser.page.command("Input.dispatchKeyEvent", type: "rawKeyDown", **key) + page.driver.browser.page.command("Input.dispatchKeyEvent", type: "keyUp", **key) + end + # Expects the "Create new work package" button below the results to be rendered. def expect_inline_create_button expect(table_container).to have_button("Create new work package")