From 9ed6438d53b6731ccadfb22fa584c0a9c47defcf Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 06:03:47 +0100 Subject: [PATCH 1/8] [DREAM-938] Characterize hierarchy page order Covers child-before-parent ordering with the parent both inside and outside the query window, plus the parent-before-child case. Adds the cache state accessor needed to exercise deferred insertion in the existing table harness. https://community.openproject.org/wp/DREAM-938 --- .../modes/hierarchy/hierarchy-order.spec.ts | 55 +++++++++++++++++++ .../wp-fast-table/testing/table-harness.ts | 5 +- 2 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-order.spec.ts diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-order.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-order.spec.ts new file mode 100644 index 000000000000..45e64fb5cd99 --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-order.spec.ts @@ -0,0 +1,55 @@ +//-- 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 { buildTable, TableHarness } from '../../../testing/table-harness'; + +describe('Hierarchy ordering across query windows', () => { + let harness:TableHarness; + const parent = { id: '1' }; + const child = { id: '2', ancestors: [parent] }; + const other = { id: '3' }; + + afterEach(() => harness?.destroy()); + + it('inserts a parent absent from the result window before its first child', async () => { + harness = buildTable({ workPackages: [child, other], showHierarchies: true }); + await harness.render(); + expect(harness.rowIds()).toEqual(['1', '2', '3']); + }); + + it('currently defers the child when its parent is in the same window', async () => { + harness = buildTable({ workPackages: [child, other, parent], showHierarchies: true }); + await harness.render(); + expect(harness.rowIds()).toEqual(['3', '1', '2']); + }); + + it('keeps a child under an earlier parent', async () => { + harness = buildTable({ workPackages: [parent, other, child], showHierarchies: true }); + await harness.render(); + expect(harness.rowIds()).toEqual(['1', '2', '3']); + }); +}); 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 1a5037338748..dcc87c971882 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 @@ -510,7 +510,10 @@ export function harnessProviders(dragService:FakeDragAndDropService, options:Tab requireAll: options.requireAll ?? ((ids:string[]) => Promise.resolve( ids.map((id) => states.workPackages.get(id).value!).filter(Boolean), )), - cache: { current: (_id:string, fallback:unknown) => fallback }, + cache: { + current: (_id:string, fallback:unknown) => fallback, + state: (id:string) => states.workPackages.get(id), + }, id: (id:string) => ({ get: () => of(states.workPackages.get(id).value), requireAndStream: () => of(states.workPackages.get(id).value), From c3647aea894d60ec9a999e7fd141f8e53c18c250 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 06:19:56 +0100 Subject: [PATCH 2/8] [DREAM-938] Calculate global body row positions Adds validated pagination and group-prefix arithmetic for stable global body positions and a draft-agreement check that confirms the rendered rows follow those ordinals. Rejects contradictory, unsafe or negative metadata. https://community.openproject.org/wp/DREAM-938 --- .../semantics/body-row-positions.spec.ts | 134 ++++++++++++++ .../semantics/body-row-positions.ts | 170 ++++++++++++++++++ 2 files changed, 304 insertions(+) create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/semantics/body-row-positions.spec.ts create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/semantics/body-row-positions.ts diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/body-row-positions.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/body-row-positions.spec.ts new file mode 100644 index 000000000000..12e0e1d7582b --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/body-row-positions.spec.ts @@ -0,0 +1,134 @@ +//-- 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 { bodyRowPositions, draftAgrees, PositionWindow } from './body-row-positions'; + +const groups = [ + { headerKey: 'G', sumsKey: 'G-sum', count: 4, hasSums: true }, + { headerKey: 'H', sumsKey: 'H-sum', count: 1, hasSums: true }, +]; + +function window(overrides:Partial = {}):PositionWindow { + return { pageNumber: 1, pageSize: 20, total: 100, primaryKeys: [], groups: [], ...overrides }; +} + +describe('bodyRowPositions', () => { + it('retains global ordinals on the second flat page', () => { + const primaryKeys = Array.from({ length: 20 }, (_, index) => String(index)); + const result = bodyRowPositions(window({ pageNumber: 2, primaryKeys })); + expect(result?.count).toBe(100); + expect(result?.ordinals.get('0')).toBe(21); + expect(result?.ordinals.get('19')).toBe(40); + }); + + it('includes every group boundary while preserving unloaded member gaps', () => { + const result = bodyRowPositions(window({ pageNumber: 2, pageSize: 2, total: 5, primaryKeys: ['C', 'D'], groups })); + expect(result?.count).toBe(9); + expect(Array.from(result?.ordinals ?? [])).toEqual([ + ['G', 1], ['G-sum', 6], ['H', 7], ['H-sum', 9], ['C', 4], ['D', 5], + ]); + }); + + it('maps a window crossing a group boundary', () => { + const result = bodyRowPositions(window({ pageNumber: 2, pageSize: 3, total: 5, primaryKeys: ['D', 'E'], groups })); + expect(result?.ordinals.get('D')).toBe(5); + expect(result?.ordinals.get('E')).toBe(8); + }); + + it('omits boundaries for zero count groups', () => { + const result = bodyRowPositions(window({ total: 1, primaryKeys: ['A'], groups: [ + { headerKey: 'empty', sumsKey: 'empty-sum', count: 0, hasSums: true }, + { headerKey: 'G', sumsKey: 'unused-sum', count: 1, hasSums: false }, + ] })); + expect(result?.count).toBe(2); + expect(Array.from(result?.ordinals ?? [])).toEqual([['G', 1], ['A', 2]]); + }); + + it('accepts empty exact collections and out-of-range empty pages', () => { + expect(bodyRowPositions(window({ total: 0 }))?.count).toBe(0); + expect(bodyRowPositions(window({ pageNumber: 100, total: 0 }))?.ordinals.size).toBe(0); + expect(bodyRowPositions(window({ pageNumber: 100, total: 5, groups }))?.count).toBe(9); + }); + + const invalid:Partial[] = [ + { pageNumber: 0 }, { pageNumber: -1 }, { pageNumber: 1.5 }, { pageNumber: NaN }, + { pageSize: 0 }, { pageSize: Infinity }, { pageSize: undefined }, + { total: -1 }, { total: 1.5 }, { total: undefined }, + { pageSize: 1, primaryKeys: ['A', 'B'] }, + { primaryKeys: ['A', 'A'] }, + { total: 0, primaryKeys: ['A'] }, + { pageNumber: 6, pageSize: 20, primaryKeys: ['A'] }, + { pageNumber: Number.MAX_SAFE_INTEGER, pageSize: 20 }, + { total: -1, groups }, { total: 6, groups }, + { total: 0, groups: [{ ...groups[0], count: -1 }] }, + { total: 0, groups: [{ ...groups[0], count: 0.5 }] }, + { total: 5, groups, primaryKeys: ['G'] }, + { total: 5, groups: [groups[0], { ...groups[1], headerKey: 'G' }] }, + { total: Number.MAX_SAFE_INTEGER, groups: [{ ...groups[0], count: Number.MAX_SAFE_INTEGER }] }, + { total: -1, pageNumber: Number.MAX_SAFE_INTEGER, pageSize: 1, primaryKeys: ['A', 'B'] }, + ]; + + it.each(invalid)('rejects contradictory or unsafe metadata %o', (overrides) => { + expect(bodyRowPositions(window(overrides))).toBeNull(); + }); +}); + +describe('draftAgrees', () => { + const member = (key:string) => ({ key, member: true }); + const structural = (key:string) => ({ key, member: false }); + const flat = () => bodyRowPositions(window({ primaryKeys: ['A', 'B'] }))!; + const grouped = () => bodyRowPositions(window({ pageNumber: 2, pageSize: 2, total: 5, primaryKeys: ['C', 'D'], groups }))!; + + it('accepts increasing flat ordinals', () => { + expect(draftAgrees(flat(), [member('A'), member('B')], [])).toBe(true); + }); + + it('rejects flat rows out of rank order', () => { + expect(draftAgrees(flat(), [member('B'), member('A')], [])).toBe(false); + }); + + it('accepts headers, members and sums in group order', () => { + const rows = [structural('G'), member('C'), member('D'), structural('G-sum'), structural('H'), structural('H-sum')]; + expect(draftAgrees(grouped(), rows, groups)).toBe(true); + }); + + it('rejects a member rendered under another group header', () => { + expect(draftAgrees(grouped(), [structural('H'), member('C')], groups)).toBe(false); + }); + + it('rejects a sums row before its members', () => { + expect(draftAgrees(grouped(), [structural('G'), structural('G-sum'), member('C')], groups)).toBe(false); + }); + + it('rejects a structural row that is not the active sums row', () => { + expect(draftAgrees(grouped(), [structural('G'), structural('H-sum')], groups)).toBe(false); + }); + + it('rejects unknown keys', () => { + expect(draftAgrees(grouped(), [structural('G'), member('X')], groups)).toBe(false); + }); +}); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/body-row-positions.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/body-row-positions.ts new file mode 100644 index 000000000000..d692d7a735fb --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/body-row-positions.ts @@ -0,0 +1,170 @@ +//-- 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 interface BodyRowPositions { + readonly count:number; + readonly ordinals:ReadonlyMap; +} + +export interface PositionGroup { + readonly headerKey:string; + readonly sumsKey:string; + readonly count:number; + readonly hasSums:boolean; +} + +export interface PositionWindow { + readonly pageNumber:number; + readonly pageSize:number; + readonly total:number; + readonly primaryKeys:readonly string[]; + readonly groups:readonly PositionGroup[]; +} + +export function bodyRowPositions(window:PositionWindow):BodyRowPositions|null { + const { pageNumber, pageSize, total, primaryKeys, groups } = window; + if (!Number.isSafeInteger(pageNumber) || pageNumber < 1 + || !Number.isSafeInteger(pageSize) || pageSize < 1 + || !Number.isSafeInteger(total) || total < 0 + || primaryKeys.length > pageSize) { + return null; + } + + const firstQueryRank = (pageNumber - 1) * pageSize; + const lastQueryRank = firstQueryRank + primaryKeys.length; + if (!Number.isSafeInteger(firstQueryRank) || !Number.isSafeInteger(lastQueryRank) + || (primaryKeys.length > 0 && lastQueryRank > total)) { + return null; + } + + const ordinals = new Map(); + const add = (key:string, ordinal:number):boolean => { + if (ordinals.has(key) || !Number.isSafeInteger(ordinal) || ordinal < 1) { + return false; + } + ordinals.set(key, ordinal); + return true; + }; + + if (groups.length === 0) { + for (const [index, key] of primaryKeys.entries()) { + if (!add(key, firstQueryRank + index + 1)) { + return null; + } + } + return { count: total, ordinals }; + } + + let queryPrefix = 0; + let bodyPrefix = 0; + const boundaries:{ queryPrefix:number; queryEnd:number; headerOrdinal:number }[] = []; + for (const group of groups) { + if (!Number.isSafeInteger(group.count) || group.count < 0) { + return null; + } + if (group.count === 0) { + continue; + } + + const headerOrdinal = bodyPrefix + 1; + const queryEnd = queryPrefix + group.count; + const bodyEnd = bodyPrefix + group.count + 1 + Number(group.hasSums); + if (!Number.isSafeInteger(queryEnd) || !Number.isSafeInteger(bodyEnd) + || !add(group.headerKey, headerOrdinal) + || (group.hasSums && !add(group.sumsKey, bodyEnd))) { + return null; + } + boundaries.push({ queryPrefix, queryEnd, headerOrdinal }); + queryPrefix = queryEnd; + bodyPrefix = bodyEnd; + } + + if (queryPrefix !== total) { + return null; + } + + let groupIndex = 0; + for (const [index, key] of primaryKeys.entries()) { + const rank = firstQueryRank + index; + while (groupIndex < boundaries.length && rank >= boundaries[groupIndex].queryEnd) { + groupIndex += 1; + } + const boundary = boundaries[groupIndex]; + if (!boundary || !add(key, boundary.headerOrdinal + 1 + (rank - boundary.queryPrefix))) { + return null; + } + } + + return { count: bodyPrefix, ordinals }; +} + +export interface DraftRow { + readonly key:string; + readonly member:boolean; +} + +interface MemberRange { + readonly first:number; + readonly last:number; + readonly sumsKey:string; +} + +export function draftAgrees(body:BodyRowPositions, rows:readonly DraftRow[], groups:readonly PositionGroup[]):boolean { + const ranges = new Map(); + for (const group of groups) { + if (group.count === 0) { + continue; + } + const header = body.ordinals.get(group.headerKey); + if (header === undefined) { + return false; + } + ranges.set(group.headerKey, { first: header + 1, last: header + group.count, sumsKey: group.sumsKey }); + } + + let active:MemberRange|undefined; + let previous = 0; + for (const row of rows) { + const ordinal = body.ordinals.get(row.key); + if (ordinal === undefined || ordinal <= previous) { + return false; + } + const range = ranges.get(row.key); + if (range) { + active = range; + } else if (groups.length > 0) { + if (!active) { + return false; + } + if (row.member ? ordinal < active.first || ordinal > active.last : row.key !== active.sumsKey) { + return false; + } + } + previous = ordinal; + } + return true; +} From 4fc2ec3c835d6eba24a7dea94fa408a143c0d12c Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 06:20:51 +0100 Subject: [PATCH 3/8] [DREAM-938] Project complete table row positions Binds global positions to accepted render snapshots and validates the complete table before publishing index and count attributes. Includes structural rows, preserves collapsed gaps and clears unsupported or contradictory projections. Children-column expansions withdraw the family like other relation expansions. Staged drag moves repaint at once, so the family clears while a row is out of rank order and returns when a failed drop puts it back. A successful drop waits for a collection in the new order. Adds coverage for pagination, stale frames, group membership mismatch, nested tables, children-column expansion, staged drag moves and capability-off consumers. https://community.openproject.org/wp/DREAM-938 --- frontend/eslint.config.mjs | 2 + .../state/drag-and-drop-lifetime.spec.ts | 49 +- .../state/drag-and-drop-transformer.ts | 2 + .../handlers/state/keyboard/navigable-rows.ts | 12 +- .../helpers/structural-rows.spec.ts | 82 +++ .../wp-fast-table/helpers/structural-rows.ts | 61 +++ .../semantics/table-row-positions.spec.ts | 481 ++++++++++++++++++ .../semantics/table-row-positions.ts | 198 +++++++ .../wp-fast-table/testing/table-harness.ts | 32 +- .../components/wp-fast-table/wp-fast-table.ts | 10 +- 10 files changed, 914 insertions(+), 15 deletions(-) create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/helpers/structural-rows.spec.ts create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/helpers/structural-rows.ts create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/semantics/table-row-positions.spec.ts create mode 100644 frontend/src/app/features/work-packages/components/wp-fast-table/semantics/table-row-positions.ts diff --git a/frontend/eslint.config.mjs b/frontend/eslint.config.mjs index 634c6a986e06..d2470ec93bfb 100644 --- a/frontend/eslint.config.mjs +++ b/frontend/eslint.config.mjs @@ -201,6 +201,8 @@ export default defineConfig([ // Allow more than one class definitions per file (test components) 'max-classes-per-file': 'off', + // Test harnesses expose `expect*` helpers that wrap several assertions + 'vitest/expect-expect': ['error', { assertFunctionNames: ['expect', '*.expect*'] }], }, }, { diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/drag-and-drop-lifetime.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/drag-and-drop-lifetime.spec.ts index e70bc6b92941..a0de2371befb 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/drag-and-drop-lifetime.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/drag-and-drop-lifetime.spec.ts @@ -133,7 +133,7 @@ describe('accepted table drag persistence lifetime', () => { await waitFor(() => expect(calls).toEqual(expected)); } - async function mount(kind:'default'|'group'|'hierarchy' = 'group', saved = true) { + async function mount(kind:'default'|'group'|'hierarchy' = 'group', saved = true, positions = false) { const grouped = kind === 'group'; original = query(saved, grouped, kind === 'hierarchy'); const providers:Provider[] = [ @@ -178,7 +178,7 @@ describe('accepted table drag persistence lifetime', () => { views: { post: view }, } }, ]; - dom = buildDom(); + dom = buildDom({ productionMarkup: positions }); table = buildTable({ query: original, builtinDragActions: true, @@ -194,6 +194,11 @@ describe('accepted table drag persistence lifetime', () => { ], groups: grouped ? [{ value: 'New', href: status1.href, count: 1 }, { value: 'Progress', href: status2.href, count: 2 }] : undefined, showHierarchies: kind === 'hierarchy', + ...(positions ? { + productionMarkup: true, + pagination: { offset: 1, pageSize: 50, total: 3 }, + configuration: { keyboardNavigationEnabled: true }, + } : {}), }); await table.render(); table.injector.get(WorkPackageViewPaginationService).initialize(original, { elements: [], offset: 1, pageSize: 50, total: 3, count: 3 } as unknown as WorkPackageCollectionResource); @@ -455,6 +460,46 @@ describe('accepted table drag persistence lifetime', () => { checkSuccessor(); }); + describe('with row positions', () => { + const positions = ['1', '2', '3', '4', '5', '6']; + + it('stays withdrawn through a switch to manual sorting until a matching collection arrives', async () => { + await mount('default', true, true); + table.expectRowPositions(positions, '6'); + const rebuilt = table.nextRender(); + const dropping = table.drop('1', '2', 'bottom'); + await expectCalls(['order']); + expect(table.rowIds()).toEqual(['2', '1', '3']); + table.expectNoRowPositions(); + orderSaved.resolve('now'); + await expectCalls(['order', 'query']); + table.expectNoRowPositions(); + querySaved.next(original); + expect(await dropping).toBe(true); + expect(original.sortBy).toEqual([manualSort]); + await rebuilt; + expect(table.rowIds()).toEqual(['2', '1', '3']); + table.expectNoRowPositions(); + + await table.render(['2', '1', '3'].map((id) => ({ id, attributes: { status: id === '1' ? status1 : status2 } }))); + table.expectRowPositions(positions, '6'); + }); + + it('stays withdrawn when the saved order remains visible after query saving fails', async () => { + await mount('default', true, true); + const rebuilt = table.nextRender(); + const dropping = table.drop('1', '2', 'bottom'); + await expectCalls(['order']); + orderSaved.resolve('now'); + await expectCalls(['order', 'query']); + querySaved.error(new Error('query failed')); + expect(await dropping).toBe(false); + await rebuilt; + expect(table.rowIds()).toEqual(['2', '1', '3']); + table.expectNoRowPositions(); + }); + }); + it('lets the adapter report a hierarchy failure once with the captured resource', async () => { await mount('hierarchy'); const dropping = table.drop('1', '3', 'bottom'); 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 f162e6b78227..69748a9c9a93 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 @@ -205,6 +205,7 @@ export class DragAndDropTransformer { recoverIfCurrent = () => { if (!isCurrent() || table.lastRenderPass !== renderPass || source.parentNode !== parentNode) return; parentNode?.insertBefore(source, nextSibling?.parentNode === parentNode ? nextSibling : null); + table.rowPositions?.paint(); }; if (!stillEligible()) { complete(false); @@ -234,6 +235,7 @@ export class DragAndDropTransformer { // Only preparation observes the staged neighbors. Persistence captures no row. if (target) table.tbody.insertBefore(source, edge === 'top' ? target : target.nextSibling); else table.tbody.appendChild(source); + table.rowPositions?.paint(); const preparedAction = await actionService.prepareDrop(workPackage, source); if (!stillEligible()) { recoverIfCurrent(); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/navigable-rows.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/navigable-rows.ts index c23d8c3b5e95..ce4d15744892 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/navigable-rows.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/navigable-rows.ts @@ -31,20 +31,12 @@ import type { WorkPackageTable } from '../../../wp-fast-table'; import { NavigableRow, RowKind } from './focus-position'; import { navigableCells } from './roving-tabindex'; import { expandedOf, rowKindOf } from '../../../helpers/row-expansion'; +import { headerRowOf, totalsRowOf } from '../../../helpers/structural-rows'; export const headerRowKey = 'header'; export const totalsRowKey = 'totals'; -/** The `thead` row of the table, when rendered. */ -export function headerRowOf(table:WorkPackageTable):HTMLTableRowElement|null { - return table.tbody.closest('table')?.querySelector('thead tr') ?? null; -} - -/** The totals row below the results, when rendered. */ -export function totalsRowOf(table:WorkPackageTable):HTMLTableRowElement|null { - const rows = table.tbody.closest('table')?.querySelectorAll('tr.wp-table--sums-row') ?? []; - return Array.from(rows).find((row) => !table.tbody.contains(row)) ?? null; -} +export { headerRowOf, totalsRowOf }; function occurrenceRow( table:WorkPackageTable, diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/helpers/structural-rows.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/helpers/structural-rows.spec.ts new file mode 100644 index 000000000000..31c5b838b21d --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/helpers/structural-rows.spec.ts @@ -0,0 +1,82 @@ +//-- 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 { buildTable, TableHarness } from '../testing/table-harness'; +import { groupOccurrenceKey } from '../rendered-occurrence-ledger'; +import { + headerRowOf, isEmptyResultsRow, isExposedRow, isInlineCreateRow, isTotalsRow, totalsRowOf, +} from './structural-rows'; + +describe('structural rows', () => { + let harness:TableHarness; + + afterEach(() => harness?.destroy()); + + it('classifies the totals row below the results but not a group sums row', async () => { + const status = { href: '/api/v3/statuses/1' }; + harness = buildTable({ + workPackages: [{ id: '1', attributes: { status } }], + productionMarkup: true, + groups: [{ value: 'G', href: status.href, count: 1, sums: { storyPoints: 1 } }], + }); + await harness.render(); + const groupSums = harness.table.ledger.byKey(groupOccurrenceKey(0, 'sums'))!.element!; + expect(isTotalsRow(harness.table, harness.totalsRow())).toBe(true); + expect(isTotalsRow(harness.table, groupSums)).toBe(false); + expect(totalsRowOf(harness.table)).toBe(harness.totalsRow()); + expect(headerRowOf(harness.table)).toBe(harness.headerRow()); + }); + + it('classifies the create and empty-results rows by their sections', async () => { + harness = buildTable({ workPackages: [], productionMarkup: true }); + await harness.render(); + const createRow = harness.table.tbody.closest('table')!.querySelector('tbody[wpInlineCreate] tr')!; + const empty = document.createElement('tbody'); + empty.className = 'work-package--empty-tbody'; + const emptyRow = empty.insertRow(); + harness.tbody.before(empty); + expect(isInlineCreateRow(createRow)).toBe(true); + expect(isInlineCreateRow(harness.totalsRow())).toBe(false); + expect(isEmptyResultsRow(emptyRow)).toBe(true); + expect(isEmptyResultsRow(createRow)).toBe(false); + }); + + it('treats rows hidden themselves or by their section as not exposed', async () => { + harness = buildTable({ workPackages: [{ id: '1' }], productionMarkup: true }); + await harness.render(); + const row = harness.row('1'); + expect(isExposedRow(row)).toBe(true); + row.setAttribute('aria-hidden', 'true'); + expect(isExposedRow(row)).toBe(false); + row.removeAttribute('aria-hidden'); + row.hidden = true; + expect(isExposedRow(row)).toBe(false); + row.hidden = false; + harness.tbody.setAttribute('aria-hidden', 'true'); + expect(isExposedRow(row)).toBe(false); + }); +}); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/helpers/structural-rows.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/helpers/structural-rows.ts new file mode 100644 index 000000000000..a17791ac3249 --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/helpers/structural-rows.ts @@ -0,0 +1,61 @@ +//-- 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 type { WorkPackageTable } from '../wp-fast-table'; + +const totalsRowSelector = 'tr.wp-table--sums-row'; + +/** The `thead` row of the table, when rendered. */ +export function headerRowOf(table:WorkPackageTable):HTMLTableRowElement|null { + return table.tbody.closest('table')?.querySelector('thead tr') ?? null; +} + +/** The totals row below the results; group sums rows inside the results body do not qualify. */ +export function isTotalsRow(table:WorkPackageTable, row:HTMLTableRowElement):boolean { + return row.matches(totalsRowSelector) && !table.tbody.contains(row); +} + +/** The totals row below the results, when rendered. */ +export function totalsRowOf(table:WorkPackageTable):HTMLTableRowElement|null { + const rows = table.tbody.closest('table')?.querySelectorAll(totalsRowSelector) ?? []; + return Array.from(rows).find((row) => isTotalsRow(table, row)) ?? null; +} + +export function isInlineCreateRow(row:HTMLTableRowElement):boolean { + return row.parentElement?.hasAttribute('wpInlineCreate') === true; +} + +export function isEmptyResultsRow(row:HTMLTableRowElement):boolean { + return row.parentElement?.classList.contains('work-package--empty-tbody') === true; +} + +/** A row assistive technology can reach: neither hidden itself nor inside a hidden section. */ +export function isExposedRow(row:HTMLTableRowElement):boolean { + return !row.hidden + && row.getAttribute('aria-hidden') !== 'true' + && row.parentElement?.getAttribute('aria-hidden') !== 'true'; +} diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/table-row-positions.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/table-row-positions.spec.ts new file mode 100644 index 000000000000..4c5d5caed86e --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/table-row-positions.spec.ts @@ -0,0 +1,481 @@ +//-- 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 { buildTable, TableHarness, TableHarnessOptions } from '../testing/table-harness'; +import { WorkPackageViewCollapsedGroupsService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-collapsed-groups.service'; +import { waitFor } from '@testing-library/dom'; +import { nextTask } from 'core-common/testing/timing'; +import { States } from 'core-app/core/states/states.service'; +import { groupOccurrenceKey } from '../rendered-occurrence-ledger'; +import { buildWorkPackage } from '../testing/work-package-fixture'; + +function deferred() { + let resolve!:(value:T) => void; + let reject!:(error:unknown) => void; + const promise = new Promise((yes, no) => { resolve = yes; reject = no; }); + return { promise, resolve, reject }; +} + +describe('TableRowPositions', () => { + let harness:TableHarness; + const mount = (options:Partial = {}) => { + harness = buildTable({ + workPackages: [{ id: '21' }, { id: '22' }], + productionMarkup: true, + pagination: { offset: 2, pageSize: 20, total: 100 }, + ...options, + configuration: { keyboardNavigationEnabled: true, ...options.configuration }, + }); + return harness.render(); + }; + const table = () => harness.tbody.closest('table')!; + const index = (row:Element) => row.getAttribute('aria-rowindex'); + const createRow = () => table().querySelector('tbody[wpInlineCreate] tr')!; + const expectIndices = (rows:Element[], indices:string[]) => { + expect(rows).toHaveLength(indices.length); + rows.forEach((row, position) => expect(row).toHaveAttribute('aria-rowindex', indices[position])); + }; + afterEach(() => harness?.destroy()); + + it('uses global positions and includes trailing totals and Create', async () => { + await mount(); + harness.expectRowPositions(['1', '22', '23', '102', '103'], '103'); + }); + + it('reprojects local structure without changing the body coordinates', async () => { + await mount(); + harness.totalsRow().setAttribute('aria-hidden', 'true'); + harness.table.rowPositions!.paint(); + expect(harness.totalsRow()).not.toHaveAttribute('aria-rowindex'); + expect(harness.row('21')).toHaveAttribute('aria-rowindex', '22'); + expect(createRow()).toHaveAttribute('aria-rowindex', '102'); + expect(table()).toHaveAttribute('aria-rowcount', '102'); + }); + + it('leaves nested tables alone', async () => { + await mount(); + const nested = document.createElement('table'); + nested.innerHTML = 'nested'; + harness.row('21').cells[0].append(nested); + harness.table.rowPositions!.paint(); + expect(nested.rows[0]).toHaveAttribute('aria-rowindex', '7'); + expect(table()).toHaveAttribute('aria-rowcount', '103'); + }); + + it('clears the whole family for an exposed unclassified row', async () => { + await mount(); + const extra = table().createTBody().insertRow(); + extra.insertCell().textContent = 'Unclassified'; + harness.table.rowPositions!.paint(); + harness.expectNoRowPositions(); + }); + + it('keeps group continuation and collapsed gaps global', async () => { + const status = { href: '/api/v3/statuses/1' }; + await mount({ + workPackages: [{ id: '3', attributes: { status } }, { id: '4', attributes: { status } }], + pagination: { offset: 2, pageSize: 2, total: 5 }, + groups: [ + { value: 'G', href: status.href, count: 4, sums: { storyPoints: 5 } }, + { value: 'H', href: '/api/v3/statuses/2', count: 1, sums: { storyPoints: 2 } }, + ], + }); + const header = () => harness.groupHeader(0); + const sum = () => harness.table.ledger.byKey(groupOccurrenceKey(0, 'sums'))!.element!; + expectIndices([header(), harness.row('3'), harness.row('4'), sum()], ['2', '5', '6', '7']); + expect(harness.totalsRow()).toHaveAttribute('aria-rowindex', '11'); + expect(table()).toHaveAttribute('aria-rowcount', '12'); + harness.injector.get(WorkPackageViewCollapsedGroupsService).toggleGroupCollapseState(header().dataset.groupIdentifier!); + expect(header()).toHaveAttribute('aria-rowindex', '2'); + expect(harness.totalsRow()).toHaveAttribute('aria-rowindex', '11'); + expect(table()).toHaveAttribute('aria-rowcount', '12'); + }); + + it('rejects a group header that disagrees with the query rank', async () => { + await mount({ + workPackages: [{ id: '5', attributes: { status: { href: '/api/v3/statuses/1' } } }], + pagination: { offset: 3, pageSize: 2, total: 5 }, + groups: [ + { value: 'G', href: '/api/v3/statuses/1', count: 4 }, + { value: 'H', href: '/api/v3/statuses/2', count: 1 }, + ], + }); + harness.expectNoRowPositions(); + }); + + it('does not infer a flat count for hierarchy mode', async () => { + await mount({ showHierarchies: true }); + expect(table()).not.toHaveAttribute('aria-rowcount'); + expect(harness.row('21')).not.toHaveAttribute('aria-rowindex'); + }); + + it('does not activate capability-off consumers', async () => { + await mount({ configuration: { keyboardNavigationEnabled: false } }); + expect(harness.table.rowPositions).toBeNull(); + expect(table()).not.toHaveAttribute('aria-rowcount'); + }); + + it('counts the genuinely empty message and excludes the drag placeholder', async () => { + await mount({ workPackages: [], pagination: { offset: 1, pageSize: 20, total: 0 }, configuration: { dragAndDropEnabled: true } }); + const empty = document.createElement('tbody'); + empty.className = 'work-package--empty-tbody'; + const row = empty.insertRow(); + row.insertCell().textContent = 'No results'; + harness.tbody.before(empty); + harness.table.rowPositions!.paint(); + expect(row).toHaveAttribute('aria-rowindex', '2'); + expect(harness.totalsRow()).toHaveAttribute('aria-rowindex', '3'); + expect(table()).toHaveAttribute('aria-rowcount', '4'); + expect(harness.tbody.rows[0]).not.toHaveAttribute('aria-rowindex'); + }); + + it('does not label an out-of-range page as an empty query', async () => { + await mount({ workPackages: [], pagination: { offset: 10, pageSize: 20, total: 100 } }); + const empty = document.createElement('tbody'); + empty.className = 'work-package--empty-tbody'; + empty.insertRow().insertCell().textContent = 'No results'; + harness.tbody.before(empty); + harness.table.rowPositions!.paint(); + harness.expectNoRowPositions(); + }); + + it('clears stale values for duplicate, backwards or missing descriptors', async () => { + await mount(); + const variants:ReadonlyMap[] = [ + new Map([['wp:21', 22], ['wp:22', 21]]), + new Map([['wp:21', 21], ['wp:22', 21]]), + new Map([['wp:21', 21]]), + ]; + for (const ordinals of variants) { + harness.table.rowPositions!.commit({ source: harness.querySpace.results.value!, body: { count: 100, ordinals } }); + harness.table.rowPositions!.paint(); + harness.expectNoRowPositions(); + } + }); + + it('clears stale coordinates before a newer collection commits', async () => { + await mount(); + const frames:(() => void)[] = []; + vi.spyOn(harness.table.uiWork, 'frame').mockImplementation((callback) => { frames.push(callback); }); + const rendered = harness.render([{ id: '31' }, { id: '32' }]); + harness.table.rowPositions!.paint(); + expect(table()).not.toHaveAttribute('aria-rowcount'); + expect(harness.row('21')).not.toHaveAttribute('aria-rowindex'); + frames.splice(0).forEach((callback) => callback()); + await rendered; + expect(harness.row('31')).toHaveAttribute('aria-rowindex', '22'); + expect(table()).toHaveAttribute('aria-rowcount', '103'); + }); + + it('paints replacement work-package rows before refresh notification', async () => { + await mount(); + let observed:string|null = null; + harness.table.rowsRefreshed$.subscribe(() => { observed = index(harness.row('21')); }); + const resource = harness.table.originalRowIndex['21'].object; + harness.table.refreshRows(resource); + expect(observed).toBe('22'); + }); + + it('stops painting after teardown', async () => { + await mount(); + await harness.destroy(); + table().removeAttribute('aria-rowcount'); + harness.table.rowPositions!.paint(); + expect(table()).not.toHaveAttribute('aria-rowcount'); + }); + + it('does not treat an offscreen expanded owner as zero additional rows', async () => { + harness = buildTable({ + workPackages: [{ id: '21' }], productionMarkup: true, + pagination: { offset: 2, pageSize: 20, total: 100 }, + columns: ['id', 'subject', { id: 'relationsOfTypeFollows', relationType: 'follows' }], + configuration: { keyboardNavigationEnabled: true }, + }); + harness.expand('999', 'relationsOfTypeFollows'); + await harness.render(); + expect(table()).not.toHaveAttribute('aria-rowcount'); + }); + + describe('while a dragged row is staged', () => { + let prepared:ReturnType>; + let persisted:ReturnType>; + const dragAction = { + prepareDrop: () => prepared.promise.then(() => ({ persist: () => persisted.promise })), + }; + const cleared = () => harness.expectNoRowPositions(); + const mountFlat = () => mount({ workPackages: [{ id: '21' }, { id: '22' }, { id: '23' }], dragAction }); + const staged = (ids:string[]) => waitFor(() => expect(harness.rowIds()).toEqual(ids)); + + beforeEach(() => { + prepared = deferred(); + persisted = deferred(); + }); + + it('withdraws positions until preparation settles and restores them after it fails', async () => { + await mountFlat(); + const dropping = harness.drop('21', '22', 'bottom'); + await staged(['22', '21', '23']); + cleared(); + + prepared.reject(new Error('prepare failed')); + expect(await dropping).toBe(false); + expect(harness.rowIds()).toEqual(['21', '22', '23']); + expectIndices(harness.rows(), ['22', '23', '24']); + expect(table()).toHaveAttribute('aria-rowcount', '103'); + }); + + it('stays withdrawn during persistence and restores positions after it fails', async () => { + await mountFlat(); + const dropping = harness.drop('21', '22', 'bottom'); + await staged(['22', '21', '23']); + prepared.resolve(); + await nextTask(); + cleared(); + + persisted.reject(new Error('persist failed')); + expect(await dropping).toBe(false); + expect(harness.rowIds()).toEqual(['21', '22', '23']); + expectIndices(harness.rows(), ['22', '23', '24']); + }); + + it('waits for a matching collection after a successful drop', async () => { + await mountFlat(); + const rebuilt = harness.nextRender(); + const dropping = harness.drop('21', '22', 'bottom'); + await staged(['22', '21', '23']); + prepared.resolve(); + persisted.resolve(); + expect(await dropping).toBe(true); + await rebuilt; + expect(harness.rowIds()).toEqual(['22', '21', '23']); + cleared(); + + await harness.render([{ id: '22' }, { id: '21' }, { id: '23' }]); + expectIndices(harness.rows(), ['22', '23', '24']); + expect(table()).toHaveAttribute('aria-rowcount', '103'); + }); + + it('keeps the moved row, its selection and tab stop while withdrawn', async () => { + await mountFlat(); + const moved = harness.row('21'); + harness.click('21'); + expect(moved).toHaveAttribute('tabindex', '0'); + const dropping = harness.drop('21', '22', 'bottom'); + await staged(['22', '21', '23']); + expect(harness.row('21')).toBe(moved); + expect(moved).toHaveClass('-checked'); + expect(moved).toHaveAttribute('tabindex', '0'); + prepared.reject(new Error('prepare failed')); + await dropping; + expect(harness.row('21')).toBe(moved); + expect(moved).toHaveClass('-checked'); + expect(moved).toHaveAttribute('tabindex', '0'); + }); + + it('keeps grouped gaps when a staged move is put back', async () => { + const status1 = { href: '/api/v3/statuses/1' }; + const status2 = { href: '/api/v3/statuses/2' }; + await mount({ + workPackages: [ + { id: '3', attributes: { status: status1 } }, + { id: '4', attributes: { status: status1 } }, + { id: '5', attributes: { status: status2 } }, + ], + pagination: { offset: 1, pageSize: 3, total: 5 }, + groups: [ + { value: 'G', href: status1.href, count: 2 }, + { value: 'H', href: status2.href, count: 3 }, + ], + dragAction, + }); + expectIndices(harness.rows(), ['3', '4', '6']); + const dropping = harness.drop('3', '5', 'bottom'); + await staged(['4', '5', '3']); + cleared(); + prepared.reject(new Error('prepare failed')); + await dropping; + expectIndices(harness.rows(), ['3', '4', '6']); + expect(table()).toHaveAttribute('aria-rowcount', '10'); + }); + + it('does not paint a torn-down table when a staged move settles', async () => { + await mountFlat(); + const dropping = harness.drop('21', '22', 'bottom'); + await staged(['22', '21', '23']); + await harness.destroy(); + prepared.reject(new Error('prepare failed')); + expect(await dropping).toBe(false); + expect(harness.rowIds()).toEqual(['22', '21', '23']); + cleared(); + }); + }); + + describe('with the children column', () => { + const children = { id: 'children', children: true } as const; + + it('does not treat an offscreen expanded owner as zero additional rows', async () => { + await mount({ workPackages: [{ id: '21' }], columns: ['id', 'subject', children] }); + harness.expand('999', 'children'); + await harness.render(); + expect(table()).not.toHaveAttribute('aria-rowcount'); + expect(harness.row('21')).not.toHaveAttribute('aria-rowindex'); + }); + + it('stays absent while expanded children load and after they appear', async () => { + let resolveLoad!:() => void; + const loaded = new Promise((resolve) => { resolveLoad = resolve; }); + harness = buildTable({ + workPackages: [{ id: '21', children: [{ id: '23' }] }, { id: '22' }], + productionMarkup: true, + pagination: { offset: 2, pageSize: 20, total: 100 }, + columns: ['id', 'subject', children], + loadChildren: false, + configuration: { keyboardNavigationEnabled: true }, + requireAll: async (ids) => { + await loaded; + return ids.map((id) => { + const child = buildWorkPackage({ id }); + harness.injector.get(States).workPackages.get(id).putValue(child); + return child; + }); + }, + }); + harness.expand('21', 'children'); + await harness.render(); + expect(table()).not.toHaveAttribute('aria-rowcount'); + expect(harness.row('21')).not.toHaveAttribute('aria-rowindex'); + + const redrawn = harness.nextRender(); + resolveLoad(); + await redrawn; + expect(harness.relationRow('21', '23', 'children')).toBeInTheDocument(); + harness.expectNoRowPositions(); + }); + + it('keeps global positions when nothing is expanded', async () => { + await mount({ columns: ['id', 'subject', children] }); + expect(harness.row('21')).toHaveAttribute('aria-rowindex', '22'); + expect(table()).toHaveAttribute('aria-rowcount', '103'); + }); + }); + + + it('publishes indices from the accepted render instead of a superseded frame', async () => { + await mount(); + const frames:(() => void)[] = []; + vi.spyOn(harness.table.uiWork, 'frame').mockImplementation((callback) => { frames.push(callback); }); + const first = harness.render([{ id: '31' }]); + const second = harness.render([{ id: '41' }, { id: '42' }]); + const publications:(string|null)[][] = []; + const subscription = harness.querySpace.tableRendered.values$().subscribe(() => { + publications.push(harness.rows().map(index)); + }); + frames.splice(0).forEach((callback) => callback()); + await Promise.all([first, second]); + subscription.unsubscribe(); + expect(harness.rowIds()).toEqual(['41', '42']); + expect(publications.at(-1)).toEqual(['22', '23']); + }); + + it('clears indices when group counts do not match the query total', async () => { + const status = { href: '/api/v3/statuses/1' }; + await mount({ + workPackages: [{ id: '1', attributes: { status } }], + pagination: { offset: 1, pageSize: 20, total: 10 }, + groups: [{ value: 'G', href: status.href, count: 2 }], + }); + expect(table()).not.toHaveAttribute('aria-rowcount'); + expect(harness.row('1')).not.toHaveAttribute('aria-rowindex'); + }); + + it.each([ + '/api/v3/work_packages?offset=1', + '/api/v3/projects/12/work_packages', + '/api/v3/workspaces/12/work_packages?pageSize=20', + ])('trusts grouped aggregates of the primary collection %s', async (collectionHref) => { + const status = { href: '/api/v3/statuses/1' }; + await mount({ + workPackages: [{ id: '1', attributes: { status } }], + pagination: { offset: 1, pageSize: 20, total: 1 }, + groups: [{ value: 'G', href: status.href, count: 1 }], + collectionHref, + }); + expect(table()).toHaveAttribute('aria-rowcount', '5'); + }); + + it('does not use grouped aggregates of a scoped collection', async () => { + const status = { href: '/api/v3/statuses/1' }; + await mount({ + workPackages: [{ id: '1', attributes: { status } }], + pagination: { offset: 1, pageSize: 20, total: 1 }, + groups: [{ value: 'G', href: status.href, count: 1 }], + collectionHref: '/api/v3/versions/3/work_packages', + }); + harness.expectNoRowPositions(); + }); + + it('does not use grouped aggregates while the baseline is active', async () => { + const status = { href: '/api/v3/statuses/1' }; + await mount({ + workPackages: [{ id: '1', attributes: { status } }], + pagination: { offset: 1, pageSize: 20, total: 1 }, + groups: [{ value: 'G', href: status.href, count: 1 }], + baseline: true, + }); + harness.expectNoRowPositions(); + }); + + it('withdraws positions while the create section exposes two rows', async () => { + await mount(); + (createRow().parentElement as HTMLTableSectionElement).insertRow().insertCell().textContent = 'Editing'; + harness.table.rowPositions!.paint(); + harness.expectNoRowPositions(); + }); + + it('leaves attributes untouched when values are unchanged', async () => { + await mount(); + const records:MutationRecord[] = []; + const observer = new MutationObserver((batch) => records.push(...batch)); + observer.observe(table(), { subtree: true, attributes: true }); + harness.table.rowPositions!.paint(); + await nextTask(); + observer.disconnect(); + expect(records).toEqual([]); + }); + + it('keeps sibling metadata outside this projection', async () => { + await mount(); + const row = harness.row('21'); + row.setAttribute('aria-posinset', '21'); + row.setAttribute('aria-setsize', '100'); + harness.table.rowPositions!.paint(); + expect(row).toHaveAttribute('aria-posinset', '21'); + expect(row).toHaveAttribute('aria-setsize', '100'); + expect(harness.totalsRow()).not.toHaveAttribute('aria-posinset'); + }); + +}); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/table-row-positions.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/table-row-positions.ts new file mode 100644 index 000000000000..b3ce9469d874 --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/table-row-positions.ts @@ -0,0 +1,198 @@ +//-- 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 { Injector } from '@angular/core'; +import { WorkPackageViewHierarchiesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-hierarchy.service'; +import { WorkPackageViewRelationColumnsService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-relation-columns.service'; +import { WorkPackageViewColumnsService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-columns.service'; +import { WorkPackageViewBaselineService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-baseline.service'; +import type { GroupObject, WorkPackageCollectionResource } from 'core-app/features/hal/resources/wp-collection-resource'; +import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; +import { groupOccurrenceKey, placeholderOccurrenceKey, wpOccurrenceKey, type RenderDraft, type RenderedOccurrence } from '../rendered-occurrence-ledger'; +import type { WorkPackageTable } from '../wp-fast-table'; +import { isEmptyResultsRow, isExposedRow, isInlineCreateRow, isTotalsRow } from '../helpers/structural-rows'; +import { bodyRowPositions, draftAgrees, type BodyRowPositions, type PositionGroup } from './body-row-positions'; + +export interface CapturedRowPositions { + readonly source:WorkPackageCollectionResource; + readonly body:BodyRowPositions; +} + +// Group counts are computed from the unscoped query results, so only the primary collections carry trustworthy aggregates. +const UNSCOPED_COLLECTION_HREF = /\/api\/v3\/(?:(?:projects|workspaces)\/[^/]+\/)?work_packages(?:\?|$)/; + +type ProjectedRowKind = 'ledger'|'skipped'|'empty'|'totals'|'create'|'foreign'; + +function setIfChanged(element:Element, name:string, value:string|null):void { + if (value === null) { + if (element.hasAttribute(name)) element.removeAttribute(name); + } else if (element.getAttribute(name) !== value) { + element.setAttribute(name, value); + } +} + +export class TableRowPositions { + @LazyInject(WorkPackageViewHierarchiesService) private readonly hierarchies:WorkPackageViewHierarchiesService; + + @LazyInject(WorkPackageViewColumnsService) private readonly columns:WorkPackageViewColumnsService; + + @LazyInject(WorkPackageViewRelationColumnsService) private readonly relationColumns:WorkPackageViewRelationColumnsService; + + @LazyInject(WorkPackageViewBaselineService) private readonly baseline:WorkPackageViewBaselineService; + + public readonly injector:Injector = this.table.injector; + + private snapshot:CapturedRowPositions|null = null; + + constructor(private readonly table:WorkPackageTable) {} + + public capture(draft:RenderDraft):CapturedRowPositions|null { + const source = this.table.querySpace.results.value; + if (!source || this.withdrawnByMode() || !this.matchesRenderedRows(source)) return null; + + const groups = this.table.querySpace.groups.value ?? []; + if (groups.length > 0 && !this.trustsGroupedAggregates(source, groups)) return null; + + const positionGroups:PositionGroup[] = groups.map((group, index) => ({ + headerKey: groupOccurrenceKey(index, 'header'), + sumsKey: groupOccurrenceKey(index, 'sums'), + count: group.count, + hasSums: !!group.sums, + })); + const body = bodyRowPositions({ + pageNumber: source.offset, + pageSize: source.pageSize, + total: source.total, + primaryKeys: this.table.originalRows.map(wpOccurrenceKey), + groups: positionGroups, + }); + if (!body) return null; + + const rows = draft.occurrences + .filter((occurrence) => occurrence.key !== placeholderOccurrenceKey()) + .map((occurrence) => ({ key: occurrence.key, member: occurrence.workPackageId !== null })); + return draftAgrees(body, rows, positionGroups) ? { source, body } : null; + } + + public commit(snapshot:CapturedRowPositions|null):void { + this.snapshot = snapshot; + } + + /** Producers call this synchronously after every imperative row move or structural change, so no frame exposes a stale index. */ + public paint():void { + if (this.table.destroyed) return; + const element = this.table.tbody.closest('table'); + if (!element) return; + const rows = Array.from(element.rows); + const projection = this.project(element, rows); + setIfChanged(element, 'aria-rowcount', projection ? String(projection.count) : null); + rows.forEach((row) => { + const position = projection?.indices.get(row); + setIfChanged(row, 'aria-rowindex', position === undefined ? null : String(position)); + }); + } + + private withdrawnByMode():boolean { + if (this.hierarchies.isEnabled) return true; + return (this.columns.hasRelationColumns() || this.columns.hasChildRelationsColumn()) + && Object.keys(this.relationColumns.current).length > 0; + } + + private matchesRenderedRows(source:WorkPackageCollectionResource):boolean { + const { originalRows } = this.table; + return source.count === originalRows.length + && source.elements.length === originalRows.length + && source.elements.every((wp, index) => wp.id === originalRows[index]); + } + + private trustsGroupedAggregates(source:WorkPackageCollectionResource, groups:GroupObject[]):boolean { + return source.groups === groups + && !this.baseline.isActive() + && UNSCOPED_COLLECTION_HREF.test(source.href ?? ''); + } + + private project(element:HTMLTableElement, rows:HTMLTableRowElement[]) { + const snapshot = this.snapshot; + if (!snapshot || snapshot.source !== this.table.querySpace.results.value) return null; + const { body } = snapshot; + const header = element.tHead?.rows; + if (header?.length !== 1) return null; + + const occurrences = new Map(this.table.ledger.occurrences + .filter((occurrence) => occurrence.element !== null) + .map((occurrence) => [occurrence.element!, occurrence])); + const indices = new Map([[header[0], 1]]); + const trailers:HTMLTableRowElement[] = []; + const seen = new Set(); + let emptySurrogate:HTMLTableRowElement|null = null; + for (const row of rows) { + if (row === header[0]) continue; + const kind = this.classify(row, occurrences); + if (kind === 'skipped') continue; + if (kind === 'ledger') { + const ordinal = body.ordinals.get(occurrences.get(row)!.key); + if (ordinal === undefined) return null; + indices.set(row, ordinal + 1); + } else if (kind === 'empty') { + if (body.count !== 0 || emptySurrogate) return null; + emptySurrogate = row; + } else if (kind === 'totals' || kind === 'create') { + if (seen.has(kind)) return null; + seen.add(kind); + trailers.push(row); + } else { + return null; + } + } + + let count = 1 + body.count; + for (const row of [...(emptySurrogate ? [emptySurrogate] : []), ...trailers]) { + count += 1; + indices.set(row, count); + } + if (!Number.isSafeInteger(count)) return null; + + let previous = 0; + for (const row of rows) { + const position = indices.get(row); + if (position === undefined) continue; + if (!Number.isSafeInteger(position) || position <= previous || position > count) return null; + previous = position; + } + return { count, indices }; + } + + private classify(row:HTMLTableRowElement, occurrences:Map):ProjectedRowKind { + const occurrence = occurrences.get(row); + if (occurrence) return occurrence.key === placeholderOccurrenceKey() ? 'skipped' : 'ledger'; + if (!isExposedRow(row)) return 'skipped'; + if (isEmptyResultsRow(row)) return 'empty'; + if (isTotalsRow(this.table, row)) return row.cells.length === 0 ? 'skipped' : 'totals'; + if (isInlineCreateRow(row)) return row.cells.length === 0 ? 'skipped' : 'create'; + return 'foreign'; + } +} 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 dcc87c971882..27fa2a4b482f 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 @@ -117,6 +117,9 @@ export type RelationColumnSpec = { id:string, relationType:string }|{ id:string, export interface NonSortableColumnSpec { id:string, sortable:false } export interface TableHarnessOptions { + pagination?:{ offset:number; pageSize:number; total:number }; + /** Self link of the results collection; defaults to the global work packages collection. */ + collectionHref?:string; workPackages:WorkPackageFixture[]; providers?:Provider[]; /** Retains the root so a replacement table can mount with a fresh injector. */ @@ -178,6 +181,10 @@ export interface TableHarness { nextRender():Promise; /** Every rendered work-package row, relation rows included. */ rows():HTMLTableRowElement[]; + /** Asserts the `aria-rowindex` of every table row in document order and the table's `aria-rowcount`; requires `productionMarkup`. */ + expectRowPositions(indices:string[], count:string):void; + /** Asserts that neither the table nor any of its rows carries a position. */ + expectNoRowPositions():void; /** The primary row of a work package, never one of its relation rows. */ row(workPackageId:string):HTMLTableRowElement; /** The relation row of `to` under `from`, located by its occurrence key. */ @@ -301,7 +308,16 @@ export function buildTable(options:TableHarnessOptions):TableHarness { } const rendered = nextRender(); - querySpace.results.putValue({ elements: resources, sumsSchema: { href: sumsSchemaHref } } as WorkPackageCollectionResource); + querySpace.results.putValue({ + elements: resources, + groups: querySpace.groups.value ?? [], + href: options.collectionHref ?? '/api/v3/work_packages', + count: resources.length, + total: options.pagination?.total ?? resources.length, + pageSize: options.pagination?.pageSize ?? Math.max(resources.length, 1), + offset: options.pagination?.offset ?? 1, + sumsSchema: { href: sumsSchemaHref }, + } as WorkPackageCollectionResource); querySpace.initialized.putValue(null); return rendered; @@ -313,6 +329,18 @@ export function buildTable(options:TableHarnessOptions):TableHarness { return Array.from(dom.tbody.querySelectorAll('tr.wp-table--row')); }, + expectRowPositions(indices, count) { + const element = dom.tbody.closest('table')!; + expect(Array.from(element.rows).map((row) => row.getAttribute('aria-rowindex'))).toEqual(indices); + expect(element).toHaveAttribute('aria-rowcount', count); + }, + + expectNoRowPositions() { + const element = dom.tbody.closest('table')!; + expect(element).not.toHaveAttribute('aria-rowcount'); + Array.from(element.rows).forEach((row) => expect(row).not.toHaveAttribute('aria-rowindex')); + }, + row(workPackageId) { const row = dom.tbody.querySelector(`tr.wp-table--row:not(.wp-table--relations-additional-row)[data-work-package-id="${workPackageId}"]`); if (!row) { @@ -338,7 +366,7 @@ export function buildTable(options:TableHarnessOptions):TableHarness { }, totalsRow() { - return productionMarkupElement(dom.wrapper, 'tr.wp-table--sums-row'); + return productionMarkupElement(dom.wrapper, '.work-package-table > tbody:not(.work-package--results-tbody) > tr.wp-table--sums-row'); }, timelineRow(workPackageId) { 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 e5cf0c618d60..caecf3072494 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,11 +49,13 @@ 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 { TableRowPositions, type CapturedRowPositions } from './semantics/table-row-positions'; import { TreegridSemantics } from './semantics/treegrid-semantics'; interface PendingRender { readonly pass:PrimaryRenderPass; readonly timeline:boolean; + readonly positions:CapturedRowPositions|null; } export class WorkPackageTable { @@ -100,6 +102,8 @@ export class WorkPackageTable { public readonly semantics:TreegridSemantics|null = this.configuration.keyboardNavigationEnabled ? new TreegridSemantics(this) : null; + public readonly rowPositions:TableRowPositions|null = this.configuration.keyboardNavigationEnabled ? new TableRowPositions(this) : null; + 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. */ @@ -252,6 +256,7 @@ export class WorkPackageTable { } public notifyRowsRefreshed(key:string):void { + this.rowPositions?.paint(); this.rowsRefreshedSubject.next(key); } @@ -268,7 +273,8 @@ export class WorkPackageTable { this.keyboardNavigation?.captureOwnership(); this.editing.reset(); const promoted = timeline || this.pendingRender?.timeline === true; - const pending:PendingRender = { pass: this.rowBuilder.buildRows({ timeline: promoted }), timeline: promoted }; + const pass = this.rowBuilder.buildRows({ timeline: promoted }); + const pending:PendingRender = { pass, timeline: promoted, positions: this.rowPositions?.capture(pass.draft) ?? null }; this.pendingRender = pending; this.uiWork.frame(() => this.commitRender(pending)); } @@ -283,12 +289,14 @@ export class WorkPackageTable { this.timelineBody.replaceChildren(timelinePass.timelineBody); } this.ledger.commit(pass.draft); + this.rowPositions?.commit(pending.positions); this.lastRenderPass = pass; this.publishRendered(); } private publishRendered():void { this.semantics?.paint(); + this.rowPositions?.paint(); this.querySpace.tableRendered.putValue(this.ledger.snapshot()); } From 341695571fbfb7e50a449fd04be9cb64d11e103b Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 06:20:58 +0100 Subject: [PATCH 4/8] [DREAM-938] Refresh structural row positions Repaints global positions after Angular header, totals and creation rows change exposure or content. Covers asynchronous schema updates, replacement tables, permissions and active creation input identity. https://community.openproject.org/wp/DREAM-938 --- .../components/wp-fast-table/wp-fast-table.ts | 9 +- .../wp-inline-create.component.ts | 5 +- .../wp-table-sums-row.directive.spec.ts | 40 +++++++ .../wp-table-sums-row.directive.ts | 22 ++-- .../wp-table/wp-table.component.spec.ts | 110 +++++++++++++++++- .../components/wp-table/wp-table.component.ts | 7 +- 6 files changed, 178 insertions(+), 15 deletions(-) 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 caecf3072494..7b81a121f7c1 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 @@ -50,7 +50,7 @@ import { WorkPackageTableEditingContext } from './wp-table-editing'; import type { WorkPackageTableRow } from './wp-table.interfaces'; import type { KeyboardNavigation } from './handlers/state/keyboard/keyboard-navigation-transformer'; import { TableRowPositions, type CapturedRowPositions } from './semantics/table-row-positions'; -import { TreegridSemantics } from './semantics/treegrid-semantics'; +import { paintSectionExposure, TreegridSemantics } from './semantics/treegrid-semantics'; interface PendingRender { readonly pass:PrimaryRenderPass; @@ -260,6 +260,13 @@ export class WorkPackageTable { this.rowsRefreshedSubject.next(key); } + /** Re-exposes a structural section whose rows changed outside the render pass. */ + public reconcileSection(section:HTMLTableSectionElement):void { + if (!this.semantics) return; + paintSectionExposure(section); + this.rowPositions?.paint(); + } + /** * Determine whether we need an empty placeholder row. * When D&D is enabled, the table requires a drag target that is non-empty, diff --git a/frontend/src/app/features/work-packages/components/wp-inline-create/wp-inline-create.component.ts b/frontend/src/app/features/work-packages/components/wp-inline-create/wp-inline-create.component.ts index 27b5b034eb3c..efda1990a0f5 100644 --- a/frontend/src/app/features/work-packages/components/wp-inline-create/wp-inline-create.component.ts +++ b/frontend/src/app/features/work-packages/components/wp-inline-create/wp-inline-create.component.ts @@ -57,7 +57,6 @@ import { HalResourceEditingService, } from 'core-app/shared/components/fields/edit/services/hal-resource-editing.service'; import { delegate, DelegateEvent } from '@knowledgecode/delegate'; -import { paintSectionExposure } from '../wp-fast-table/semantics/treegrid-semantics'; @Component({ // eslint-disable-next-line @angular-eslint/component-selector @@ -237,7 +236,7 @@ export class WorkPackageInlineCreateComponent extends UntilDestroyedMixin implem } public addWorkPackageRow() { - this.wpCreate + void this.wpCreate .createOrContinueWorkPackage(this.projectIdentifier) .then((change:WorkPackageChangeset) => { const wp = this.currentWorkPackage = change.projectedResource; @@ -349,7 +348,7 @@ export class WorkPackageInlineCreateComponent extends UntilDestroyedMixin implem private reconcileExposure():void { if (this.treegrid) { - paintSectionExposure(this.element); + this.table.reconcileSection(this.element); } } } diff --git a/frontend/src/app/features/work-packages/components/wp-table/wp-table-sums-row/wp-table-sums-row.directive.spec.ts b/frontend/src/app/features/work-packages/components/wp-table/wp-table-sums-row/wp-table-sums-row.directive.spec.ts index f56f699d9220..544a7727a693 100644 --- a/frontend/src/app/features/work-packages/components/wp-table/wp-table-sums-row/wp-table-sums-row.directive.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-table/wp-table-sums-row/wp-table-sums-row.directive.spec.ts @@ -243,6 +243,25 @@ describe('WorkPackageTableSumsRowController semantics', () => { vi.restoreAllMocks(); }); + it('repaints positions once per update and before the row notification', async () => { + const harness = await setUp({ keyboardNavigationEnabled: true }); + const events:string[] = []; + vi.spyOn(harness.table.rowPositions!, 'paint').mockImplementation(() => { + events.push(`paint:${row.getAttribute('aria-hidden')}:${row.cells.length > 0}`); + expect(row).toHaveAttribute('role', 'row'); + }); + harness.table.rowsRefreshed$.subscribe(() => events.push('notify')); + attach(harness, true); + events.length = 0; + + await resolveSchema(); + + expect(events).toEqual(['paint:null:true', 'notify']); + events.length = 0; + directive.wpTableSums.setEnabled(false); + expect(events).toEqual(['paint:true:false', 'notify']); + }); + it('paints roles and exposure before notifying the table', async () => { const harness = await setUp({ keyboardNavigationEnabled: true }); const painted = firstValueFrom(harness.table.rowsRefreshed$.pipe(take(1), map((key) => ({ @@ -291,15 +310,34 @@ describe('WorkPackageTableSumsRowController semantics', () => { expect(section()).toHaveAttribute('aria-hidden', 'true'); }); + it('repaints only the replacement table after a pending schema resolves', async () => { + const previous = await setUp({ keyboardNavigationEnabled: true }); + attach(previous, true); + const previousPaint = vi.spyOn(previous.table.rowPositions!, 'paint'); + const replacement = buildTable({ workPackages: [], configuration: { keyboardNavigationEnabled: true } }); + tables.push(replacement); + directive.workPackageTable = replacement.table; + const replacementPaint = vi.spyOn(replacement.table.rowPositions!, 'paint'); + previous.table.destroy(); + + await resolveSchema(); + + expect(previousPaint).not.toHaveBeenCalled(); + expect(replacementPaint).toHaveBeenCalled(); + expect(row).not.toHaveAttribute('aria-hidden'); + }); + it('stays hidden when the schema resolves after sums were disabled', async () => { const harness = await setUp({ keyboardNavigationEnabled: true }); const refreshed = vi.fn(); harness.table.rowsRefreshed$.subscribe(refreshed); attach(harness, true); directive.wpTableSums.setEnabled(false); + const painted = vi.spyOn(harness.table.rowPositions!, 'paint'); await resolveSchema(); + expect(painted).not.toHaveBeenCalled(); expect(row).toBeEmptyDOMElement(); expect(row).toHaveAttribute('aria-hidden', 'true'); expect(section()).toHaveAttribute('aria-hidden', 'true'); @@ -310,9 +348,11 @@ describe('WorkPackageTableSumsRowController semantics', () => { const harness = await setUp({ keyboardNavigationEnabled: true }); attach(harness, true); harness.table.destroy(); + const painted = vi.spyOn(harness.table.rowPositions!, 'paint'); await resolveSchema(); + expect(painted).not.toHaveBeenCalled(); expect(row).toBeEmptyDOMElement(); expect(row).toHaveAttribute('aria-hidden', 'true'); expect(section()).toHaveAttribute('aria-hidden', 'true'); diff --git a/frontend/src/app/features/work-packages/components/wp-table/wp-table-sums-row/wp-table-sums-row.directive.ts b/frontend/src/app/features/work-packages/components/wp-table/wp-table-sums-row/wp-table-sums-row.directive.ts index d47959defebc..5c8ca0d1fb73 100644 --- a/frontend/src/app/features/work-packages/components/wp-table/wp-table-sums-row/wp-table-sums-row.directive.ts +++ b/frontend/src/app/features/work-packages/components/wp-table/wp-table-sums-row/wp-table-sums-row.directive.ts @@ -38,7 +38,6 @@ import { combineLatest, ReplaySubject } from 'rxjs'; import { GroupSumsBuilder } from 'core-app/features/work-packages/components/wp-fast-table/builders/modes/grouped/group-sums-builder'; import { WorkPackageTable } from 'core-app/features/work-packages/components/wp-fast-table/wp-fast-table'; import { SchemaCacheService } from 'core-app/core/schemas/schema-cache.service'; -import { SchemaResource } from 'core-app/features/hal/resources/schema-resource'; import { WorkPackageCollectionResource } from 'core-app/features/hal/resources/wp-collection-resource'; import { totalsRowKey } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/navigable-rows'; import { keyboardNavigationEnabled } from 'core-app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/roving-tabindex'; @@ -115,24 +114,28 @@ export class WorkPackageTableSumsRowController implements AfterViewInit { if (sum && resource.sumsSchema) { void this.schemaCache .ensureLoaded(resource.sumsSchema.href!) - .then((schema:SchemaResource) => { + .then(() => { if (table.destroyed || this.destroyRef.destroyed || this.workPackageTable !== table || !this.wpTableSums.isEnabled) return; table.keyboardNavigation?.captureOwnership(); - this.refresh(columns, resource, schema); - this.reconcileSemantics(table); + this.refresh(columns, resource); + this.paintSemantics(table); table.notifyRowsRefreshed(totalsRowKey); }); } else if (this.element.hasChildNodes()) { table.keyboardNavigation?.captureOwnership(); this.clear(); - this.reconcileSemantics(table); + this.paintSemantics(table); table.notifyRowsRefreshed(totalsRowKey); } }); } private reconcileSemantics(table:WorkPackageTable) { - if (table.destroyed || !keyboardNavigationEnabled(table.configuration)) return; + if (this.paintSemantics(table)) table.rowPositions?.paint(); + } + + private paintSemantics(table:WorkPackageTable):boolean { + if (table.destroyed || !keyboardNavigationEnabled(table.configuration)) return false; if (this.isHidden || this.element.cells.length === 0) { this.element.setAttribute('aria-hidden', 'true'); @@ -141,18 +144,19 @@ export class WorkPackageTableSumsRowController implements AfterViewInit { } paintRowRole(this.element, 1); paintSectionExposure(this.element.parentElement as HTMLTableSectionElement); + return true; } private clear() { this.element.innerHTML = ''; } - private refresh(columns:QueryColumn[], resource:WorkPackageCollectionResource, schema:SchemaResource) { + private refresh(columns:QueryColumn[], resource:WorkPackageCollectionResource) { this.clear(); - this.render(columns, resource, schema); + this.render(columns, resource); } - private render(columns:QueryColumn[], resource:WorkPackageCollectionResource, schema:SchemaResource) { + private render(columns:QueryColumn[], resource:WorkPackageCollectionResource) { this.groupSumsBuilder = new GroupSumsBuilder(this.injector, this.workPackageTable); this.groupSumsBuilder.text = this.text; this.groupSumsBuilder.renderColumns(resource.totalSums!, this.element, 'total'); diff --git a/frontend/src/app/features/work-packages/components/wp-table/wp-table.component.spec.ts b/frontend/src/app/features/work-packages/components/wp-table/wp-table.component.spec.ts index 4626a109819d..49f534298270 100644 --- a/frontend/src/app/features/work-packages/components/wp-table/wp-table.component.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-table/wp-table.component.spec.ts @@ -498,7 +498,7 @@ describe('WorkPackagesTableComponent treegrid structure', () => { function putResults(fixtures:WorkPackageFixture[]) { const resources = fixtures.map(buildWorkPackage); resources.forEach((wp) => states.workPackages.get(wp.id!).putValue(wp)); - querySpace.results.putValue({ elements: resources, count: resources.length, total: resources.length } as unknown as WorkPackageCollectionResource); + querySpace.results.putValue({ elements: resources, offset: 1, pageSize: 20, count: resources.length, total: resources.length } as unknown as WorkPackageCollectionResource); } function tables():WorkPackagesTableComponent[] { @@ -592,6 +592,19 @@ describe('WorkPackagesTableComponent treegrid structure', () => { }); }); + it('repaints positions after Angular updates the header', async () => { + const component = await mount(enabled); + const painted:string[] = []; + vi.spyOn(component.workPackageTable.rowPositions!, 'paint').mockImplementation(() => { + painted.push(headerRow().getAttribute('aria-label') ?? headerRow().textContent ?? ''); + expect(headerRow().querySelector('[aria-sort="descending"]')).not.toBeNull(); + }); + + sortBy(['subject', QUERY_SORT_BY_DESC]); + + expect(painted).not.toHaveLength(0); + }); + describe('header structure', () => { it('exposes a level 1 header row inside a rowgroup', async () => { await mount(enabled); @@ -792,6 +805,101 @@ describe('WorkPackagesTableComponent treegrid structure', () => { expect(within(row).getAllByRole('button')).toHaveLength(2); }); + it('repaints positions after permission and reference template changes', async () => { + canReference.next(true); + const component = await mount(inlineCreate); + const painted = vi.spyOn(component.workPackageTable.rowPositions!, 'paint'); + painted.mockImplementation(() => { + expect(createSection()).not.toHaveAttribute('aria-hidden'); + }); + + fireEvent.click(within(createSection()).getByText('Reference an existing work package')); + expect(painted).toHaveBeenCalled(); + expect(within(createSection()).getByRole('textbox', { name: 'Work package to reference' })).toBeInTheDocument(); + const exposure:(string|null)[] = []; + painted.mockImplementation(() => exposure.push(createSection().getAttribute('aria-hidden'))); + canAdd.next(false); + canReference.next(false); + expect(exposure).toEqual([null, 'true']); + }); + + it('updates absolute positions through create and permission changes', async () => { + const component = await mount(inlineCreate); + const table = component.workPackageTable.tbody.closest('table')!; + expect(table).toHaveAttribute('aria-rowcount', '4'); + expect(within(createSection()).getByRole('row')).toHaveAttribute('aria-rowindex', '4'); + + const row = await openCreateRow(); + expect(table).toHaveAttribute('aria-rowcount', '4'); + expect(row).toHaveAttribute('aria-rowindex', '4'); + const editor = within(row).getByRole('textbox'); + editor.focus(); + component.workPackageTable.rowPositions!.paint(); + expect(document.activeElement).toBe(editor); + + fireEvent.keyDown(editor, { key: 'Escape' }); + await waitFor(() => expectExposed()); + expect(within(createSection()).getByRole('row')).toHaveAttribute('aria-rowindex', '4'); + canAdd.next(false); + expect(table).toHaveAttribute('aria-rowcount', '3'); + expect(createSection()).toHaveAttribute('aria-hidden', 'true'); + }); + + it('withdraws positions while the button row and the editing row coexist', async () => { + const component = await mount(inlineCreate); + const table = component.workPackageTable.tbody.closest('table')!; + const positions = component.workPackageTable.rowPositions!; + const paint = positions.paint.bind(positions); + const counts:(string|null)[] = []; + vi.spyOn(positions, 'paint').mockImplementation(() => { + paint(); + if (createSection().querySelectorAll('tr').length === 2) counts.push(table.getAttribute('aria-rowcount')); + }); + + await openCreateRow(); + + expect(counts).not.toHaveLength(0); + expect(new Set(counts)).toEqual(new Set([null])); + }); + + it('stops repainting a removed table on permission changes', async () => { + const component = await mount(inlineCreate); + const painted = vi.spyOn(component.workPackageTable.rowPositions!, 'paint'); + const listView = fixture as ComponentFixture; + listView.componentInstance.showTable.set(false); + listView.detectChanges(); + painted.mockClear(); + + canAdd.next(false); + canReference.next(true); + await nextTask(); + + expect(painted).not.toHaveBeenCalled(); + }); + + it('repaints positions after imperative create append and removal', async () => { + const component = await mount(inlineCreate); + const painted = vi.spyOn(component.workPackageTable.rowPositions!, 'paint'); + painted.mockImplementation(() => { + expect(createRow()).toBeInTheDocument(); + }); + const row = await openCreateRow(); + expect(painted).toHaveBeenCalled(); + const editor = within(row).getByRole('textbox'); + editor.focus(); + const focused = document.activeElement; + painted.mockRestore(); + component.workPackageTable.rowPositions!.paint(); + expect(document.activeElement).toBe(focused); + + const removed = vi.spyOn(component.workPackageTable.rowPositions!, 'paint').mockImplementation(() => { + expect(createRow()).not.toBeInTheDocument(); + }); + fireEvent.keyDown(editor, { key: 'Escape' }); + expect(removed).toHaveBeenCalled(); + await waitFor(() => expectExposed()); + }); + it('renders the active reference row with the same semantics', async () => { canReference.next(true); await mount(inlineCreate); diff --git a/frontend/src/app/features/work-packages/components/wp-table/wp-table.component.ts b/frontend/src/app/features/work-packages/components/wp-table/wp-table.component.ts index f58bd8fd808c..11f09f1196a4 100644 --- a/frontend/src/app/features/work-packages/components/wp-table/wp-table.component.ts +++ b/frontend/src/app/features/work-packages/components/wp-table/wp-table.component.ts @@ -238,7 +238,7 @@ export class WorkPackagesTableComponent extends UntilDestroyedMixin implements O this.scrollSync.update(timelines.visible); } - this.cdRef.detectChanges(); + this.renderHeader(); }); this.cdRef.detectChanges(); @@ -293,7 +293,12 @@ export class WorkPackagesTableComponent extends UntilDestroyedMixin implements O hover.activate(); scroll.update(this.timelineVisible); + this.renderHeader(); + } + + private renderHeader():void { this.cdRef.detectChanges(); + this.workPackageTable?.rowPositions?.paint(); } public get isEmbedded() { From 6672abe94c88916d86a6ec89d6d60175d3f38bb3 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 07:30:42 +0100 Subject: [PATCH 5/8] [DREAM-938] Cover flat table row positions Adds page-object helpers that assert the table's row count and every exposed row's index in document order, or that both are absent. The flat treegrid example now checks global positions including totals and Create, their removal while a children relation is expanded and their return once it collapses. https://community.openproject.org/wp/DREAM-938 --- .../table/treegrid_semantics_spec.rb | 117 +++++++++--------- .../work_packages/work_packages_table.rb | 21 ++++ 2 files changed, 79 insertions(+), 59 deletions(-) diff --git a/spec/features/work_packages/table/treegrid_semantics_spec.rb b/spec/features/work_packages/table/treegrid_semantics_spec.rb index 738c74ef4924..a6642b99507d 100644 --- a/spec/features/work_packages/table/treegrid_semantics_spec.rb +++ b/spec/features/work_packages/table/treegrid_semantics_spec.rb @@ -69,32 +69,35 @@ it "exposes rows, sort, totals, selection and the relation button" do wp_table.visit_query(query) wp_table.expect_work_package_order(child, parent) - - expect(treegrid).to match_selector(:table, accessible_description: "Sorted by Subject ascending.", exact: true) - expect(treegrid).to have_selector(:row, "Subject", aria: { level: 1, selected: nil, expanded: nil }) - expect(treegrid).to have_selector(:columnheader, "Subject", aria: { sort: "ascending" }) - expect(treegrid).to have_selector(:columnheader, "ID", aria: { sort: nil }) - expect(treegrid).to have_selector(:row, "Parent task", aria: { level: 1, selected: false, expanded: nil }) - expect(treegrid).to have_selector(:row, accessible_name: /Total sum/, aria: { level: 1, selected: nil }) - expect(treegrid).to have_selector(:gridcell, accessible_description: "Total sum") + wp_table.expect_row_positions(rowcount: 5, rows: [1, 2, 3, 4, 5]) + + expect(wp_table.treegrid).to match_selector(:table, accessible_description: "Sorted by Subject ascending.", exact: true) + expect(wp_table.treegrid).to have_selector(:row, "Subject", aria: { level: 1, selected: nil, expanded: nil }) + expect(wp_table.treegrid).to have_selector(:columnheader, "Subject", aria: { sort: "ascending" }) + expect(wp_table.treegrid).to have_selector(:columnheader, "ID", aria: { sort: nil }) + expect(wp_table.treegrid).to have_selector(:row, "Parent task", aria: { level: 1, selected: false, expanded: nil }) + expect(wp_table.treegrid).to have_selector(:row, accessible_name: /Total sum/, aria: { level: 1, selected: nil }) + expect(wp_table.treegrid).to have_selector(:gridcell, accessible_description: "Total sum") expect_axe_clean_table wp_table.focus_row_by_keyboard(parent) wp_table.press_keys(:enter) - expect(treegrid).to have_selector(:row, "Parent task", aria: { selected: true }) - expect(treegrid).to have_selector(:row, "Child task", aria: { selected: false }) + expect(wp_table.treegrid).to have_selector(:row, "Parent task", aria: { selected: true }) + expect(wp_table.treegrid).to have_selector(:row, "Child task", aria: { selected: false }) expect_axe_clean_table relations = "Relations in Children for #{parent.formatted_id}" find(:button, accessible_name: relations, aria: { expanded: false }).click expect(page).to have_button(accessible_name: relations, aria: { expanded: true }) - expect(treegrid).to have_selector(:row, "Child task", aria: { level: 2, selected: false }, count: 1) - expect(treegrid).to have_selector(:row, "Parent task", aria: { selected: true }) + expect(wp_table.treegrid).to have_selector(:row, "Child task", aria: { level: 2, selected: false }, count: 1) + expect(wp_table.treegrid).to have_selector(:row, "Parent task", aria: { selected: true }) + wp_table.expect_no_row_positions expect_axe_clean_table find(:button, accessible_name: relations, aria: { expanded: true }).click expect(page).to have_button(accessible_name: relations, aria: { expanded: false }) - expect(treegrid).to have_no_selector(:element, "tr", aria: { level: 2 }, text: "Child task", visible: :all) + expect(wp_table.treegrid).to have_no_selector(:element, "tr", aria: { level: 2 }, text: "Child task", visible: :all) + wp_table.expect_row_positions(rowcount: 5, rows: [1, 2, 3, 4, 5]) expect_axe_clean_table expect(page).to have_selector(:element, class: "work-packages-tabletimeline--timeline-side", @@ -117,32 +120,32 @@ wp_table.visit_query(query) wp_table.expect_work_package_listed(alpha, bravo, charlie) - expect(treegrid).to have_selector(:row, "Foo", aria: { level: 1, expanded: true, selected: nil }) - expect(treegrid).to have_selector(:row, "Alpha in foo", aria: { level: 2, selected: false, expanded: nil }) - expect(treegrid).to have_selector(:row, /\bSum\b/, aria: { level: 2, selected: nil, expanded: nil }, count: 2) - expect(treegrid).to have_selector(:gridcell, accessible_description: "Sum", minimum: 2) - expect(treegrid).to have_selector(:row, accessible_name: /Total sum/, aria: { level: 1, selected: nil }) + expect(wp_table.treegrid).to have_selector(:row, "Foo", aria: { level: 1, expanded: true, selected: nil }) + expect(wp_table.treegrid).to have_selector(:row, "Alpha in foo", aria: { level: 2, selected: false, expanded: nil }) + expect(wp_table.treegrid).to have_selector(:row, /\bSum\b/, aria: { level: 2, selected: nil, expanded: nil }, count: 2) + expect(wp_table.treegrid).to have_selector(:gridcell, accessible_description: "Sum", minimum: 2) + expect(wp_table.treegrid).to have_selector(:row, accessible_name: /Total sum/, aria: { level: 1, selected: nil }) expect_axe_clean_table wp_table.focus_row_by_keyboard(alpha) wp_table.press_keys(:enter) - expect(treegrid).to have_selector(:row, "Alpha in foo", aria: { selected: true }) + expect(wp_table.treegrid).to have_selector(:row, "Alpha in foo", aria: { selected: true }) expect_axe_clean_table - foo_header = treegrid.find(:row, "Foo", aria: { expanded: true }) + foo_header = wp_table.treegrid.find(:row, "Foo", aria: { expanded: true }) foo_sums_key = foo_header["data-occurrence-key"].sub(/:header\z/, ":sums") foo_header.find(".expander").click - expect(treegrid).to have_selector(:row, "Foo", aria: { level: 1, expanded: false }) - foo_members = [alpha, bravo].map { |wp| treegrid.find(wp_table.row_selector(wp), visible: :hidden) } - foo_sum = treegrid.find("tr[data-occurrence-key='#{foo_sums_key}']", visible: :hidden) + expect(wp_table.treegrid).to have_selector(:row, "Foo", aria: { level: 1, expanded: false }) + foo_members = [alpha, bravo].map { |wp| wp_table.treegrid.find(wp_table.row_selector(wp), visible: :hidden) } + foo_sum = wp_table.treegrid.find("tr[data-occurrence-key='#{foo_sums_key}']", visible: :hidden) expect(foo_members.first).to match_selector(:element, aria: { hidden: true, selected: true }, visible: :hidden) [*foo_members, foo_sum].each { |row| expect_excluded_from_accessibility_tree(row) } - expect(treegrid).to have_selector(:row, "Charlie in bar", aria: { level: 2 }) + expect(wp_table.treegrid).to have_selector(:row, "Charlie in bar", aria: { level: 2 }) expect_axe_clean_table - treegrid.find(:row, "Foo", aria: { expanded: false }).find(".expander").click - expect(treegrid).to have_selector(:row, "Foo", aria: { level: 1, expanded: true }) - expect(treegrid).to have_selector(:row, "Alpha in foo", aria: { level: 2, selected: true }) + wp_table.treegrid.find(:row, "Foo", aria: { expanded: false }).find(".expander").click + expect(wp_table.treegrid).to have_selector(:row, "Foo", aria: { level: 1, expanded: true }) + expect(wp_table.treegrid).to have_selector(:row, "Alpha in foo", aria: { level: 2, selected: true }) [*foo_members, foo_sum].each { |row| expect_exposed_at_level(row, 2) } expect_axe_clean_table end @@ -153,22 +156,22 @@ wp_table.focus_row_by_keyboard(charlie) wp_table.press_keys(:up) - expect(treegrid).to have_selector(:row, "Bar", aria: { level: 1, expanded: true }, focused: true) + expect(wp_table.treegrid).to have_selector(:row, "Bar", aria: { level: 1, expanded: true }, focused: true) wp_table.press_keys(:left) - expect(treegrid).to have_selector(:row, "Bar", aria: { level: 1, expanded: false }, focused: true) - charlie_row = treegrid.find(wp_table.row_selector(charlie), visible: :hidden) + expect(wp_table.treegrid).to have_selector(:row, "Bar", aria: { level: 1, expanded: false }, focused: true) + charlie_row = wp_table.treegrid.find(wp_table.row_selector(charlie), visible: :hidden) expect_excluded_from_accessibility_tree(charlie_row) expect_axe_clean_table wp_table.press_keys(:right) - expect(treegrid).to have_selector(:row, "Bar", aria: { level: 1, expanded: true }, focused: true) + expect(wp_table.treegrid).to have_selector(:row, "Bar", aria: { level: 1, expanded: true }, focused: true) expect_exposed_at_level(charlie_row, 2) wp_table.press_keys(:enter) - expect(treegrid).to have_selector(:row, "Bar", aria: { level: 1, expanded: false }, focused: true) + expect(wp_table.treegrid).to have_selector(:row, "Bar", aria: { level: 1, expanded: false }, focused: true) wp_table.press_keys(:enter) - expect(treegrid).to have_selector(:row, "Bar", aria: { level: 1, expanded: true }, focused: true) + expect(wp_table.treegrid).to have_selector(:row, "Bar", aria: { level: 1, expanded: true }, focused: true) expect_axe_clean_table end end @@ -190,9 +193,9 @@ wp_table.expect_work_package_listed(alpha, bravo, charlie) expect(page).to have_css(".wp-table--drag-and-drop-handle", visible: :all) - expect(treegrid).to have_selector(:row, /\bSum\b/, aria: { level: 2, selected: nil, expanded: nil }, count: 2) - expect(treegrid).to have_selector(:gridcell, accessible_description: "Sum", minimum: 2) - expect(treegrid).to have_selector(:gridcell, accessible_description: "Total sum") + expect(wp_table.treegrid).to have_selector(:row, /\bSum\b/, aria: { level: 2, selected: nil, expanded: nil }, count: 2) + expect(wp_table.treegrid).to have_selector(:gridcell, accessible_description: "Sum", minimum: 2) + expect(wp_table.treegrid).to have_selector(:gridcell, accessible_description: "Total sum") expect_axe_clean_table end end @@ -210,32 +213,32 @@ wp_table.visit_query(query) wp_table.expect_work_package_listed(parent, child, grandchild, sibling) - expect(treegrid).to have_selector(:row, "Alpha parent", aria: { level: 1, expanded: true, selected: false }) - expect(treegrid).to have_selector(:row, "Bravo child", aria: { level: 2, expanded: true, selected: false }) - expect(treegrid).to have_selector(:row, "Charlie grandchild", - aria: { level: 3, expanded: nil, selected: false }) - expect(treegrid).to have_selector(:row, "Delta sibling", aria: { level: 1, expanded: nil, selected: false }) + expect(wp_table.treegrid).to have_selector(:row, "Alpha parent", aria: { level: 1, expanded: true, selected: false }) + expect(wp_table.treegrid).to have_selector(:row, "Bravo child", aria: { level: 2, expanded: true, selected: false }) + expect(wp_table.treegrid).to have_selector(:row, "Charlie grandchild", + aria: { level: 3, expanded: nil, selected: false }) + expect(wp_table.treegrid).to have_selector(:row, "Delta sibling", aria: { level: 1, expanded: nil, selected: false }) expect_axe_clean_table wp_table.focus_row_by_keyboard(grandchild) wp_table.press_keys(:enter) - expect(treegrid).to have_selector(:row, "Charlie grandchild", aria: { selected: true }) + expect(wp_table.treegrid).to have_selector(:row, "Charlie grandchild", aria: { selected: true }) expect_axe_clean_table - within(treegrid.find(:row, "Alpha parent", aria: { expanded: true })) do + within(wp_table.treegrid.find(:row, "Alpha parent", aria: { expanded: true })) do find(:role, "button", accessible_name: /\AHierarchy level 1, expanded\./).click end - expect(treegrid).to have_selector(:row, "Alpha parent", aria: { level: 1, expanded: false }) - descendants = [child, grandchild].map { |wp| treegrid.find(wp_table.row_selector(wp), visible: :hidden) } + expect(wp_table.treegrid).to have_selector(:row, "Alpha parent", aria: { level: 1, expanded: false }) + descendants = [child, grandchild].map { |wp| wp_table.treegrid.find(wp_table.row_selector(wp), visible: :hidden) } expect(descendants.last).to match_selector(:element, aria: { hidden: true, selected: true }, visible: :hidden) descendants.each { |row| expect_excluded_from_accessibility_tree(row) } expect_axe_clean_table - within(treegrid.find(:row, "Alpha parent", aria: { expanded: false })) do + within(wp_table.treegrid.find(:row, "Alpha parent", aria: { expanded: false })) do find(:role, "button", accessible_name: /\AHierarchy level 1, collapsed\./).click end - expect(treegrid).to have_selector(:row, "Alpha parent", aria: { level: 1, expanded: true }) - expect(treegrid).to have_selector(:row, "Charlie grandchild", aria: { level: 3, selected: true }) + expect(wp_table.treegrid).to have_selector(:row, "Alpha parent", aria: { level: 1, expanded: true }) + expect(wp_table.treegrid).to have_selector(:row, "Charlie grandchild", aria: { level: 3, selected: true }) expect_exposed_at_level(descendants.first, 2) expect_exposed_at_level(descendants.last, 3) expect_axe_clean_table @@ -247,14 +250,14 @@ wp_table.focus_row_by_keyboard(parent) wp_table.press_keys(:left) - expect(treegrid).to have_selector(:row, "Alpha parent", aria: { level: 1, expanded: false }, focused: true) - descendants = [child, grandchild].map { |wp| treegrid.find(wp_table.row_selector(wp), visible: :hidden) } + expect(wp_table.treegrid).to have_selector(:row, "Alpha parent", aria: { level: 1, expanded: false }, focused: true) + descendants = [child, grandchild].map { |wp| wp_table.treegrid.find(wp_table.row_selector(wp), visible: :hidden) } descendants.each { |row| expect_excluded_from_accessibility_tree(row) } - expect(treegrid).to have_selector(:row, "Delta sibling", aria: { level: 1 }) + expect(wp_table.treegrid).to have_selector(:row, "Delta sibling", aria: { level: 1 }) expect_axe_clean_table wp_table.press_keys(:right) - expect(treegrid).to have_selector(:row, "Alpha parent", aria: { level: 1, expanded: true }, focused: true) + expect(wp_table.treegrid).to have_selector(:row, "Alpha parent", aria: { level: 1, expanded: true }, focused: true) expect_exposed_at_level(descendants.first, 2) expect_exposed_at_level(descendants.last, 3) end @@ -280,8 +283,8 @@ field.input_element.set("Renamed task") wp_table.press_keys(:enter) - expect(treegrid).to have_selector(:row, "No work packages to display.", aria: { level: 1, selected: nil }) - expect(treegrid).to have_selector(:row, "Create new work package", aria: { level: 1, selected: nil }) + expect(wp_table.treegrid).to have_selector(:row, "No work packages to display.", aria: { level: 1, selected: nil }) + expect(wp_table.treegrid).to have_selector(:row, "Create new work package", aria: { level: 1, selected: nil }) expect_focus_on_header_row expect_axe_clean_table @@ -298,16 +301,12 @@ end end - def treegrid - find(:table, role: "treegrid", accessible_name: "Work packages") - end - def expect_axe_clean_table expect(page).to be_axe_clean.within("table.work-package-table") end def expect_focus_on_header_row - expect(treegrid).to have_selector(:row, "Subject", aria: { level: 1, selected: nil }, focused: true) + expect(wp_table.treegrid).to have_selector(:row, "Subject", aria: { level: 1, selected: nil }, focused: true) end def accessibility_node(row) diff --git a/spec/support/pages/work_packages/work_packages_table.rb b/spec/support/pages/work_packages/work_packages_table.rb index 6170106ce9b1..24d7dba17584 100644 --- a/spec/support/pages/work_packages/work_packages_table.rb +++ b/spec/support/pages/work_packages/work_packages_table.rb @@ -371,6 +371,27 @@ def expect_focus_outside_table expect(page).to have_css(":focus") end + # @return [Capybara::Node::Element] The work package table exposed as a treegrid. + def treegrid + find(:table, role: "treegrid", accessible_name: "Work packages") + end + + # Expects the table's `aria-rowcount` and the `aria-rowindex` of every + # visible row, in document order from the header row down. + # + # @param rowcount [Integer] The expected `aria-rowcount` of the table. + # @param rows [Array] The expected `aria-rowindex` of each exposed row. + def expect_row_positions(rowcount:, rows:) + wait_for { treegrid.all(:row).pluck("aria-rowindex") }.to eq(rows.map(&:to_s)) + expect(treegrid).to match_selector(:table, aria: { rowcount: }) + end + + # Expects the table and all of its rows, hidden ones included, to carry no row position. + def expect_no_row_positions + expect(treegrid).to match_selector(:table, aria: { rowcount: nil }) + expect(treegrid).to have_no_selector(:element, "tr", "aria-rowindex": /./, visible: :all) + end + # Returns the row element for the specified work package. # # @param work_package [WorkPackage] The work package object. From a5d484835198373df63750f4184afe507cfbf651 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 07:35:42 +0100 Subject: [PATCH 6/8] [DREAM-938] Cover grouped and empty row positions Checks global positions for group headers, group sums, totals and Create in grouped mode, the stable gap left by a collapsed group and the positions restored on expanding it again. Also checks that hierarchy mode exposes no positions and that the empty-results table counts the header, empty message and Create rows. https://community.openproject.org/wp/DREAM-938 --- spec/features/work_packages/table/treegrid_semantics_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/features/work_packages/table/treegrid_semantics_spec.rb b/spec/features/work_packages/table/treegrid_semantics_spec.rb index a6642b99507d..de9749cbefbb 100644 --- a/spec/features/work_packages/table/treegrid_semantics_spec.rb +++ b/spec/features/work_packages/table/treegrid_semantics_spec.rb @@ -119,6 +119,7 @@ it "exposes group levels and removes collapsed members from the accessibility tree" do wp_table.visit_query(query) wp_table.expect_work_package_listed(alpha, bravo, charlie) + wp_table.expect_row_positions(rowcount: 10, rows: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) expect(wp_table.treegrid).to have_selector(:row, "Foo", aria: { level: 1, expanded: true, selected: nil }) expect(wp_table.treegrid).to have_selector(:row, "Alpha in foo", aria: { level: 2, selected: false, expanded: nil }) @@ -141,12 +142,14 @@ expect(foo_members.first).to match_selector(:element, aria: { hidden: true, selected: true }, visible: :hidden) [*foo_members, foo_sum].each { |row| expect_excluded_from_accessibility_tree(row) } expect(wp_table.treegrid).to have_selector(:row, "Charlie in bar", aria: { level: 2 }) + wp_table.expect_row_positions(rowcount: 10, rows: [1, 2, 3, 4, 5, 9, 10]) expect_axe_clean_table wp_table.treegrid.find(:row, "Foo", aria: { expanded: false }).find(".expander").click expect(wp_table.treegrid).to have_selector(:row, "Foo", aria: { level: 1, expanded: true }) expect(wp_table.treegrid).to have_selector(:row, "Alpha in foo", aria: { level: 2, selected: true }) [*foo_members, foo_sum].each { |row| expect_exposed_at_level(row, 2) } + wp_table.expect_row_positions(rowcount: 10, rows: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) expect_axe_clean_table end @@ -212,6 +215,7 @@ it "exposes levels and expansion and removes collapsed descendants from the accessibility tree" do wp_table.visit_query(query) wp_table.expect_work_package_listed(parent, child, grandchild, sibling) + wp_table.expect_no_row_positions expect(wp_table.treegrid).to have_selector(:row, "Alpha parent", aria: { level: 1, expanded: true, selected: false }) expect(wp_table.treegrid).to have_selector(:row, "Bravo child", aria: { level: 2, expanded: true, selected: false }) @@ -285,6 +289,7 @@ expect(wp_table.treegrid).to have_selector(:row, "No work packages to display.", aria: { level: 1, selected: nil }) expect(wp_table.treegrid).to have_selector(:row, "Create new work package", aria: { level: 1, selected: nil }) + wp_table.expect_row_positions(rowcount: 3, rows: [1, 2, 3]) expect_focus_on_header_row expect_axe_clean_table From 58258fd77c256ea98f8dfeaefcd35e89336f0cf3 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 07:38:52 +0100 Subject: [PATCH 7/8] [DREAM-938] Cover row positions across pages Adds pagination helpers for changing the page and the page size, and checks in both project and global scope that a row on page 2 keeps its query-wide position while the table count stays the same. https://community.openproject.org/wp/DREAM-938 --- spec/features/work_packages/pagination_spec.rb | 16 ++++++++++------ spec/support/components/table_pagination.rb | 12 ++++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/spec/features/work_packages/pagination_spec.rb b/spec/features/work_packages/pagination_spec.rb index fa83c973fe86..092ce417ec36 100644 --- a/spec/features/work_packages/pagination_spec.rb +++ b/spec/features/work_packages/pagination_spec.rb @@ -39,6 +39,8 @@ shared_examples_for "paginated work package list" do let!(:work_package_1) { create(:work_package, project:) } let!(:work_package_2) { create(:work_package, project:) } + let(:hierarchies) { Components::WorkPackages::Hierarchies.new } + let(:pagination) { Components::TablePagination.new } before do login_as(admin) @@ -58,30 +60,31 @@ expect(page).to have_content(work_package_1.subject) expect(page).to have_no_content(work_package_2.subject) end + hierarchies.disable_via_header + wp_table.expect_row_positions(rowcount: 4, rows: [1, 2, 4]) - within(".op-pagination--pages") do - find(".op-pagination--item button", text: "2").click - end + pagination.go_to_page(2) within(".work-packages-partitioned-query-space--container") do expect(page).to have_content(work_package_2.subject) expect(page).to have_no_content(work_package_1.subject) end + wp_table.expect_row_positions(rowcount: 4, rows: [1, 3, 4]) - within(".op-pagination--options") do - find(".op-pagination--item button", text: "50").click - end + pagination.select_per_page(50) within(".work-packages-partitioned-query-space--container") do expect(page).to have_content(work_package_1.subject) expect(page).to have_content(work_package_2.subject) end + wp_table.expect_row_positions(rowcount: 4, rows: [1, 2, 3, 4]) end end context "with project scope" do it_behaves_like "paginated work package list" do let(:path) { project_work_packages_path(project) } + let(:wp_table) { Pages::WorkPackagesTable.new(project) } let(:expected_path) { "/projects/project1/work_packages" } end end @@ -89,6 +92,7 @@ context "globally" do it_behaves_like "paginated work package list" do let(:path) { work_packages_path } + let(:wp_table) { Pages::WorkPackagesTable.new } let(:expected_path) { "/work_packages" } end end diff --git a/spec/support/components/table_pagination.rb b/spec/support/components/table_pagination.rb index 5d822833997c..835c8b6573cc 100644 --- a/spec/support/components/table_pagination.rb +++ b/spec/support/components/table_pagination.rb @@ -40,6 +40,18 @@ def expect_range(from, to, total) end end + def go_to_page(number) + within_pagination do + within(".op-pagination--pages") { click_on number.to_s } + end + end + + def select_per_page(number) + within_pagination do + within(".op-pagination--options") { click_on number.to_s } + end + end + def expect_no_per_page_options within_pagination do expect(page) From 866679ad97723fe8d17718ddce35b9f96624baef Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 07:47:08 +0100 Subject: [PATCH 8/8] [DREAM-938] Cover row positions through drops Checks that a drop on a flat table brings global positions back once the reloaded query has rendered, and that a drop on an already manually sorted query persists the order which the next visit exposes with positions again. Transient states during a drop are covered by the Vitest lifetime and row-positions specs. https://community.openproject.org/wp/DREAM-938 --- .../sorting/manual_sorting_spec.rb | 18 +++++++++ .../table/treegrid_semantics_spec.rb | 40 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/spec/features/work_packages/sorting/manual_sorting_spec.rb b/spec/features/work_packages/sorting/manual_sorting_spec.rb index e1f616f6215e..99c444815f3c 100644 --- a/spec/features/work_packages/sorting/manual_sorting_spec.rb +++ b/spec/features/work_packages/sorting/manual_sorting_spec.rb @@ -376,6 +376,24 @@ def expect_query_order(query, expected) wp_table.expect_work_package_order work_package3, work_package1, work_package2, work_package4 end + it "exposes row positions again once a drop has reloaded the query" do + wp_table.expect_row_positions(rowcount: 6, rows: [1, 2, 3, 4, 5, 6]) + + wp_table.drag_and_drop_work_package from: 1, to: 3 + wp_table.expect_work_package_order work_package1, work_package3, work_package2, work_package4 + wp_table.expect_and_dismiss_toaster message: "Successful creation." + wp_table.expect_row_positions(rowcount: 6, rows: [1, 2, 3, 4, 5, 6]) + + query = Query.last + wp_table.drag_and_drop_work_package from: 0, to: 2 + wp_table.expect_work_package_order work_package3, work_package1, work_package2, work_package4 + expect_query_order(query, [work_package3.id, work_package1.id, work_package2.id]) + + wp_table.visit_query query + wp_table.expect_work_package_order work_package3, work_package1, work_package2, work_package4 + wp_table.expect_row_positions(rowcount: 6, rows: [1, 2, 3, 4, 5, 6]) + end + it "keeps the source row visible as a placeholder and collapses multi-selection" do # Multi-select rows 1 and 2 via ctrl-click on the ID cells (established pattern). # loading_indicator_saveguard between the two clicks mirrors diff --git a/spec/features/work_packages/table/treegrid_semantics_spec.rb b/spec/features/work_packages/table/treegrid_semantics_spec.rb index de9749cbefbb..8c75aeba5551 100644 --- a/spec/features/work_packages/table/treegrid_semantics_spec.rb +++ b/spec/features/work_packages/table/treegrid_semantics_spec.rb @@ -103,6 +103,20 @@ expect(page).to have_selector(:element, class: "work-packages-tabletimeline--timeline-side", aria: { hidden: true }, visible: :all) end + + it "keeps the positions while the inline create row is open and after it is cancelled" do + wp_table.visit_query(query) + wp_table.expect_work_package_order(child, parent) + wp_table.expect_row_positions(rowcount: 5, rows: [1, 2, 3, 4, 5]) + + wp_table.click_inline_create + expect(wp_table.creation_row).to match_selector(:row, aria: { rowindex: 5 }) + wp_table.expect_row_positions(rowcount: 5, rows: [1, 2, 3, 4, 5]) + + wp_table.press_keys(:escape) + expect(page).to have_button("Create new work package") + wp_table.expect_row_positions(rowcount: 5, rows: [1, 2, 3, 4, 5]) + end end context "in grouped mode" do @@ -203,6 +217,32 @@ end end + context "in grouped mode across pages", with_settings: { per_page_options: "3" } do + let(:foo) { create(:category, project:, name: "Foo") } + let(:bar) { create(:category, project:, name: "Bar") } + let!(:alpha) { create(:work_package, project:, category: foo, subject: "Alpha in foo", estimated_hours: 1) } + let!(:bravo) { create(:work_package, project:, category: foo, subject: "Bravo in foo", estimated_hours: 2) } + let!(:charlie) { create(:work_package, project:, category: bar, subject: "Charlie in bar", estimated_hours: 4) } + let!(:delta) { create(:work_package, project:, category: foo, subject: "Delta in foo", estimated_hours: 8) } + let!(:echo) { create(:work_package, project:, category: foo, subject: "Echo in foo", estimated_hours: 16) } + 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 + + it "continues the numbering on the next page and keeps the gap for the members of the other page" do + wp_table.visit_query(query) + wp_table.expect_work_package_order(charlie, alpha, bravo) + wp_table.expect_row_positions(rowcount: 12, rows: [1, 2, 3, 4, 5, 6, 7, 10, 11, 12]) + + within(".op-pagination--pages") { click_on "2", exact: true } + wp_table.expect_work_package_order(delta, echo) + wp_table.expect_row_positions(rowcount: 12, rows: [1, 5, 8, 9, 10, 11, 12]) + expect_axe_clean_table + end + end + context "in hierarchy mode" do let!(:parent) { create(:work_package, project:, subject: "Alpha parent") } let!(:child) { create(:work_package, project:, parent:, subject: "Bravo child") }