From e175783aa68051f62d2dab81bc8b0b504e11027c Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 24 Oct 2026 05:40:35 +0100 Subject: [PATCH] [OP-20570] Rebuild a pending render on collapse Rebuilds a queued render when a group or hierarchy toggle reaches setHidden before the render's frame commit, so the committed rows carry the current collapse state instead of the state the draft was built with. The stale frame callback is superseded the same way a second redraw supersedes it, and a timeline pass stays promoted. Adds holdFrames and atNextPublication to the table harness, and deferred-frame regressions for group collapse and expand, hierarchy collapse, and a timeline-promoted render. https://community.openproject.org/wp/OP-20570 --- .../modes/grouped/grouped-collapse.spec.ts | 37 ++++++++++++++ .../hierarchy/hierarchy-collapse.spec.ts | 50 +++++++++++++++++++ .../wp-fast-table/testing/table-harness.ts | 20 +++++++- .../components/wp-fast-table/wp-fast-table.ts | 7 +++ 4 files changed, 113 insertions(+), 1 deletion(-) diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-collapse.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-collapse.spec.ts index 0a01674f02ff..e9dcb8b40959 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-collapse.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-collapse.spec.ts @@ -139,4 +139,41 @@ describe('Grouped table collapse', () => { expect(harness.table.ledger.byElement(header)?.key).toBe(groupOccurrenceKey(0, 'header')); expect(harness.table.ledger.keyAt(0)).toBe(firstKey); }); + + describe('when a toggle lands on a queued render', () => { + const memberState = (before:HTMLElement) => () => ({ + committed: harness.row('1') !== before, + ledgerHidden: harness.table.ledger.byWorkPackageId('1')[0].hidden, + display: getComputedStyle(harness.row('1')).display, + expander: within(harness.groupHeader(0)).queryByText('js.label_expand') ? 'expand' : 'collapse', + }); + + const toggleDuringQueuedRender = async () => { + const state = memberState(harness.row('1')); + const release = harness.holdFrames(); + harness.table.redrawTable(); + toggleGroup(0); + const published = harness.atNextPublication(state); + release(); + return { published: await published, final: state() }; + }; + + it('commits a collapsed group with its rows hidden', async () => { + const collapsed = { + committed: true, ledgerHidden: true, display: 'none', expander: 'expand', + }; + + expect(await toggleDuringQueuedRender()).toEqual({ published: collapsed, final: collapsed }); + }); + + it('commits an expanded group with its rows shown', async () => { + toggleGroup(0); + await waitFor(() => expect(harness.row('1')).not.toBeVisible()); + const expanded = { + committed: true, ledgerHidden: false, display: 'table-row', expander: 'collapse', + }; + + expect(await toggleDuringQueuedRender()).toEqual({ published: expanded, final: expanded }); + }); + }); }); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-collapse.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-collapse.spec.ts index 2ca975e66336..86eda035ab28 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-collapse.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-collapse.spec.ts @@ -30,6 +30,7 @@ import { buildTable, TableHarness } from '../../../testing/table-harness'; import { WorkPackageFixture } from '../../../testing/work-package-fixture'; import { States } from 'core-app/core/states/states.service'; import { hierarchyGroupClass } from '../../../helpers/wp-table-hierarchy-helpers'; +import { indicatorCollapsedClass } from './single-hierarchy-row-builder'; import { WorkPackageViewHierarchiesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-hierarchy.service'; const parent = { id: '1' }; @@ -159,6 +160,55 @@ describe('Hierarchy table collapse', () => { }); }); + describe('when a collapse lands on a queued render', () => { + const collapsed = { + committed: true, ledgerHidden: true, display: 'none', indicatorCollapsed: true, + }; + + const childState = () => { + const before = harness.row('2'); + return () => ({ + committed: harness.row('2') !== before, + ledgerHidden: harness.table.ledger.byWorkPackageId('2')[0].hidden, + display: getComputedStyle(harness.row('2')).display, + indicatorCollapsed: within(harness.row('1')).getByRole('button').classList.contains(indicatorCollapsedClass), + }); + }; + + const collapseDuringQueuedRender = async (redraw:() => void, state:() => T) => { + const release = harness.holdFrames(); + redraw(); + harness.injector.get(WorkPackageViewHierarchiesService).collapse('1'); + const published = harness.atNextPublication(state); + release(); + return { published: await published, final: state() }; + }; + + it('commits a collapsed parent with its children hidden', async () => { + await renderHierarchy([parent, child]); + + expect(await collapseDuringQueuedRender(() => harness.table.redrawTable(), childState())) + .toEqual({ published: collapsed, final: collapsed }); + }); + + it('commits hidden timeline cells when the queued render includes the timeline', async () => { + harness = buildTable({ workPackages: [parent, child], showHierarchies: true, timelineVisible: true }); + await harness.render(); + const timelineCell = () => harness.timelineRowOf(harness.row('2')); + const timelineBefore = timelineCell(); + const tableState = childState(); + const state = () => ({ + ...tableState(), + timelineCommitted: timelineCell() !== timelineBefore, + timelineDisplay: getComputedStyle(timelineCell()).display, + }); + const expected = { ...collapsed, timelineCommitted: true, timelineDisplay: 'none' }; + + expect(await collapseDuringQueuedRender(() => harness.table.redrawTableAndTimeline(), state)) + .toEqual({ published: expected, final: expected }); + }); + }); + describe('with a contextual ancestor outside the results', () => { beforeEach(() => renderHierarchy([ { id: '2', ancestors: [parent] }, diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/testing/table-harness.ts index 8416b8a7c18c..e8d67fa92020 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 @@ -31,7 +31,7 @@ import { } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { firstValueFrom, of, Subject } from 'rxjs'; -import { skip, take } from 'rxjs/operators'; +import { map, skip, take } from 'rxjs/operators'; import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource'; import { QueryOrder } from 'core-app/core/apiv3/endpoints/queries/apiv3-query-order'; import { WorkPackageNotificationService } from 'core-app/features/work-packages/services/notifications/work-package-notification.service'; @@ -161,6 +161,10 @@ export interface TableHarness { render(workPackages?:WorkPackageFixture[]):Promise; /** Resolves with the next completed table render. */ nextRender():Promise; + /** Resolves with `read()` evaluated inside the next `tableRendered` publication. */ + atNextPublication(read:() => T):Promise; + /** Queues the table's frame callbacks until the returned release runs them, so a test can act between a render's build and its commit. */ + holdFrames():() => void; /** Every rendered work-package row, relation rows included. */ rows():HTMLTableRowElement[]; /** The primary row of a work package, never one of its relation rows. */ @@ -284,6 +288,20 @@ export function buildTable(options:TableHarnessOptions):TableHarness { nextRender, + atNextPublication(read) { + return firstValueFrom(querySpace.tableRendered.values$().pipe(skip(1), take(1), map(read))); + }, + + holdFrames() { + const held:(() => void)[] = []; + const frame = table.uiWork.frame.bind(table.uiWork); + table.uiWork.frame = (callback) => { held.push(callback); }; + return () => { + table.uiWork.frame = frame; + held.splice(0).forEach((callback) => callback()); + }; + }, + rows() { return Array.from(dom.tbody.querySelectorAll('tr.wp-table--row')); }, diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table.ts index fe059cebc566..9b2a5752a477 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 @@ -190,11 +190,18 @@ export class WorkPackageTable { public setHidden(updates:ReadonlyMap):void { if (this.destroyed) return; + this.rebuildPendingRender(); if (updates.size === 0) return; this.ledger.setHidden(updates); this.publishRendered(); } + private rebuildPendingRender():void { + if (this.pendingRender) { + this.performRenderPass({ timeline: this.pendingRender.timeline }); + } + } + /** * Redraw single rows for a given work package being updated. */