From 86a7f7c967431af0acd2e114c46e9a07b7c525c3 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Thu, 22 Oct 2026 21:23:32 +0100 Subject: [PATCH 01/16] [DREAM-886] Add navigation flag and harness markup Adds the keyboardNavigationEnabled table configuration flag, off by default. Teaches the fast-table test harness an opt-in productionMarkup mode that renders the sticky thead, its column and configuration cells, and the totals row, with headerRow(), headerCells() and totalsRow() accessors, so keyboard navigation specs can walk production-shaped markup. addInlineCreateRow now appends to the table's tbody[wpInlineCreate], as production does, and creates that body after the totals body only when there is none. https://community.openproject.org/wp/DREAM-886 --- .../testing/table-harness.spec.ts | 54 +++++++++++++ .../wp-fast-table/testing/table-harness.ts | 78 +++++++++++++++++-- .../wp-table/wp-table-configuration.ts | 3 + 3 files changed, 128 insertions(+), 7 deletions(-) create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.spec.ts diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.spec.ts new file mode 100644 index 000000000000..855555b84ff2 --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.spec.ts @@ -0,0 +1,54 @@ +//-- 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 { WorkPackageTableConfiguration } from 'core-app/features/work-packages/components/wp-table/wp-table-configuration'; +import { buildTable, TableHarness } from './table-harness'; + +describe('table harness production markup', () => { + let harness:TableHarness|undefined; + + afterEach(async () => { + await harness?.destroy(); + harness = undefined; + }); + + it('renders a sticky header row with one cell per column plus the configuration cell, and a totals row', async () => { + harness = buildTable({ workPackages: [{ id: '1' }], columns: ['id', 'subject'], productionMarkup: true }); + await harness.render(); + + expect(harness.headerCells().map((th) => th.textContent?.trim())).toEqual(['id', 'subject', '']); + expect(harness.headerRow().closest('thead')).toHaveClass('-sticky'); + expect(harness.totalsRow()).toHaveClass('wp-table--sums-row'); + expect(harness.headerCells()[0].querySelector('a[href]')).not.toBeNull(); + expect(harness.headerCells()[2].querySelector('button.wp-table--columns-selection')).not.toBeNull(); + }); + + it('keyboardNavigationEnabled defaults to false and can be turned on', () => { + expect(new WorkPackageTableConfiguration({}).keyboardNavigationEnabled).toBe(false); + expect(new WorkPackageTableConfiguration({ keyboardNavigationEnabled: true }).keyboardNavigationEnabled).toBe(true); + }); +}); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.ts index 8416b8a7c18c..5b004962023c 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.ts @@ -130,6 +130,8 @@ export interface TableHarnessOptions { configuration?:WorkPackageTableConfigurationObject; /** Keeps production defaults for every setting but the extra columns the harness cannot build. */ productionDefaults?:boolean; + /** Renders the production `thead` and totals row around the results, for navigation that walks them. */ + productionMarkup?:boolean; /** Overrides for the drag action service the drop handler resolves. */ dragAction?:Partial; /** Uses the production action registry, including group and hierarchy actions. */ @@ -167,6 +169,12 @@ export interface TableHarness { row(workPackageId:string):HTMLTableRowElement; /** The relation row of `to` under `from`, located by its occurrence key. */ relationRow(from:string, to:string, type?:string):HTMLTableRowElement; + /** The `thead` row; requires `productionMarkup`. */ + headerRow():HTMLTableRowElement; + /** The header cells in column order, ending with the configuration cell; requires `productionMarkup`. */ + headerCells():HTMLTableCellElement[]; + /** The totals row; requires `productionMarkup`. */ + totalsRow():HTMLTableRowElement; timelineRow(workPackageId:string):HTMLElement; /** The timeline cell at the position of the given table row; both sides render one element per row. */ timelineRowOf(row:HTMLElement):HTMLElement; @@ -214,7 +222,7 @@ export function buildTable(options:TableHarnessOptions):TableHarness { .addFieldType(IntegerDisplayField, 'integer', ['Integer']); const querySpace = injector.get(IsolatedQuerySpace); const states = injector.get(States); - const dom = options.dom ?? buildDom(); + const dom = options.dom ?? buildDom({ productionMarkup: options.productionMarkup }); const groupBy = options.groupBy ?? 'status'; const query = options.query ?? buildQuery( @@ -225,6 +233,9 @@ export function buildTable(options:TableHarnessOptions):TableHarness { options.highlightingMode, ); querySpace.query.putValue(query); + if (options.productionMarkup) { + fillProductionMarkup(dom.wrapper, query); + } querySpace.groups.putValue((options.groups ?? []).map((group, index) => buildGroup(group, groupBy, index))); initializeViewServices(injector, query); @@ -304,6 +315,18 @@ export function buildTable(options:TableHarnessOptions):TableHarness { return row; }, + headerRow() { + return productionMarkupElement(dom.wrapper, 'thead tr'); + }, + + headerCells() { + return Array.from(productionMarkupElement(dom.wrapper, 'thead tr').querySelectorAll('th')); + }, + + totalsRow() { + return productionMarkupElement(dom.wrapper, 'tr.wp-table--sums-row'); + }, + timelineRow(workPackageId) { const row = dom.timelineBody.querySelector(`.wp-timeline-cell[data-work-package-id="${workPackageId}"]`); if (!row) { @@ -375,9 +398,7 @@ export function buildTable(options:TableHarnessOptions):TableHarness { const builder = new InlineCreateRowBuilder(injector, table); table.editing.startEditing(workPackage, builder.classIdentifier(workPackage)); const [row] = builder.buildEmpty(workPackage); - const inlineCreateBody = document.createElement('tbody'); - inlineCreateBody.append(row); - dom.tbody.after(inlineCreateBody); + inlineCreateBodyOf(dom).append(row); return row; }, @@ -556,12 +577,55 @@ function relationsStub(relations:RelationFixture[]) { }; } -export function buildDom() { +function productionMarkupElement(wrapper:HTMLElement, selector:string):T { + const element = wrapper.querySelector(selector); + if (!element) { + throw new Error(`No ${selector} rendered; build the table with productionMarkup`); + } + return element; +} + +function fillProductionMarkup(wrapper:HTMLElement, query:QueryResource) { + const columns = query.columns as unknown as { id:string, name:string }[]; + const headerCells = columns.map(({ id, name }) => ` + +
+ ${name} +
+ `).join(''); + const configurationCell = ` + + + `; + + productionMarkupElement(wrapper, 'thead tr').innerHTML = headerCells + configurationCell; + productionMarkupElement(wrapper, 'tr.wp-table--sums-row').innerHTML = columns.map(() => '').join(''); +} + +function inlineCreateBodyOf(dom:ReturnType):HTMLTableSectionElement { + const table = dom.tbody.closest('table')!; + const existing = table.querySelector('tbody[wpInlineCreate]'); + if (existing) { + return existing; + } + + const body = document.createElement('tbody'); + body.setAttribute('wpInlineCreate', ''); + const totalsBody = table.querySelector(':scope > tbody:has(> tr.wp-table--sums-row)'); + (totalsBody ?? dom.tbody).after(body); + return body; +} + +export function buildDom(options:{ productionMarkup?:boolean } = {}) { const wrapper = document.createElement('div'); wrapper.innerHTML = `
-
+ + ${options.productionMarkup ? '' : ''} + + ${options.productionMarkup ? '' : ''} +
@@ -572,7 +636,7 @@ export function buildDom() { wrapper, container: wrapper.querySelector('.work-packages-tabletimeline--table-side')!, scroll: wrapper.querySelector('.work-package-table--container')!, - tbody: wrapper.querySelector('tbody')!, + tbody: wrapper.querySelector('tbody.work-package--results-tbody')!, timelineBody: wrapper.querySelector('.wp-table-timeline--body')!, }; } diff --git a/frontend/src/app/features/work-packages/components/wp-table/wp-table-configuration.ts b/frontend/src/app/features/work-packages/components/wp-table/wp-table-configuration.ts index d5a7bf6ee097..aa618df94ba4 100644 --- a/frontend/src/app/features/work-packages/components/wp-table/wp-table-configuration.ts +++ b/frontend/src/app/features/work-packages/components/wp-table/wp-table-configuration.ts @@ -37,6 +37,9 @@ export class WorkPackageTableConfiguration { /** Render the action column (last column) with the actions defined in the TableActionsService */ public actionsColumnEnabled = true; + /** Whether the table is navigable by keyboard through a roving tabindex */ + public keyboardNavigationEnabled = false; + /** Whether the work package context menu is enabled */ public contextMenuEnabled = true; From b071a5d36a5353aab08d5c605ec77fae2c3b3e78 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Thu, 22 Oct 2026 21:27:01 +0100 Subject: [PATCH 02/16] [DREAM-886] Add pure focus position model Adds a DOM-free navigation model for the work package table. nextPosition maps a key press and the current position to a move, expand, collapse or none result, skipping hidden rows and keeping the desired column across rows of different widths. initialPosition picks the first visible work package row, and reconcilePosition keeps focus on the same row after a re-render, rebinding group headers and sums rows by group identifier, else falling back to the nearest visible row. https://community.openproject.org/wp/DREAM-886 --- .../state/keyboard/focus-position.spec.ts | 222 ++++++++++++++++ .../handlers/state/keyboard/focus-position.ts | 246 ++++++++++++++++++ 2 files changed, 468 insertions(+) create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position.spec.ts create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position.ts diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position.spec.ts new file mode 100644 index 000000000000..1561fa5a9796 --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position.spec.ts @@ -0,0 +1,222 @@ +//-- 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 { + FocusPosition, + initialPosition, + NavigableRow, + nextPosition, + reconcilePosition, +} from './focus-position'; + +function buildRows():NavigableRow[] { + return [ + { key: 'header', kind: 'header', hidden: false, cellCount: 3, expanded: null }, + { key: 'A', kind: 'workPackage', hidden: false, cellCount: 3, expanded: null }, + { key: '0:header', kind: 'groupHeader', hidden: false, cellCount: 1, expanded: true, groupIdentifier: 'g' }, + { key: 'B', kind: 'workPackage', hidden: true, cellCount: 3, expanded: null }, + { key: 'C', kind: 'workPackage', hidden: false, cellCount: 3, expanded: false }, + { key: '0:sums', kind: 'sums', hidden: false, cellCount: 3, expanded: null, groupIdentifier: 'g' }, + { key: 'totals', kind: 'totals', hidden: false, cellCount: 3, expanded: null }, + ]; +} + +function groupOf(key:string) { + return ['0:header', '0:sums'].includes(key) ? { groupIdentifier: 'g' } : {}; +} + +function row(key:string):FocusPosition { + return { key, cell: null, desiredColumn: null, ...groupOf(key) }; +} + +function cell(key:string, index:number, desiredColumn:number = index):FocusPosition { + return { key, cell: index, desiredColumn, ...groupOf(key) }; +} + +function move(position:FocusPosition) { + return { type: 'move', position }; +} + +describe('focus position model', () => { + it('ArrowDown in row focus skips hidden rows', () => { + const rows = buildRows(); + + expect(nextPosition(rows, row('A'), 'ArrowDown', 5)).toEqual(move(row('0:header'))); + expect(nextPosition(rows, row('0:header'), 'ArrowDown', 5)).toEqual(move(row('C'))); + }); + + it('ArrowUp from first body row lands on the header row', () => { + expect(nextPosition(buildRows(), row('A'), 'ArrowUp', 5)).toEqual(move(row('header'))); + }); + + it('ArrowUp and ArrowDown at the edges are none', () => { + const rows = buildRows(); + + expect(nextPosition(rows, row('header'), 'ArrowUp', 5)).toEqual({ type: 'none' }); + expect(nextPosition(rows, row('totals'), 'ArrowDown', 5)).toEqual({ type: 'none' }); + }); + + it('Home/End go to first/last visible row in row focus', () => { + const rows = buildRows(); + + expect(nextPosition(rows, row('C'), 'Home', 5)).toEqual(move(row('header'))); + expect(nextPosition(rows, row('C'), 'End', 5)).toEqual(move(row('totals'))); + }); + + it('Home/End in cell focus go to the first/last cell of the row', () => { + const rows = buildRows(); + + expect(nextPosition(rows, cell('A', 1), 'Home', 5)).toEqual(move(cell('A', 0))); + expect(nextPosition(rows, cell('A', 1), 'End', 5)).toEqual(move(cell('A', 2))); + }); + + it('ArrowRight on an expanded or leaf row enters cell 0', () => { + const rows = buildRows(); + + expect(nextPosition(rows, row('A'), 'ArrowRight', 5)).toEqual(move(cell('A', 0))); + expect(nextPosition(rows, row('0:header'), 'ArrowRight', 5)).toEqual(move(cell('0:header', 0))); + }); + + it('ArrowRight on a collapsed parent returns expand', () => { + expect(nextPosition(buildRows(), row('C'), 'ArrowRight', 5)).toEqual({ type: 'expand', key: 'C' }); + }); + + it('ArrowLeft on an expanded group header returns collapse', () => { + expect(nextPosition(buildRows(), row('0:header'), 'ArrowLeft', 5)).toEqual({ type: 'collapse', key: '0:header' }); + }); + + it('ArrowLeft on a non-expanded row is none', () => { + expect(nextPosition(buildRows(), row('A'), 'ArrowLeft', 5)).toEqual({ type: 'none' }); + }); + + it('ArrowLeft on cell 0 returns to the row', () => { + expect(nextPosition(buildRows(), cell('A', 0), 'ArrowLeft', 5)) + .toEqual(move({ key: 'A', cell: null, desiredColumn: 0 })); + }); + + it('ArrowRight on the last cell is none', () => { + expect(nextPosition(buildRows(), cell('A', 2), 'ArrowRight', 5)).toEqual({ type: 'none' }); + }); + + it('ArrowDown keeps the desired column and clamps on a shorter row', () => { + const rows = buildRows(); + + const toGroup = nextPosition(rows, cell('A', 2), 'ArrowDown', 5); + expect(toGroup).toEqual(move({ ...cell('0:header', 0, 2) })); + + const toC = nextPosition(rows, (toGroup as { position:FocusPosition }).position, 'ArrowDown', 5); + expect(toC).toEqual(move({ key: 'C', cell: 2, desiredColumn: 2 })); + }); + + it('ArrowLeft/Right set the desired column to the landing cell', () => { + const rows = buildRows(); + + expect(nextPosition(rows, cell('A', 0, 2), 'ArrowRight', 5)).toEqual(move(cell('A', 1))); + expect(nextPosition(rows, cell('A', 2, 0), 'ArrowLeft', 5)).toEqual(move(cell('A', 1))); + }); + + it('CtrlHome/CtrlEnd go to the grid corners in cell focus and first/last row in row focus', () => { + const rows = buildRows(); + + expect(nextPosition(rows, cell('C', 1), 'CtrlHome', 5)).toEqual(move(cell('header', 0))); + expect(nextPosition(rows, cell('C', 1), 'CtrlEnd', 5)).toEqual(move(cell('totals', 2))); + expect(nextPosition(rows, row('C'), 'CtrlHome', 5)).toEqual(move(row('header'))); + expect(nextPosition(rows, row('C'), 'CtrlEnd', 5)).toEqual(move(row('totals'))); + }); + + it('PageDown moves pageSize visible rows and stops at the end', () => { + const rows = buildRows(); + + expect(nextPosition(rows, row('header'), 'PageDown', 2)).toEqual(move(row('0:header'))); + expect(nextPosition(rows, row('0:header'), 'PageDown', 10)).toEqual(move(row('totals'))); + expect(nextPosition(rows, row('totals'), 'PageDown', 2)).toEqual({ type: 'none' }); + }); + + it('PageUp moves pageSize visible rows and stops at the start', () => { + const rows = buildRows(); + + expect(nextPosition(rows, row('0:sums'), 'PageUp', 2)).toEqual(move(row('0:header'))); + expect(nextPosition(rows, row('A'), 'PageUp', 10)).toEqual(move(row('header'))); + }); + + it('PageDown in cell focus keeps cell focus with the desired column', () => { + expect(nextPosition(buildRows(), cell('A', 2), 'PageDown', 2)) + .toEqual(move({ key: 'C', cell: 2, desiredColumn: 2 })); + }); + + it('initialPosition prefers the first visible work package row', () => { + const rows = buildRows(); + + expect(initialPosition(rows)).toEqual(row('A')); + expect(initialPosition(rows.filter((r) => r.kind !== 'workPackage'))).toEqual(row('header')); + expect(initialPosition(rows.map((r) => ({ ...r, hidden: true })))).toBeNull(); + }); + + it('reconcilePosition keeps the same key, else nearest visible by previous index, else null', () => { + const rows = buildRows(); + + expect(reconcilePosition(rows, cell('A', 2), 1)).toEqual(cell('A', 2)); + expect(reconcilePosition(rows, cell('0:header', 2), 2)).toEqual(cell('0:header', 0, 2)); + expect(reconcilePosition(rows, cell('B', 1), 3)).toEqual(row('0:header')); + expect(reconcilePosition(rows, row('gone'), 40)).toEqual(row('totals')); + expect(reconcilePosition(rows.map((r) => ({ ...r, hidden: true })), row('A'), 1)).toBeNull(); + }); + + it('reconcilePosition keeps a sums position on the sums row, not the header, after rebinding', () => { + const rows = buildRows().map((r) => (r.groupIdentifier ? { ...r, key: r.key.replace('0:', '1:') } : r)); + + expect(reconcilePosition(rows, cell('0:sums', 1), 5)).toEqual({ ...cell('0:sums', 1), key: '1:sums' }); + }); + + it('reconcilePosition rebinds a group header by identifier when its ordinal key changed', () => { + const rows = buildRows().map((r) => (r.key === '0:header' ? { ...r, key: '1:header' } : r)); + + expect(reconcilePosition(rows, row('0:header'), 2)).toEqual({ ...row('0:header'), key: '1:header' }); + }); + + it('reconcilePosition does not rebind a removed group to another group at the same ordinal', () => { + const rows = buildRows().filter((r) => !r.groupIdentifier); + rows.splice(rows.length - 1, 0, { key: '0:header', kind: 'groupHeader', hidden: false, cellCount: 1, expanded: true, groupIdentifier: 'h' }); + + expect(reconcilePosition(rows, row('0:header'), 2)).toEqual(row('A')); + }); + + it('reconcilePosition falls back to row focus when the row has no cells', () => { + const rows = buildRows().map((r) => (r.key === 'A' ? { ...r, cellCount: 0 } : r)); + + expect(reconcilePosition(rows, cell('A', 1), 1)).toEqual({ key: 'A', cell: null, desiredColumn: 1 }); + }); + + it('nextPosition on a hidden current row is none for every key', () => { + const keys = ['ArrowUp', 'ArrowDown', 'ArrowLeft', 'ArrowRight', 'Home', 'End', 'PageUp', 'PageDown', 'CtrlHome', 'CtrlEnd'] as const; + + keys.forEach((key) => { + expect(nextPosition(buildRows(), row('B'), key, 5)).toEqual({ type: 'none' }); + expect(nextPosition(buildRows(), cell('B', 1), key, 5)).toEqual({ type: 'none' }); + }); + }); +}); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position.ts new file mode 100644 index 000000000000..b5e2b41988d4 --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position.ts @@ -0,0 +1,246 @@ +//-- 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. +//++ + +export type RowKind = 'header'|'workPackage'|'groupHeader'|'sums'|'totals'; + +export interface NavigableRow { + key:string; + kind:RowKind; + hidden:boolean; + cellCount:number; + expanded:boolean|null; + groupIdentifier?:string; +} + +export interface FocusPosition { + key:string; + cell:number|null; + desiredColumn:number|null; + groupIdentifier?:string; +} + +export type NavKey = 'ArrowUp'|'ArrowDown'|'ArrowLeft'|'ArrowRight'|'Home'|'End'|'PageUp'|'PageDown'|'CtrlHome'|'CtrlEnd'; + +export type NavResult = + | { type:'move', position:FocusPosition } + | { type:'expand', key:string } + | { type:'collapse', key:string } + | { type:'none' }; + +const NONE:NavResult = { type: 'none' }; + +function positionOn(row:NavigableRow, cell:number|null, desiredColumn:number|null):FocusPosition { + const position:FocusPosition = { key: row.key, cell, desiredColumn }; + return row.groupIdentifier === undefined ? position : { ...position, groupIdentifier: row.groupIdentifier }; +} + +function visibleIndexes(rows:readonly NavigableRow[], requireCells:boolean):number[] { + return rows.flatMap((row, index) => (row.hidden || (requireCells && row.cellCount === 0) ? [] : [index])); +} + +function landOn(row:NavigableRow, cellFocus:boolean, desiredColumn:number|null):FocusPosition { + if (!cellFocus) { + return positionOn(row, null, null); + } + + const column = desiredColumn ?? 0; + return positionOn(row, Math.min(column, row.cellCount - 1), column); +} + +function toResult(position:FocusPosition, current:FocusPosition):NavResult { + const unchanged = position.key === current.key + && position.cell === current.cell + && position.desiredColumn === current.desiredColumn; + return unchanged ? NONE : { type: 'move', position }; +} + +function moveRows( + rows:readonly NavigableRow[], + position:FocusPosition, + currentIndex:number, + delta:number, +):NavResult { + const cellFocus = position.cell !== null; + const visible = visibleIndexes(rows, cellFocus); + const at = visible.indexOf(currentIndex); + const target = visible[Math.min(Math.max(at + delta, 0), visible.length - 1)]; + + if (at === -1 || target === currentIndex) { + return NONE; + } + + return toResult(landOn(rows[target], cellFocus, position.desiredColumn), position); +} + +function moveToEdge( + rows:readonly NavigableRow[], + position:FocusPosition, + edge:'first'|'last', + column:'keep'|'edge', +):NavResult { + const cellFocus = position.cell !== null; + const visible = visibleIndexes(rows, cellFocus); + const target = rows[edge === 'first' ? visible[0] : visible[visible.length - 1]]; + + if (!target) { + return NONE; + } + + if (!cellFocus || column === 'keep') { + return toResult(landOn(target, cellFocus, position.desiredColumn), position); + } + + const cell = edge === 'first' ? 0 : target.cellCount - 1; + return toResult(positionOn(target, cell, cell), position); +} + +function moveWithinRow(row:NavigableRow, position:FocusPosition, direction:'Home'|'End'|'ArrowLeft'|'ArrowRight'):NavResult { + const cell = position.cell ?? 0; + const last = row.cellCount - 1; + const landing = { Home: 0, End: last, ArrowLeft: cell - 1, ArrowRight: cell + 1 }[direction]; + + if (landing < 0) { + return toResult(positionOn(row, null, position.desiredColumn), position); + } + + if (landing > last) { + return NONE; + } + + return toResult(positionOn(row, landing, landing), position); +} + +function navigateRowFocus(rows:readonly NavigableRow[], row:NavigableRow, position:FocusPosition, key:NavKey, index:number, pageSize:number):NavResult { + switch (key) { + case 'ArrowRight': + if (row.expanded === false) { + return { type: 'expand', key: row.key }; + } + return row.cellCount > 0 ? { type: 'move', position: positionOn(row, 0, 0) } : NONE; + case 'ArrowLeft': + return row.expanded === true ? { type: 'collapse', key: row.key } : NONE; + default: + return navigateVertically(rows, position, key, index, pageSize); + } +} + +function navigateVertically(rows:readonly NavigableRow[], position:FocusPosition, key:NavKey, index:number, pageSize:number):NavResult { + switch (key) { + case 'ArrowUp': + return moveRows(rows, position, index, -1); + case 'ArrowDown': + return moveRows(rows, position, index, 1); + case 'PageUp': + return moveRows(rows, position, index, -pageSize); + case 'PageDown': + return moveRows(rows, position, index, pageSize); + case 'Home': + return moveToEdge(rows, position, 'first', 'keep'); + case 'End': + return moveToEdge(rows, position, 'last', 'keep'); + case 'CtrlHome': + return moveToEdge(rows, position, 'first', 'edge'); + case 'CtrlEnd': + return moveToEdge(rows, position, 'last', 'edge'); + default: + return NONE; + } +} + +/** Computes what a navigation key does at the given position. */ +export function nextPosition( + rows:readonly NavigableRow[], + position:FocusPosition, + key:NavKey, + pageSize:number, +):NavResult { + const index = rows.findIndex((row) => row.key === position.key); + if (index === -1) { + return NONE; + } + + const row = rows[index]; + if (row.hidden) { + return NONE; + } + + if (position.cell === null) { + return navigateRowFocus(rows, row, position, key, index, pageSize); + } + + switch (key) { + case 'ArrowLeft': + case 'ArrowRight': + case 'Home': + case 'End': + return moveWithinRow(row, position, key); + default: + return navigateVertically(rows, position, key, index, pageSize); + } +} + +/** Returns the position focus starts on after a render, or null when no row is visible. */ +export function initialPosition(rows:readonly NavigableRow[]):FocusPosition|null { + const visible = rows.filter((row) => !row.hidden); + const first = visible.find((row) => row.kind === 'workPackage') ?? visible[0]; + return first ? positionOn(first, null, null) : null; +} + +function keyPart(key:string):string { + return key.slice(key.lastIndexOf(':') + 1); +} + +function findRebound(rows:readonly NavigableRow[], previous:FocusPosition):NavigableRow|undefined { + const { groupIdentifier } = previous; + + if (groupIdentifier !== undefined) { + const part = keyPart(previous.key); + return rows.find((row) => !row.hidden && row.groupIdentifier === groupIdentifier && keyPart(row.key) === part); + } + + return rows.find((row) => !row.hidden && row.key === previous.key); +} + +/** Keeps focus on the same row after a re-render, else on the nearest visible row. */ +export function reconcilePosition( + rows:readonly NavigableRow[], + previous:FocusPosition, + previousIndex:number, +):FocusPosition|null { + const kept = findRebound(rows, previous); + if (kept) { + const cell = previous.cell === null || kept.cellCount === 0 ? null : Math.min(previous.cell, kept.cellCount - 1); + return positionOn(kept, cell, previous.desiredColumn); + } + + const nearest = visibleIndexes(rows, false).reduce( + (best, index) => (best === null || Math.abs(index - previousIndex) < Math.abs(best - previousIndex) ? index : best), + null, + ); + + return nearest === null ? null : positionOn(rows[nearest], null, null); +} From e885cc244d7aec1db1fa1ed6b758344f9bbee805 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Thu, 22 Oct 2026 21:34:58 +0100 Subject: [PATCH 03/16] [DREAM-886] Expose group identity and row refresh Adds an optional groupIdentifier to rendered occurrences and sets it on group header and sums rows, so focus recovery can find a group by its stable identifier instead of its ordinal key. Adds WorkPackageTable#rowsRefreshed$, which emits the work package id once refreshRows replaced at least one of its rows, so consumers can restore state lost when cells are rebuilt in place. WorkPackageTable#notifyRowsRefreshed lets other owners of table rows, such as the totals row, emit an edge row key on the same stream. GroupedRowsBuilder#refreshExpansionState returns the visibility updates instead of applying them, and the table publishes a render after a group collapse even when no row changed visibility, since the group header elements are replaced regardless. https://community.openproject.org/wp/DREAM-886 --- .../modes/grouped/grouped-render-pass.ts | 4 +-- .../modes/grouped/grouped-rows-builder.ts | 4 +-- .../modes/grouped/grouped-sums-row.spec.ts | 9 +++++++ .../builders/primary-render-pass.ts | 2 ++ .../rendered-occurrence-ledger.ts | 2 ++ .../wp-fast-table/wp-fast-table.spec.ts | 26 +++++++++++++++++++ .../components/wp-fast-table/wp-fast-table.ts | 23 +++++++++++++++- 7 files changed, 65 insertions(+), 5 deletions(-) diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-render-pass.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-render-pass.ts index a8feb8070ddf..ec7ce110ba8d 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-render-pass.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-render-pass.ts @@ -78,7 +78,7 @@ export class GroupedRenderPass extends PlainRenderPass { if (nextGroup && groupsChanged) { const groupClass = groupClassNameFor(nextGroup); const rowElement = this.headerBuilder.buildGroupRow(nextGroup, this.colspan); - this.appendNonWorkPackageRow(groupOccurrenceKey(nextGroup.index, 'header'), rowElement, groupClass, [groupRowClass]); + this.appendNonWorkPackageRow(groupOccurrenceKey(nextGroup.index, 'header'), rowElement, groupClass, [groupRowClass], false, nextGroup.identifier); currentGroup = nextGroup; } @@ -184,7 +184,7 @@ export class GroupedRenderPass extends PlainRenderPass { const { additionalClasses, hidden } = this.groupRowState(group); rowElement.classList.add(...additionalClasses); - this.appendNonWorkPackageRow(groupOccurrenceKey(group.index, 'sums'), rowElement, groupClass, additionalClasses, hidden); + this.appendNonWorkPackageRow(groupOccurrenceKey(group.index, 'sums'), rowElement, groupClass, additionalClasses, hidden, group.identifier); } private groupRowState(group:GroupObject):{ additionalClasses:string[], hidden:boolean } { diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-rows-builder.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-rows-builder.ts index 867dbf7271fb..bcf979ce6bf9 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-rows-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-rows-builder.ts @@ -94,7 +94,7 @@ export class GroupedRowsBuilder extends RowsBuilder { /** * Refresh the group expansion state */ - public refreshExpansionState() { + public refreshExpansionState():ReadonlyMap { const groups = this.getGroupData(); const builder = new GroupHeaderBuilder(this.injector); const updates = new Map(); @@ -137,7 +137,7 @@ export class GroupedRowsBuilder extends RowsBuilder { }); }); - this.workPackageTable.setHidden(updates); + return updates; } /** diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-sums-row.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-sums-row.spec.ts index a294c7af1892..dcbc13472ba1 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-sums-row.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-sums-row.spec.ts @@ -26,6 +26,7 @@ //++ import { fireEvent, waitFor, within } from '@testing-library/dom'; +import { groupOccurrenceKey } from '../../../rendered-occurrence-ledger'; import { buildTable, TableHarness } from '../../../testing/table-harness'; const newStatus = { href: '/api/v3/statuses/1' }; @@ -119,4 +120,12 @@ describe('Grouped table sums rows', () => { expect(sumsTimelineRow(0)).toBeVisible(); expect(sumsTimelineRow(1)).toBeVisible(); }); + + it('tags group header and sums occurrences with the group identifier', () => { + const identifier = harness.groupHeader(1).dataset.groupIdentifier; + + expect(identifier).toBeTruthy(); + expect(harness.table.ledger.byKey(groupOccurrenceKey(1, 'header'))?.groupIdentifier).toBe(identifier); + expect(harness.table.ledger.byKey(groupOccurrenceKey(1, 'sums'))?.groupIdentifier).toBe(identifier); + }); }); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/primary-render-pass.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/primary-render-pass.ts index 1daffbc8d4b8..01c9d34e3509 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/primary-render-pass.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/primary-render-pass.ts @@ -245,6 +245,7 @@ workPackage:WorkPackageResource, classIdentifier:string, additionalClasses:string[] = [], hidden = false, + groupIdentifier?:string, ) { row.classList.add(classIdentifier); this.registerAppended(row, { @@ -255,6 +256,7 @@ workPackage:WorkPackageResource, workPackageId: null, renderType: 'primary', hidden, + groupIdentifier, }); } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.ts index f7a8b354968e..f49084526539 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.ts @@ -39,6 +39,8 @@ export interface RenderedOccurrence { readonly workPackageId:string|null; readonly renderType:'primary'|'relations'|'child_relations'; readonly hidden:boolean; + /** Stable identifier of the group for `group:*:header` and `group:*:sums` occurrences. */ + readonly groupIdentifier?:string; element:HTMLTableRowElement|null; readonly relation?:{ label:string; columnId:string; relationType:RelationColumnType }; } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table.spec.ts index 25f42235a920..fae01c98f1dc 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table.spec.ts @@ -161,4 +161,30 @@ describe('WorkPackageTable', () => { expect(harness.table.ledger.byElement(harness.row('1'))?.key).toBe('wp:1'); }); }); + + describe('rowsRefreshed$', () => { + beforeEach(async () => { + harness = buildTable({ workPackages: [{ id: '1' }, { id: '2' }] }); + await harness.render(); + }); + + it('emits the work package id after replacing its row', () => { + const emitted:string[] = []; + harness.table.rowsRefreshed$.subscribe((id) => emitted.push(id)); + + harness.table.refreshRows(buildWorkPackage({ id: '1', subject: 'Renamed' })); + + expect(emitted).toEqual(['1']); + expect(harness.table.ledger.byWorkPackageId('1')[0].element).toBe(harness.row('1')); + }); + + it('emits nothing for an unknown work package', () => { + const emitted:string[] = []; + harness.table.rowsRefreshed$.subscribe((id) => emitted.push(id)); + + harness.table.refreshRows(buildWorkPackage({ id: '99' })); + + expect(emitted).toEqual([]); + }); + }); }); 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 fe059cebc566..785b983c594c 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 @@ -26,6 +26,7 @@ //++ import { DestroyRef, Injector } from '@angular/core'; +import { Subject } from 'rxjs'; import { runCleanup } from 'core-app/shared/helpers/angular/owned-ui-cleanup'; import { TableUiWork } from './table-ui-work'; import { I18nService } from 'core-app/core/i18n/i18n.service'; @@ -90,6 +91,11 @@ export class WorkPackageTable { public readonly ledger = new RenderedOccurrenceLedger(); + private readonly rowsRefreshedSubject = new Subject(); + + /** Emits a work package id after `refreshRows` replaced at least one of its rendered rows, or an edge row key after that row re-rendered. */ + public readonly rowsRefreshed$ = this.rowsRefreshedSubject.asObservable(); + private readonly requestedRelationTargets = new Set(); private pendingRender:PendingRender|null = null; @@ -119,6 +125,7 @@ export class WorkPackageTable { this.lifetimeInjector.destroy(); } finally { runCleanup(() => this.editing.reset()); + this.rowsRefreshedSubject.complete(); } } @@ -206,12 +213,22 @@ export class WorkPackageTable { return; } + let replaced = 0; this.ledger.byWorkPackageId(workPackage.id!).forEach((occurrence) => { const replacement = pass.refresh(occurrence, workPackage); if (replacement) { this.ledger.replaceElement(occurrence.key, replacement); + replaced += 1; } }); + + if (replaced > 0) { + this.notifyRowsRefreshed(workPackage.id!); + } + } + + public notifyRowsRefreshed(key:string):void { + this.rowsRefreshedSubject.next(key); } /** @@ -254,7 +271,11 @@ export class WorkPackageTable { this.querySpace.collapsedGroups.putValue(newState); const t0 = performance.now(); - this.groupedRowsBuilder.refreshExpansionState(); + const updates = this.groupedRowsBuilder.refreshExpansionState(); + this.setHidden(updates); + if (updates.size === 0) { + this.publishRendered(); + } const t1 = performance.now(); debugLog(`Group redraw took ${t1 - t0} milliseconds.`); From a421d09ce1c83e64997a488600d7d4a8b1ed17a2 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Thu, 22 Oct 2026 21:42:04 +0100 Subject: [PATCH 04/16] [DREAM-886] Remove inner Tab stops behind the flag With keyboardNavigationEnabled on, rows, display field spans, the hierarchy and relation indicators, action links, the ID link, share cells and the header sort link and configuration button no longer take Tab focus. Rows get tabindex -1, display fields lose tabindex and role=button, and the details pane renderer keeps both through a new tabbable option that defaults to true. Adds roving-tabindex.ts with navigableCellClassName, paintRovingTabindex, navigableCells, keyboardNavigationEnabled and isNavigableColumn. One eligibility rule decides which cells are navigable: every query column, and the context-menu column while the actions column or column menu is enabled; never the internal drag or baseline columns. Body, group sums, totals and relation sub-rows apply it alike, so vertical moves keep their column. The drag placeholder is built without the class, and relation sub-rows take their tr out of the Tab order and mark every relation cell. With the flag off the DOM is unchanged. Each roving paint also sweeps late-rendered tabbable descendants (custom elements, modal focus anchors) to tabindex -1, sparing the inline-create body and an open inline editor. insideGrid defines the grid as the header, ledger and totals rows; the inline-create body is outside it. Adds removeTabStops to shared/helpers/dom for the builders and the display field renderer. It covers anchors without an href too, since display fields such as spent time set the href after rendering. The harness renders the inline-create body, builds the totals cells from the table's augmented columns with the same eligibility rule, and accepts non-sortable column specs whose header has no sort link. https://community.openproject.org/wp/DREAM-886 --- .../wp-edit-form/table-edit-form.ts | 2 +- .../wp-fast-table/builders/cell-builder.ts | 11 +- .../modes/grouped/group-sums-builder.ts | 5 + .../hierarchy/single-hierarchy-row-builder.ts | 6 +- .../builders/relation-cell-builder.ts | 10 +- .../relations/relation-row-builder.ts | 14 +- .../builders/rows/single-row-builder.ts | 15 +- .../builders/share-cell-builder.ts | 14 +- .../builders/table-action-renderer.ts | 11 +- .../state/keyboard/roving-tabindex.spec.ts | 320 ++++++++++++++++++ .../state/keyboard/roving-tabindex.ts | 93 +++++ .../wp-fast-table/testing/table-harness.ts | 51 ++- .../config-menu/config-menu.component.ts | 4 +- .../config-menu/config-menu.template.html | 1 + .../sort-header/sort-header.directive.html | 2 + .../sort-header/sort-header.directive.ts | 3 + .../wp-table/wp-table.component.html | 7 +- .../fields/display/display-field-renderer.ts | 25 +- .../shared/helpers/dom/remove-tab-stops.ts | 33 ++ 19 files changed, 589 insertions(+), 38 deletions(-) create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex.spec.ts create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex.ts create mode 100644 frontend/src/app/shared/helpers/dom/remove-tab-stops.ts diff --git a/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.ts b/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.ts index 3fd6108ed8c7..135552d18460 100644 --- a/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.ts +++ b/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.ts @@ -69,7 +69,7 @@ export class TableEditForm extends EditForm { @LazyInject() wpListService:WorkPackagesListService; // Use cell builder to reset edit fields - private cellBuilder = new CellBuilder(this.injector); + private cellBuilder = new CellBuilder(this.injector, this.table.configuration); // Subscription private resourceSubscription:Subscription = this diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/cell-builder.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/cell-builder.ts index 08dadf22a0aa..602752a79d86 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/cell-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/cell-builder.ts @@ -33,6 +33,8 @@ import { editFieldContainerClass, } from 'core-app/shared/components/fields/display/display-field-renderer'; import { Injector } from '@angular/core'; +import { WorkPackageTableConfiguration } from 'core-app/features/work-packages/components/wp-table/wp-table-configuration'; +import { keyboardNavigationEnabled, isNavigableColumn, navigableCellClassName } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex'; import { QueryColumn } from 'core-app/features/work-packages/components/wp-query/query-column'; import { SchemaCacheService } from 'core-app/core/schemas/schema-cache.service'; import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; @@ -46,9 +48,10 @@ export class CellBuilder { @LazyInject(WorkPackageViewBaselineService) wpTableBaseline:WorkPackageViewBaselineService; - public fieldRenderer = new DisplayFieldRenderer(this.injector, 'table'); + public fieldRenderer:DisplayFieldRenderer; - constructor(public injector:Injector) { + constructor(public injector:Injector, private readonly configuration?:WorkPackageTableConfiguration) { + this.fieldRenderer = new DisplayFieldRenderer(this.injector, 'table', { tabbable: !keyboardNavigationEnabled(this.configuration) }); } public build(workPackage:WorkPackageResource, column:QueryColumn) { @@ -56,6 +59,10 @@ export class CellBuilder { const attribute = column.id; td.classList.add(tdClassName, attribute); + if (this.configuration && keyboardNavigationEnabled(this.configuration) && isNavigableColumn(column, this.configuration)) { + td.classList.add(navigableCellClassName); + } + if (attribute === 'subject') { td.classList.add('-max'); } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/group-sums-builder.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/group-sums-builder.ts index 6de35ae54d35..254e2c2f3a92 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/group-sums-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/group-sums-builder.ts @@ -25,6 +25,7 @@ // See COPYRIGHT and LICENSE files for more details. //++ +import { isNavigableColumn, keyboardNavigationEnabled, navigableCellClassName } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex'; import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; import { SingleRowBuilder } from 'core-app/features/work-packages/components/wp-fast-table/builders/rows/single-row-builder'; import { IFieldSchema } from 'core-app/shared/components/fields/field.base'; @@ -59,6 +60,10 @@ export class GroupSumsBuilder extends SingleRowBuilder { public renderColumns(sums:GroupObject['sums'], tr:HTMLTableRowElement) { this.augmentedColumns.forEach((column, i:number) => { const td = document.createElement('td'); + const { configuration } = this.workPackageTable; + if (keyboardNavigationEnabled(configuration) && isNavigableColumn(column, configuration)) { + td.classList.add(navigableCellClassName); + } const div = this.renderContent(sums, column.id, this.sumsSchema[column.id] as IFieldSchema); if (i === 0) { diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/single-hierarchy-row-builder.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/single-hierarchy-row-builder.ts index 0107ff996f25..8af7a13e717a 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/single-hierarchy-row-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/single-hierarchy-row-builder.ts @@ -38,6 +38,7 @@ import { } from 'core-app/features/work-packages/components/wp-fast-table/helpers/wp-table-hierarchy-helpers'; import { WorkPackageViewHierarchiesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-hierarchy.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'; export const indicatorCollapsedClass = '-hierarchy-collapsed'; export const hierarchyCellClassName = 'wp-table--hierarchy-span'; @@ -172,6 +173,7 @@ export class SingleHierarchyRowBuilder extends SingleRowBuilder { private buildHierarchyIndicator(workPackage:WorkPackageResource, row:HTMLTableRowElement, level:number):HTMLElement { const hierarchyIndicator = document.createElement('span'); const collapsed = this.wpTableHierarchies.collapsed(workPackage.id!); + const tabindex = keyboardNavigationEnabled(this.workPackageTable.configuration) ? '-1' : '0'; const indicatorWidth = `${hierarchyBaseIndentation + (hierarchyIndentation * level)}px`; hierarchyIndicator.classList.add(hierarchyCellClassName); hierarchyIndicator.style.width = indicatorWidth; @@ -180,7 +182,7 @@ export class SingleHierarchyRowBuilder extends SingleRowBuilder { if (this.parentsWithVisibleChildren[workPackage.id!]) { const className = collapsed ? indicatorCollapsedClass : ''; hierarchyIndicator.innerHTML = ` - + ${this.text.expanded( level, @@ -192,7 +194,7 @@ export class SingleHierarchyRowBuilder extends SingleRowBuilder { `; } else { hierarchyIndicator.innerHTML = ` - + ${this.text.leaf(level)} `; diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relation-cell-builder.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relation-cell-builder.ts index 209e5f90de17..9e9f0cfc5d73 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relation-cell-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relation-cell-builder.ts @@ -35,6 +35,8 @@ import { States } from 'core-app/core/states/states.service'; import { WorkPackageRelationsService } from '../../wp-relations/wp-relations.service'; import { QueryColumn, queryColumnTypes } from '../../wp-query/query-column'; import { tdClassName } from './cell-builder'; +import { keyboardNavigationEnabled, navigableCellClassName } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex'; +import { WorkPackageTableConfiguration } from 'core-app/features/work-packages/components/wp-table/wp-table-configuration'; export const relationCellTdClassName = 'wp-table--relation-cell-td'; export const relationCellIndicatorClassName = 'wp-table--relation-indicator'; @@ -46,7 +48,7 @@ export class RelationCellbuilder { @LazyInject() wpTableRelationColumns:WorkPackageViewRelationColumnsService; - constructor(public readonly injector:Injector) { + constructor(public readonly injector:Injector, private readonly configuration?:WorkPackageTableConfiguration) { } public build(workPackage:WorkPackageResource, column:QueryColumn) { @@ -54,6 +56,10 @@ export class RelationCellbuilder { td.classList.add(tdClassName, relationCellTdClassName, column.id); td.dataset.columnId = column.id; + if (keyboardNavigationEnabled(this.configuration)) { + td.classList.add(navigableCellClassName); + } + // Get current expansion and value state const expanded = this.wpTableRelationColumns.getExpandFor(workPackage.id!) === column.id; const relationState = this.wpRelations.state(workPackage.id!).value; @@ -90,7 +96,7 @@ export class RelationCellbuilder { const indicator = document.createElement('span'); indicator.classList.add(relationCellIndicatorClassName); indicator.setAttribute('aria-hidden', 'true'); - indicator.setAttribute('tabindex', '0'); + indicator.setAttribute('tabindex', keyboardNavigationEnabled(this.configuration) ? '-1' : '0'); return indicator; } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relations/relation-row-builder.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relations/relation-row-builder.ts index b6b2b379b4bb..7673a9ae0626 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relations/relation-row-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relations/relation-row-builder.ts @@ -32,6 +32,8 @@ import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorato import { States } from 'core-app/core/states/states.service'; import { commonRowClassName, SingleRowBuilder, tableRowClassName } from '../rows/single-row-builder'; import { tdClassName } from '../cell-builder'; +import { keyboardNavigationEnabled, navigableCellClassName } from '../../handlers/state/keyboard/roving-tabindex'; +import { removeTabStops } from 'core-app/shared/helpers/dom/remove-tab-stops'; import { WorkPackageTable } from '../../wp-fast-table'; import { isRelationColumn, QueryColumn } from '../../../wp-query/query-column'; @@ -94,6 +96,9 @@ public readonly injector:Injector, const tr = document.createElement('tr'); tr.dataset.workPackageId = to.id!; tr.dataset.classIdentifier = identifier; + if (keyboardNavigationEnabled(this.workPackageTable.configuration)) { + tr.setAttribute('tabindex', '-1'); + } tr.classList.add( commonRowClassName, @@ -128,12 +133,19 @@ relationGroupClass(from.id!), relationLabel.textContent = typeLabel; row.querySelector(`.${relationCellClassName}`)!.innerHTML = ''; - row.querySelector(`.${relationCellClassName}.${columnId}`)!.append(relationLabel); + const labelCell = row.querySelector(`.${relationCellClassName}.${columnId}`)!; + labelCell.append(relationLabel); + if (keyboardNavigationEnabled(this.workPackageTable.configuration)) { + removeTabStops(labelCell); + } } protected emptyRelationCell(column:QueryColumn) { const cell = document.createElement('td'); cell.classList.add(relationCellClassName, tdClassName, column.id); + if (keyboardNavigationEnabled(this.workPackageTable.configuration)) { + cell.classList.add(navigableCellClassName); + } return cell; } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/rows/single-row-builder.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/rows/single-row-builder.ts index d3f2abed7c8e..68c15664df9e 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/rows/single-row-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/rows/single-row-builder.ts @@ -25,6 +25,7 @@ // See COPYRIGHT and LICENSE files for more details. //++ +import { isNavigableColumn, keyboardNavigationEnabled, navigableCellClassName } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex'; import { Injector } from '@angular/core'; import { I18nService } from 'core-app/core/i18n/i18n.service'; import { locateTableRowByIdentifier } from 'core-app/features/work-packages/components/wp-fast-table/helpers/wp-table-row-helpers'; @@ -74,16 +75,16 @@ export class SingleRowBuilder { @LazyInject() I18n!:I18nService; // Cell builder instance - protected cellBuilder = new CellBuilder(this.injector); + protected cellBuilder = new CellBuilder(this.injector, this.workPackageTable.configuration); // Relation cell builder instance - protected relationCellBuilder = new RelationCellbuilder(this.injector); + protected relationCellBuilder = new RelationCellbuilder(this.injector, this.workPackageTable.configuration); // Share cell builder instance - protected shareCellBuilder = new ShareCellbuilder(this.injector); + protected shareCellBuilder = new ShareCellbuilder(this.injector, this.workPackageTable.configuration); // Details Link builder - protected contextLinkBuilder = new TableActionRenderer(this.injector); + protected contextLinkBuilder = new TableActionRenderer(this.injector, this.workPackageTable.configuration); // Baseline column builder protected baselineColumnBuilder = new BaselineColumnBuilder(this.injector); @@ -141,6 +142,10 @@ export class SingleRowBuilder { if (this.workPackageTable.configuration.columnMenuEnabled) { const td = document.createElement('td'); td.classList.add('hide-when-print'); + if (keyboardNavigationEnabled(this.workPackageTable.configuration) + && isNavigableColumn(column, this.workPackageTable.configuration)) { + td.classList.add(navigableCellClassName); + } return td; } return null; @@ -169,7 +174,7 @@ export class SingleRowBuilder { public createEmptyRow(workPackage:WorkPackageResource) { const identifier = this.classIdentifier(workPackage); const tr = document.createElement('tr'); - tr.setAttribute('tabindex', '0'); + tr.setAttribute('tabindex', keyboardNavigationEnabled(this.workPackageTable.configuration) ? '-1' : '0'); tr.dataset.workPackageId = workPackage.id!; tr.dataset.classIdentifier = identifier; tr.classList.add( diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/share-cell-builder.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/share-cell-builder.ts index a53f787554a0..521f993e2fc8 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/share-cell-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/share-cell-builder.ts @@ -29,6 +29,9 @@ import { Injector } from '@angular/core'; import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource'; import { QueryColumn } from '../../wp-query/query-column'; import { tdClassName } from './cell-builder'; +import { WorkPackageTableConfiguration } from 'core-app/features/work-packages/components/wp-table/wp-table-configuration'; +import { keyboardNavigationEnabled, navigableCellClassName } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex'; +import { removeTabStops } from 'core-app/shared/helpers/dom/remove-tab-stops'; import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; import { IsolatedQuerySpace } from 'core-app/features/work-packages/directives/query-space/isolated-query-space'; import { PrincipalRendererService } from 'core-app/shared/components/principal/principal-renderer.service'; @@ -41,7 +44,7 @@ export class ShareCellbuilder { @LazyInject(I18nService) I18n:I18nService; - constructor(public readonly injector:Injector) { + constructor(public readonly injector:Injector, private readonly configuration?:WorkPackageTableConfiguration) { } public build(workPackage:WorkPackageResource, column:QueryColumn) { @@ -49,6 +52,11 @@ export class ShareCellbuilder { td.classList.add(tdClassName, column.id); td.dataset.columnId = column.id; + const navigationEnabled = keyboardNavigationEnabled(this.configuration); + if (navigationEnabled) { + td.classList.add(navigableCellClassName); + } + const relevantShares = this .isolatedQuerySpace .workPackageSharesCache @@ -68,6 +76,10 @@ export class ShareCellbuilder { td.setAttribute('title', this.I18n.t('js.work_packages.sharing.show_all_users')); } + if (navigationEnabled) { + removeTabStops(td); + } + return td; } } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/table-action-renderer.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/table-action-renderer.ts index df57eae683d5..5b7e781bf928 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/table-action-renderer.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/table-action-renderer.ts @@ -32,12 +32,15 @@ import { contextMenuSpanClassName, contextMenuTdClassName } from 'core-app/featu import { internalContextMenuColumn } from 'core-app/features/work-packages/components/wp-fast-table/builders/internal-sort-columns'; import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; import { tdClassName } from './cell-builder'; +import { WorkPackageTableConfiguration } from 'core-app/features/work-packages/components/wp-table/wp-table-configuration'; +import { keyboardNavigationEnabled, navigableCellClassName } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex'; +import { removeTabStops } from 'core-app/shared/helpers/dom/remove-tab-stops'; export class TableActionRenderer { // Injections @LazyInject() tableActionsService:OpTableActionsService; - constructor(public readonly injector:Injector) { + constructor(public readonly injector:Injector, private readonly configuration?:WorkPackageTableConfiguration) { } public build(workPackage:WorkPackageResource):HTMLTableCellElement{ @@ -56,6 +59,12 @@ export class TableActionRenderer { }); td.appendChild(span); + + if (keyboardNavigationEnabled(this.configuration)) { + td.classList.add(navigableCellClassName); + removeTabStops(td); + } + return td; } } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex.spec.ts new file mode 100644 index 000000000000..312c282ac3f6 --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex.spec.ts @@ -0,0 +1,320 @@ +//-- 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 { waitFor, within } from '@testing-library/dom'; +import { DisplayFieldRenderer } from 'core-app/shared/components/fields/display/display-field-renderer'; +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'; +import { WorkPackageViewOrderService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-order.service'; +import { buildWorkPackage } from '../../../testing/work-package-fixture'; +import { buildTable, TableHarness } from '../../../testing/table-harness'; +import { QueryColumn, queryColumnTypes } from 'core-app/features/work-packages/components/wp-query/query-column'; +import { WorkPackageTableConfiguration, WorkPackageTableConfigurationObject } from 'core-app/features/work-packages/components/wp-table/wp-table-configuration'; +import { + internalBaselineColumn, + internalContextMenuColumn, + internalSortColumn, +} from '../../../builders/internal-sort-columns'; +import { + isNavigableColumn, + navigableCellClassName, + navigableCells, + paintRovingTabindex, +} from './roving-tabindex'; + +class AnchorDisplayField extends TextDisplayField { + public render(element:HTMLElement, displayText:string):void { + const link = document.createElement('a'); + link.href = '#'; + link.textContent = displayText; + element.appendChild(link); + } +} + +class LateLinkDisplayField extends TextDisplayField { + public render(element:HTMLElement, displayText:string):void { + const link = document.createElement('a'); + link.textContent = displayText; + element.appendChild(link); + setTimeout(() => { link.href = '#'; }); + } +} + +describe('roving tabindex', () => { + let harness:TableHarness|undefined; + + const mount = async ( + keyboardNavigationEnabled:boolean, + extra:Parameters[0]['configuration'] = {}, + options:Partial[0]> = {}, + ) => { + const status = { name: 'Open', href: '/api/v3/statuses/1' }; + harness = buildTable({ + workPackages: [{ id: '1', attributes: { status } }, { id: '2', attributes: { status } }], + productionMarkup: true, + editing: {}, + groups: [{ value: 'Open', href: '/api/v3/statuses/1', count: 2, sums: { estimatedTime: 'PT1H' } as never }], + configuration: { keyboardNavigationEnabled, ...extra }, + ...options, + }); + await harness.render(); + return harness; + }; + + const tabbables = (root:HTMLElement) => Array.from(root.querySelectorAll('[tabindex="0"]')); + + const subjectField = (table:TableHarness) => table.row('1').querySelector('td.subject .inline-edit--display-field')!; + + afterEach(async () => { + await harness?.destroy(); + harness = undefined; + }); + + describe('stray Tab stops', () => { + it('sweeps a Tab stop a custom element added after the display field rendered', async () => { + const table = await mount(true); + const late = document.createElement('span'); + late.setAttribute('tabindex', '0'); + table.row('1').querySelector('td.subject')!.appendChild(late); + + paintRovingTabindex(table.table.scrollContainer, table.row('1')); + + expect(late).toHaveAttribute('tabindex', '-1'); + }); + + it('leaves the inline-create button and an open editor input alone', async () => { + const table = await mount(true); + const editor = document.createElement('div'); + editor.className = 'inline-edit--active-field'; + const input = document.createElement('input'); + editor.appendChild(input); + table.row('1').querySelector('td.subject')!.appendChild(editor); + const button = within(table.container).getByRole('button', { name: 'Create new work package' }); + + paintRovingTabindex(table.table.scrollContainer, table.row('2')); + + expect(input).not.toHaveAttribute('tabindex', '-1'); + expect(button).not.toHaveAttribute('tabindex'); + expect(button.closest('tr')).not.toHaveAttribute('tabindex'); + }); + }); + + describe('with the flag off', () => { + it('keeps rows and editable fields tabbable', async () => { + const table = await mount(false); + + expect(table.row('1')).toHaveAttribute('tabindex', '0'); + expect(subjectField(table)).toHaveAttribute('tabindex', '0'); + expect(subjectField(table)).toHaveAttribute('role', 'button'); + expect(table.container.querySelector(`.${navigableCellClassName}`)).toBeNull(); + }); + + it('leaves the header sort link without a tabindex', async () => { + const table = await mount(false); + + expect(table.headerCells()[0].querySelector('a')).not.toHaveAttribute('tabindex'); + }); + }); + + describe('with the flag on', () => { + it('leaves no Tab stop inside the table', async () => { + const table = await mount(true, { dragAndDropEnabled: true, columnMenuEnabled: true }); + + expect(tabbables(table.container)).toHaveLength(0); + }); + + it('leaves no native Tab stop, links included', async () => { + harness = buildTable({ + workPackages: [{ id: '1' }], + productionMarkup: true, + configuration: { keyboardNavigationEnabled: true, columnMenuEnabled: true }, + }); + harness.injector.get(DisplayFieldService).addFieldType(AnchorDisplayField, 'text', ['String']); + await harness.render(); + + expect(harness.row('1').querySelector('a[href]')).toHaveAttribute('tabindex', '-1'); + expect(Array.from(harness.container.querySelectorAll('[tabindex="0"], a[href]:not([tabindex="-1"]), button:not([tabindex="-1"])')) + .filter((element) => !element.closest('tbody[wpInlineCreate]'))).toHaveLength(0); + }); + + it('takes a link out of the Tab order when its href is set after render', async () => { + harness = buildTable({ + workPackages: [{ id: '1' }], + productionMarkup: true, + configuration: { keyboardNavigationEnabled: true }, + }); + harness.injector.get(DisplayFieldService).addFieldType(LateLinkDisplayField, 'text', ['String']); + await harness.render(); + const link = harness.row('1').querySelector('td.subject a')!; + + await waitFor(() => expect(link).toHaveAttribute('href', '#')); + expect(link).toHaveAttribute('tabindex', '-1'); + }); + + it('takes the hierarchy and leaf indicators out of the Tab order', async () => { + harness = buildTable({ + workPackages: [{ id: '1' }, { id: '2', ancestors: [{ id: '1' }] }], + showHierarchies: true, + configuration: { keyboardNavigationEnabled: true }, + }); + await harness.render(); + const indicators = harness.container.querySelectorAll('.wp-table--hierarchy-indicator, .wp-table--leaf-indicator'); + + expect(indicators.length).toBeGreaterThan(1); + indicators.forEach((indicator) => expect(indicator).toHaveAttribute('tabindex', '-1')); + }); + + it('drops the tabindex and button role from editable fields', async () => { + const table = await mount(true); + + expect(subjectField(table)).not.toHaveAttribute('tabindex'); + expect(subjectField(table)).not.toHaveAttribute('role'); + }); + + it('excludes the drag handle cell from the navigable cells', async () => { + const table = await mount(true, { dragAndDropEnabled: true }); + const cells = navigableCells(table.row('1')); + + expect(cells.length).toBeGreaterThan(0); + expect(cells.every((cell) => cell.classList.contains(navigableCellClassName))).toBe(true); + expect(cells.some((cell) => cell.classList.contains('wp-table--sort-td'))).toBe(false); + expect(table.row('1').querySelectorAll('td')).toHaveLength(cells.length + 1); + }); + + it('marks the header, totals and context menu cells as navigable', async () => { + const table = await mount(true, { columnMenuEnabled: true }); + + expect(table.headerCells().every((cell) => cell.classList.contains(navigableCellClassName))).toBe(true); + expect(navigableCells(table.totalsRow()).length).toBeGreaterThan(0); + expect(navigableCells(table.row('1')).at(-1)).toHaveClass('hide-when-print'); + }); + }); + + describe('isNavigableColumn', () => { + const configuration = (flags:WorkPackageTableConfigurationObject) => new WorkPackageTableConfiguration({ + actionsColumnEnabled: false, + columnMenuEnabled: false, + ...flags, + }); + + it.each([ + ['the sort column', internalSortColumn, {}, false], + ['the baseline column', internalBaselineColumn, {}, false], + ['the context column with both menus off', internalContextMenuColumn, {}, false], + ['the context column with the actions column on', internalContextMenuColumn, { actionsColumnEnabled: true }, true], + ['the context column with the column menu on', internalContextMenuColumn, { columnMenuEnabled: true }, true], + ['a query column', { id: 'subject' } as QueryColumn, {}, true], + ['a relation column', { id: 'relationsOfTypeFollows', _type: queryColumnTypes.RELATION_OF_TYPE } as QueryColumn, {}, true], + ])('%s is navigable: %s', (_name, column, flags, expected) => { + expect(isNavigableColumn(column, configuration(flags))).toBe(expected); + }); + }); + + describe('navigable cell eligibility', () => { + const groupSumsRow = (table:TableHarness) => table.container.querySelector('tbody.work-package--results-tbody tr.wp-table--sums-row')!; + + it('marks the same cells navigable in body, group sums and totals rows with drag and drop on', async () => { + const table = await mount(true, { dragAndDropEnabled: true }); + const body = navigableCells(table.row('1')); + const sums = navigableCells(groupSumsRow(table)); + const totals = navigableCells(table.totalsRow()); + + expect(body.length).toBeGreaterThan(0); + expect(sums).toHaveLength(body.length); + expect(totals).toHaveLength(body.length); + expect(groupSumsRow(table).cells[0]).not.toHaveClass(navigableCellClassName); + }); + + it('never marks the drag placeholder before the handle render pass replaces it', async () => { + const table = await mount(true, { dragAndDropEnabled: true }, { + beforeAttach: (injector) => { + vi.spyOn(injector.get(WorkPackageViewOrderService), 'positionsFor').mockReturnValue(new Promise(() => undefined)); + }, + }); + + expect(table.row('1').cells[0]).not.toHaveClass(navigableCellClassName); + }); + + it('marks the relation cells of an expanded relation row and takes its row out of the Tab order', async () => { + const table = await mount(true, {}, { + columns: ['id', 'subject', { id: 'relationsOfTypeFollows', relationType: 'follows' }], + relations: [{ from: '1', to: '2', type: 'follows', reverseType: 'precedes' }], + groups: undefined, + }); + table.expand('1', 'relationsOfTypeFollows'); + await table.render(); + const relationRow = table.relationRow('1', '2'); + + expect(relationRow).toHaveAttribute('tabindex', '-1'); + expect(navigableCells(relationRow)).toHaveLength(navigableCells(table.row('1')).length); + expect(relationRow.querySelector('td.relationsOfTypeFollows')).toHaveClass(navigableCellClassName); + expect(relationRow.querySelector('[tabindex="0"]')).toBeNull(); + }); + + it('adds nothing when the flag is off', async () => { + const table = await mount(false, { dragAndDropEnabled: true }); + + expect(table.container.querySelector(`.${navigableCellClassName}`)).toBeNull(); + expect(table.container.querySelector('tr.wp-table--relations-additional-row[tabindex]')).toBeNull(); + }); + }); + + describe('paintRovingTabindex', () => { + it.each([ + ['a header cell', (table:TableHarness) => table.headerCells()[1]], + ['a body row', (table:TableHarness) => table.row('2')], + ['a body cell', (table:TableHarness) => navigableCells(table.row('2'))[1]], + ])('marks exactly %s', async (_name, pick) => { + const table = await mount(true); + const target = pick(table); + + paintRovingTabindex(table.container, target); + + expect(tabbables(table.container)).toEqual([target]); + expect(table.row('1')).toHaveAttribute('tabindex', '-1'); + expect(table.headerRow()).toHaveAttribute('tabindex', '-1'); + }); + + it('marks nothing when no element is focused', async () => { + const table = await mount(true); + + paintRovingTabindex(table.container, null); + + expect(tabbables(table.container)).toHaveLength(0); + }); + }); + + describe('DisplayFieldRenderer', () => { + it('stays tabbable for the details pane by default', async () => { + const table = await mount(true); + const span = new DisplayFieldRenderer(table.injector, 'single-view').render(buildWorkPackage({ id: '1' }), 'subject', null); + + expect(span).toHaveAttribute('tabindex', '0'); + expect(span).toHaveAttribute('role', 'button'); + }); + }); +}); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex.ts new file mode 100644 index 000000000000..e475c0a3e167 --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex.ts @@ -0,0 +1,93 @@ +//-- 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 { QueryColumn } from 'core-app/features/work-packages/components/wp-query/query-column'; +import { WorkPackageTableConfiguration } from 'core-app/features/work-packages/components/wp-table/wp-table-configuration'; +import { activeFieldContainerClassName } from 'core-app/shared/components/fields/edit/edit-form/edit-form'; +import { + internalBaselineColumn, + internalContextMenuColumn, + internalSortColumn, +} from 'core-app/features/work-packages/components/wp-fast-table/builders/internal-sort-columns'; + +export const navigableCellClassName = 'wp-table--navigable-cell'; +export const inlineCreateSelector = 'tbody[wpInlineCreate]'; + +const nativeTabStopSelector = 'a[href], button, input, select, textarea, [tabindex]:not([tabindex="-1"])'; + +/** Whether `node` is in the grid: inside `root` but not in the inline-create body. */ +export function insideGrid(root:HTMLElement, node:Node|null):boolean { + if (!node || !root.contains(node)) { + return false; + } + + const element = node instanceof Element ? node : node.parentElement; + return !element?.closest(inlineCreateSelector); +} + +export function keyboardNavigationEnabled(configuration?:WorkPackageTableConfiguration):boolean { + return configuration?.keyboardNavigationEnabled ?? false; +} + +export function isNavigableColumn(column:QueryColumn, configuration:WorkPackageTableConfiguration):boolean { + switch (column.id) { + case internalSortColumn.id: + case internalBaselineColumn.id: + return false; + case internalContextMenuColumn.id: + return configuration.actionsColumnEnabled || configuration.columnMenuEnabled; + default: + return true; + } +} + +/** + * Makes `focused` the table's only Tab stop: it gets `tabindex="0"`, every + * other row, navigable cell and native Tab stop under `root` gets `tabindex="-1"`. + */ +export function paintRovingTabindex(root:HTMLElement, focused:HTMLElement|null):void { + root + .querySelectorAll(`tr, .${navigableCellClassName}`) + .forEach((element) => { + if (!element.closest(inlineCreateSelector)) { + element.setAttribute('tabindex', element === focused ? '0' : '-1'); + } + }); + + root + .querySelectorAll(nativeTabStopSelector) + .forEach((element) => { + if (element !== focused && !element.closest(inlineCreateSelector) && !element.closest(`.${activeFieldContainerClassName}`)) { + element.setAttribute('tabindex', '-1'); + } + }); +} + +/** The cells of `row` that keyboard navigation can focus, in column order. */ +export function navigableCells(row:HTMLTableRowElement):HTMLTableCellElement[] { + return Array.from(row.cells).filter((cell) => cell.classList.contains(navigableCellClassName)); +} diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.ts index 5b004962023c..b3e51e69593d 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.ts @@ -103,6 +103,8 @@ import { EditForm } from 'core-app/shared/components/fields/edit/edit-form/edit- import { IntegerDisplayField } from 'core-app/shared/components/fields/display/field-types/integer-display-field.module'; import { IFieldSchema } from 'core-app/shared/components/fields/field.base'; import { onDestroySafely } from 'core-app/shared/helpers/angular/owned-ui-cleanup'; +import { internalContextMenuColumn, internalSortColumn } from '../builders/internal-sort-columns'; +import { isNavigableColumn, navigableCellClassName } from '../handlers/state/keyboard/roving-tabindex'; import { QueryColumn } from 'core-app/features/work-packages/components/wp-query/query-column'; import { WorkPackageViewSelectionGesturesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection-gestures.service'; import { ConfigurationService } from 'core-app/core/config/configuration.service'; @@ -110,13 +112,16 @@ import { ConfigurationService } from 'core-app/core/config/configuration.service /** A relation column: `relationType` builds an `ofType` column, `children: true` the child relations column. */ export type RelationColumnSpec = { id:string, relationType:string }|{ id:string, children:true }; +/** A plain column whose header renders without a sort link. */ +export interface NonSortableColumnSpec { id:string, sortable:false } + export interface TableHarnessOptions { workPackages:WorkPackageFixture[]; providers?:Provider[]; /** Retains the root so a replacement table can mount with a fresh injector. */ dom?:ReturnType; /** Column ids, or relation column specs that render real relation rows once expanded. */ - columns?:(string|RelationColumnSpec)[]; + columns?:(string|RelationColumnSpec|NonSortableColumnSpec)[]; /** Feeds the relations service, so `ofType` columns find these relations between fixtures. */ relations?:RelationFixture[]; /** Loads fixture `children` into the resource cache on render (default); `false` leaves them for `requireAll`. */ @@ -233,8 +238,11 @@ export function buildTable(options:TableHarnessOptions):TableHarness { options.highlightingMode, ); querySpace.query.putValue(query); + const configuration = new WorkPackageTableConfiguration( + { ...(options.productionDefaults ? unbuildableColumns : harnessConfiguration), ...options.configuration }, + ); if (options.productionMarkup) { - fillProductionMarkup(dom.wrapper, query); + fillProductionMarkup(dom.wrapper, query, configuration); } querySpace.groups.putValue((options.groups ?? []).map((group, index) => buildGroup(group, groupBy, index))); initializeViewServices(injector, query); @@ -246,9 +254,7 @@ export function buildTable(options:TableHarnessOptions):TableHarness { dom.tbody, dom.timelineBody, {} as WorkPackageTimelineTableController, - new WorkPackageTableConfiguration( - { ...(options.productionDefaults ? unbuildableColumns : harnessConfiguration), ...options.configuration }, - ), + configuration, ); const outputs:WorkPackageViewOutputs = { @@ -585,21 +591,31 @@ function productionMarkupElement(wrapper:HT return element; } -function fillProductionMarkup(wrapper:HTMLElement, query:QueryResource) { - const columns = query.columns as unknown as { id:string, name:string }[]; - const headerCells = columns.map(({ id, name }) => ` - +function fillProductionMarkup(wrapper:HTMLElement, query:QueryResource, configuration:WorkPackageTableConfiguration) { + const keyboardNavigationEnabled = configuration.keyboardNavigationEnabled; + const tabindex = keyboardNavigationEnabled ? ' tabindex="-1"' : ''; + const navigable = keyboardNavigationEnabled ? ' wp-table--navigable-cell' : ''; + const columns = query.columns as unknown as { id:string, name:string, sortable?:boolean }[]; + const headerCells = columns.map(({ id, name, sortable }) => ` +
- ${name} + ${sortable === false ? `${name}` : `${name}`}
`).join(''); const configurationCell = ` - - + + `; productionMarkupElement(wrapper, 'thead tr').innerHTML = headerCells + configurationCell; - productionMarkupElement(wrapper, 'tr.wp-table--sums-row').innerHTML = columns.map(() => '').join(''); + const augmentedColumns = [ + ...(configuration.dragAndDropEnabled ? [internalSortColumn] : []), + ...(columns as QueryColumn[]), + internalContextMenuColumn, + ]; + productionMarkupElement(wrapper, 'tr.wp-table--sums-row').innerHTML = augmentedColumns + .map((column) => ``) + .join(''); } function inlineCreateBodyOf(dom:ReturnType):HTMLTableSectionElement { @@ -625,6 +641,7 @@ export function buildDom(options:{ productionMarkup?:boolean } = {}) { ${options.productionMarkup ? '' : ''} ${options.productionMarkup ? '' : ''} + ${options.productionMarkup ? '' : ''} @@ -647,7 +664,7 @@ function buildSumsSchema(groups:GroupFixture[]):Record { return Object.fromEntries(attributes.map((name) => [name, { type: 'Integer', name, writable: false, hasDefault: false }])); } -export function buildQuery(columns:(string|RelationColumnSpec)[], groupBy:string|null, showHierarchies:boolean, timelineVisible:boolean, highlightingMode:HighlightingMode = 'inline'):QueryResource { +export function buildQuery(columns:(string|RelationColumnSpec|NonSortableColumnSpec)[], groupBy:string|null, showHierarchies:boolean, timelineVisible:boolean, highlightingMode:HighlightingMode = 'inline'):QueryResource { return { id: null, columns: columns.map(buildColumn), @@ -662,13 +679,17 @@ export function buildQuery(columns:(string|RelationColumnSpec)[], groupBy:string } as unknown as QueryResource; } -function buildColumn(column:string|RelationColumnSpec) { +function buildColumn(column:string|RelationColumnSpec|NonSortableColumnSpec) { if (typeof column === 'string') { return { id: column, name: column, _type: 'QueryColumn', href: `/api/v3/queries/columns/${column}` }; } const { id } = column; const href = `/api/v3/queries/columns/${id}`; + if ('sortable' in column) { + return { id, name: id, _type: 'QueryColumn', href, sortable: false }; + } + return 'children' in column ? { id, name: id, _type: queryColumnTypes.RELATION_CHILD, href } : { id, name: id, _type: queryColumnTypes.RELATION_OF_TYPE, relationType: column.relationType, href }; diff --git a/frontend/src/app/features/work-packages/components/wp-table/config-menu/config-menu.component.ts b/frontend/src/app/features/work-packages/components/wp-table/config-menu/config-menu.component.ts index 9dc733becf00..f5b5abac3f80 100644 --- a/frontend/src/app/features/work-packages/components/wp-table/config-menu/config-menu.component.ts +++ b/frontend/src/app/features/work-packages/components/wp-table/config-menu/config-menu.component.ts @@ -26,7 +26,7 @@ //++ import { I18nService } from 'core-app/core/i18n/i18n.service'; -import { ChangeDetectionStrategy, Component, Injector, inject } from '@angular/core'; +import { ChangeDetectionStrategy, Component, Injector, inject, input } from '@angular/core'; import { OpModalService } from 'core-app/shared/components/modal/modal.service'; import { OPContextMenuService } from 'core-app/shared/components/op-context-menu/op-context-menu.service'; import { WpTableConfigurationModalComponent } from 'core-app/features/work-packages/components/wp-table/configuration-modal/wp-table-configuration.modal'; @@ -47,6 +47,8 @@ export class WorkPackagesTableConfigMenuComponent { readonly opModalService = inject(OpModalService); readonly opContextMenu = inject(OPContextMenuService); + readonly keyboardNavigationEnabled = input(false); + public text = { configureTable: this.I18n.t('js.toolbar.settings.configure_view'), }; diff --git a/frontend/src/app/features/work-packages/components/wp-table/config-menu/config-menu.template.html b/frontend/src/app/features/work-packages/components/wp-table/config-menu/config-menu.template.html index a2e90971563f..76ad670ad923 100644 --- a/frontend/src/app/features/work-packages/components/wp-table/config-menu/config-menu.template.html +++ b/frontend/src/app/features/work-packages/components/wp-table/config-menu/config-menu.template.html @@ -1,6 +1,7 @@