diff --git a/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.spec.ts b/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.spec.ts index ed69ec6abe1e..71f87636fbdd 100644 --- a/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.spec.ts @@ -521,6 +521,22 @@ describe('TableEditForm focus return', () => { expect(within(row).getByRole('rowheader')).toHaveFocus(); }); + it('names the row header from the reset cell before focus returns', async () => { + await mount(); + const handler = await openEditor(); + const row = harness.row('1'); + harness.table.editing.forms['1'].resource = buildWorkPackage({ id: '1', subject: 'Renamed' }); + let nameAtFocus:string|null = null; + row.addEventListener('focusin', () => { + nameAtFocus = within(row).getByRole('rowheader').getAttribute('aria-label'); + }); + + handler.handleUserCancel(); + + expect(nameAtFocus).toBe('Renamed'); + expect(within(row).getByRole('rowheader')).toHaveAccessibleName('Renamed'); + }); + it('returns to the origin after a user-ended owned overlay closes', async () => { await mount(); const handler = await openEditor(); diff --git a/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.ts b/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.ts index 8fa66fc3a2a6..b85c0fbc9321 100644 --- a/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.ts +++ b/frontend/src/app/features/work-packages/components/wp-edit-form/table-edit-form.ts @@ -197,6 +197,7 @@ export class TableEditForm extends EditForm { td.style.maxWidth = ''; this.cellBuilder.refresh(cell, this.resource, fieldName); td.classList.remove(editModeClassName); + this.table.painter.paint(this.table.painter.keysOf(this.resource.id!)); if (focus || userEnded) { this.returnFocusFrom(cell, fieldName, focus, closing); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/cell-builder.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/cell-builder.ts index b27baa9ada32..3d620be5ac33 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/cell-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/cell-builder.ts @@ -110,10 +110,6 @@ export class CellBuilder { if (attribute === 'subject' || attribute === 'id') { const name = displayElement.textContent ?? ''; cell.dataset.rowHeaderName = name; - if (cell.matches('th[scope="row"]')) { - cell.removeAttribute('aria-labelledby'); - cell.setAttribute('aria-label', name); - } } if (hasBaseline) { diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/drag-and-drop/drag-drop-handle-render-pass.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/drag-and-drop/drag-drop-handle-render-pass.ts index 032e5871fbbe..e5f63d83ee0d 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/drag-and-drop/drag-drop-handle-render-pass.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/drag-and-drop/drag-drop-handle-render-pass.ts @@ -65,7 +65,8 @@ public readonly injector:Injector, const { occurrences } = this.tablePass.draft; void this.wpTableOrder.positionsFor(query).then((positions:QueryOrder) => { if (this.table.destroyed || this.table.querySpace.query.value !== query) return; - occurrences.forEach(({ workPackage, renderType, element }) => { + const replaced = new Set(); + occurrences.forEach(({ key, workPackage, renderType, element }) => { // We only care for rows that are natural work packages and are not relation sub-rows if (!workPackage || renderType !== 'primary') { return; @@ -75,10 +76,11 @@ public readonly injector:Injector, const handle = this.dragDropHandleBuilder.build(latestWorkPackage, positions[workPackage.id!]); if (handle && element) { - handle.setAttribute('role', 'gridcell'); element.replaceChild(handle, element.firstElementChild!); + replaced.add(key); } }); + if (replaced.size > 0) this.table.painter.paint(replaced); }).catch((error:unknown) => notification.handleRawError(error)); } } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-render-pass.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-render-pass.ts index 49b8edd87d8d..91f8f6f6d58e 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-render-pass.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-render-pass.ts @@ -78,7 +78,7 @@ export class GroupedRenderPass extends PlainRenderPass { if (nextGroup && groupsChanged) { const groupClass = groupClassNameFor(nextGroup); const rowElement = this.headerBuilder.buildGroupRow(nextGroup, this.colspan); - this.appendNonWorkPackageRow(groupOccurrenceKey(nextGroup.index, 'header'), rowElement, groupClass, [groupRowClass], false, nextGroup.identifier); + this.appendNonWorkPackageRow(groupOccurrenceKey(nextGroup.index, 'header'), 'groupHeader', rowElement, groupClass, [groupRowClass], false, nextGroup.identifier); currentGroup = nextGroup; } @@ -184,7 +184,7 @@ export class GroupedRenderPass extends PlainRenderPass { const { additionalClasses, hidden } = this.groupRowState(group); rowElement.classList.add(...additionalClasses); - this.appendNonWorkPackageRow(groupOccurrenceKey(group.index, 'sums'), rowElement, groupClass, additionalClasses, hidden, group.identifier, 2); + this.appendNonWorkPackageRow(groupOccurrenceKey(group.index, 'sums'), 'sums', rowElement, groupClass, additionalClasses, hidden, group.identifier, 2); } private groupRowState(group:GroupObject):{ additionalClasses:string[], hidden:boolean } { diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-render-pass.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-render-pass.ts index c80544472981..8ad266f86c0d 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-render-pass.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/hierarchy-render-pass.ts @@ -290,6 +290,7 @@ export class HierarchyRenderPass extends PrimaryRenderPass { return { key: this.occurrenceKey(workPackage, isAncestor), + kind: 'workPackage', classIdentifier: isAncestor ? ancestorClassIdentifier(workPackage.id!) : this.rowBuilder.classIdentifier(workPackage), additionalClasses: isAncestor ? [additionalHierarchyRowClassName].concat(ancestorClasses) : ancestorClasses, workPackage, diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/single-hierarchy-row-builder.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/single-hierarchy-row-builder.ts index 194f1c841ca1..32b913b29237 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/single-hierarchy-row-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/hierarchy/single-hierarchy-row-builder.ts @@ -157,6 +157,7 @@ export class SingleHierarchyRowBuilder extends SingleRowBuilder { subjectCell.classList.add('-with-hierarchy'); subjectCell.prepend(hierarchyElement); + if (this.parentsWithVisibleChildren[workPackage.id!]) row.dataset.expandable = ''; // Assure that the content is still visible when the hierarchy indentation is very large subjectCell.style.minWidth = `${125 + (hierarchyIndentation * hierarchyLevel)}px`; diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/primary-render-pass.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/primary-render-pass.ts index 2c85dd202c5b..c07ec5205559 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/primary-render-pass.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/primary-render-pass.ts @@ -199,6 +199,7 @@ public readonly injector:Injector, if (this.draft.occurrences.length === 0 && this.workPackageTable.renderPlaceholderRow) { this.registerAppended(this.rowBuilder.placeholderRow, { key: placeholderOccurrenceKey(), + kind: 'placeholder', classIdentifier: 'wp--placeholder-row', additionalClasses: [], workPackage: null, @@ -226,6 +227,7 @@ workPackage:WorkPackageResource, ) { this.registerAppended(row, { key: wpOccurrenceKey(workPackage.id!), + kind: 'workPackage', classIdentifier: this.rowBuilder.classIdentifier(workPackage), additionalClasses, workPackage, @@ -244,6 +246,7 @@ workPackage:WorkPackageResource, */ protected appendNonWorkPackageRow( key:OccurrenceKey, + kind:'groupHeader'|'sums', row:HTMLTableRowElement, classIdentifier:string, additionalClasses:string[] = [], @@ -254,6 +257,7 @@ workPackage:WorkPackageResource, row.classList.add(classIdentifier); this.registerAppended(row, { key, + kind, classIdentifier, additionalClasses, workPackage: null, diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relations/relations-render-pass.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relations/relations-render-pass.ts index 0eda6c568b84..e4c6d6d4efd6 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relations/relations-render-pass.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/relations/relations-render-pass.ts @@ -138,6 +138,7 @@ export class RelationsRenderPass { `.${this.relationRowBuilder.classIdentifier(from)},.${relationGroupClass(from.id!)}`, { key: relationOccurrenceKey(type, from.id!, to.id!), + kind: 'workPackage', classIdentifier: this.relationRowBuilder.relationClassIdentifier(from, to), additionalClasses: row.additionalClasses.concat(['wp-table--relations-additional-row']), workPackage: to, diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/rows/single-row-builder.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/rows/single-row-builder.ts index 0d423924ad68..01491fca96d3 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/builders/rows/single-row-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/builders/rows/single-row-builder.ts @@ -32,7 +32,6 @@ import { locateTableRowByIdentifier } from 'core-app/features/work-packages/comp import { WorkPackageResource } from 'core-app/features/hal/resources/work-package-resource'; import { WorkPackageViewColumnsService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-columns.service'; import { TableActionRenderer } from 'core-app/features/work-packages/components/wp-fast-table/builders/table-action-renderer'; -import { WorkPackageViewSelectionService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service'; import { internalBaselineColumn, internalContextMenuColumn, @@ -41,8 +40,6 @@ import { } from 'core-app/features/work-packages/components/wp-fast-table/builders/internal-sort-columns'; import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; import { debugLog } from 'core-app/shared/helpers/debug_output'; -import { paintRowSelection } from './row-selection-painter'; -import { WorkPackageViewFocusService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-focus.service'; import { RelationCellbuilder } from '../relation-cell-builder'; import { CellBuilder, @@ -65,10 +62,6 @@ export const commonRowClassName = 'wp--row'; export class SingleRowBuilder { // Injections - @LazyInject() wpTableSelection:WorkPackageViewSelectionService; - - @LazyInject() wpTableFocus:WorkPackageViewFocusService; - @LazyInject() wpTableColumns:WorkPackageViewColumnsService; @LazyInject() wpTableBaseline:WorkPackageViewBaselineService; @@ -296,11 +289,6 @@ export class SingleRowBuilder { } }); - paintRowSelection(row, { - selected: this.wpTableSelection.isSelected(workPackage.id!), - pressed: this.wpTableFocus.isFocused(workPackage.id!), - }); - return [row, false]; } } 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 54b79ca38f87..2aa5586f7761 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 @@ -208,7 +208,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(); + table.painter.paint(new Set()); }; if (!stillEligible()) { complete(false); @@ -238,7 +238,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(); + table.painter.paint(new Set()); 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/focus-position.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position.ts index b08dac59e4ec..c7e8f55897b4 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/keyboard/focus-position.ts @@ -25,7 +25,11 @@ // See COPYRIGHT and LICENSE files for more details. //++ -export type RowKind = 'header'|'workPackage'|'groupHeader'|'sums'|'totals'; +import type { RowRecordKind } from '../../../semantics/row-classifier'; + +export const ROW_KINDS = ['header', 'workPackage', 'groupHeader', 'sums', 'totals'] as const satisfies readonly RowRecordKind[]; + +export type RowKind = (typeof ROW_KINDS)[number]; export interface NavigableRow { key:string; 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 a143c565e55a..0639a34ced1f 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 @@ -25,12 +25,10 @@ // See COPYRIGHT and LICENSE files for more details. //++ -import { WorkPackageViewHierarchiesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-hierarchy.service'; -import type { RenderedOccurrence } from '../../../rendered-occurrence-ledger'; import type { WorkPackageTable } from '../../../wp-fast-table'; -import { NavigableRow, RowKind } from './focus-position'; +import type { RowRecord, RowRecordKind } from '../../../semantics/row-classifier'; +import { NavigableRow, ROW_KINDS, RowKind } from './focus-position'; import { navigableCells } from './roving-tabindex'; -import { expansionStateOf, rowKindOf } from '../../../helpers/row-expansion'; import { headerRowOf, totalsRowOf } from '../../../helpers/structural-rows'; export const headerRowKey = 'header'; @@ -38,49 +36,27 @@ export const totalsRowKey = 'totals'; export { headerRowOf, totalsRowOf }; -function occurrenceRow( - table:WorkPackageTable, - hierarchies:WorkPackageViewHierarchiesService, - occurrence:RenderedOccurrence, -):NavigableRow|null { - const kind = rowKindOf(occurrence); - if (kind === null || !occurrence.element) { - return null; - } +const navigableKinds:ReadonlySet = new Set(ROW_KINDS); +function isNavigableKind(kind:RowRecordKind):kind is RowKind { + return navigableKinds.has(kind); +} + +function navigableRow({ element, occurrence, state }:RowRecord, key:string, kind:RowKind):NavigableRow { const row:NavigableRow = { - key: occurrence.key, + key, kind, - hidden: occurrence.hidden, - cellCount: navigableCells(occurrence.element).length, - expanded: expansionStateOf(table, hierarchies, occurrence, kind), + hidden: state.hidden, + cellCount: navigableCells(element).length, + expanded: state.expanded, }; - - return occurrence.groupIdentifier === undefined ? row : { ...row, groupIdentifier: occurrence.groupIdentifier }; -} - -function edgeRow(key:string, kind:RowKind, element:HTMLTableRowElement|null):NavigableRow[] { - if (!element) { - return []; - } - - const cellCount = navigableCells(element).length; - return [{ - key, kind, hidden: kind === 'totals' && cellCount === 0, cellCount, expanded: null, - }]; + return occurrence?.groupIdentifier === undefined ? row : { ...row, groupIdentifier: occurrence.groupIdentifier }; } /** The rows keyboard navigation walks, in visual order: header, rendered occurrences, totals. */ export function buildNavigableRows(table:WorkPackageTable):NavigableRow[] { - const { ledger } = table; - const hierarchies = table.injector.get(WorkPackageViewHierarchiesService); - const body = Array.from({ length: ledger.size }, (_, index) => ledger.byKey(ledger.keyAt(index)!)!) - .map((occurrence) => occurrenceRow(table, hierarchies, occurrence)) - .filter((row):row is NavigableRow => row !== null); - - return [ - ...edgeRow(headerRowKey, 'header', headerRowOf(table)), - ...body, - ...edgeRow(totalsRowKey, 'totals', totalsRowOf(table)), - ]; + return table.rows.rows().flatMap((record) => { + const { key, state: { kind } } = record; + return key !== null && isNavigableKind(kind) ? [navigableRow(record, key, kind)] : []; + }); } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.spec.ts index c771134d0531..b97f8b348245 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.spec.ts @@ -89,7 +89,7 @@ describe('SelectionTransformer', () => { destroyRef: lifetime.get(DestroyRef), tableAndTimelineContainer: root, renderedRows, - semantics: { paintSelection: vi.fn() }, + painter: { paint: vi.fn() }, } as unknown as WorkPackageTable; new SelectionTransformer(injector, table); @@ -128,7 +128,7 @@ describe('SelectionTransformer', () => { const lifetime = Injector.create({ providers: [], parent: injector }); const root = document.createElement('div'); document.body.append(root); - new SelectionTransformer(injector, { injector, destroyRef: lifetime.get(DestroyRef), tableAndTimelineContainer: root, renderedRows: [], semantics: { paintSelection: vi.fn() } } as unknown as WorkPackageTable); + new SelectionTransformer(injector, { injector, destroyRef: lifetime.get(DestroyRef), tableAndTimelineContainer: root, renderedRows: [], painter: { paint: vi.fn() } } as unknown as WorkPackageTable); return { injector, lifetime, root, reset }; }; const first = build(); @@ -163,7 +163,7 @@ describe('SelectionTransformer', () => { const root = document.createElement('div'); root.innerHTML = '
Row
'; document.body.append(root); - new SelectionTransformer(injector, { injector, destroyRef: lifetime.get(DestroyRef), tableAndTimelineContainer: root, renderedRows: [], semantics: { paintSelection: vi.fn() } } as unknown as WorkPackageTable); + new SelectionTransformer(injector, { injector, destroyRef: lifetime.get(DestroyRef), tableAndTimelineContainer: root, renderedRows: [], painter: { paint: vi.fn() } } as unknown as WorkPackageTable); selection.initializeSelection(['2']); const allowed = fireEvent.keyDown(root.firstElementChild!, { @@ -191,7 +191,7 @@ describe('SelectionTransformer', () => { const root = document.createElement('div'); document.body.append(root); new SelectionTransformer(injector, { - injector, destroyRef: lifetime.get(DestroyRef), tableAndTimelineContainer: root, renderedRows: [], ledger, semantics: { paintSelection: vi.fn() }, + injector, destroyRef: lifetime.get(DestroyRef), tableAndTimelineContainer: root, renderedRows: [], ledger, painter: { paint: vi.fn() }, } as unknown as WorkPackageTable); const stale = [{ workPackageId: '2', classIdentifier: 'wp-card-2', hidden: false }]; diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts index 734db7ea22fc..e700a616e232 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/state/selection-transformer.ts @@ -32,8 +32,6 @@ import { onDestroySafely } from 'core-app/shared/helpers/angular/owned-ui-cleanu import { IsolatedQuerySpace } from 'core-app/features/work-packages/directives/query-space/isolated-query-space'; import { WorkPackageViewSelectionService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service'; import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; -import { tableRowClassName } from '../../builders/rows/single-row-builder'; -import { paintRowSelection } from '../../builders/rows/row-selection-painter'; import { WorkPackageTable } from '../../wp-fast-table'; import { selectionGesture, TableSelectionFeedback } from './selection-feedback'; import { registerWorkPackageDeselectAll } from 'core-app/features/work-packages/routing/wp-view-base/event-handling/wp-selection-keyboard'; @@ -83,15 +81,6 @@ export class SelectionTransformer { } private paintRows():void { - this.table.tableAndTimelineContainer - .querySelectorAll(`.${tableRowClassName}[data-work-package-id]`) - .forEach((row) => { - const workPackageId = row.dataset.workPackageId!; - paintRowSelection(row, { - selected: this.wpTableSelection.isSelected(workPackageId), - pressed: this.wpTableFocus.isFocused(workPackageId), - }); - }); - this.table.semantics.paintSelection(); + this.table.painter.paint(); } } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/helpers/row-expansion.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/helpers/row-expansion.ts index 7ba42a43287b..3ab9f9b567b1 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/helpers/row-expansion.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/helpers/row-expansion.ts @@ -34,7 +34,7 @@ import { occurrenceRowKind } from './structural-rows'; /** The row's navigation category, or null when the occurrence is excluded from keyboard navigation. */ export function rowKindOf(occurrence:RenderedOccurrence):RowKind|null { const kind = occurrenceRowKind(occurrence); - return kind === 'placeholder' || kind === 'foreign' ? null : kind; + return kind === 'placeholder' ? null : kind; } export function expansionStateOf( @@ -47,7 +47,7 @@ export function expansionStateOf( return !table.workPackageViewCollapsedGroupsService.config.state[occurrence.groupIdentifier ?? '']; } - if (kind === 'workPackage' && occurrence.element?.querySelector('.wp-table--hierarchy-indicator')) { + if (kind === 'workPackage' && occurrence.element?.dataset.expandable !== undefined) { return !hierarchies.collapsed(occurrence.workPackageId!); } 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 index 002d851eca5c..674ea08e2e0c 100644 --- 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 @@ -97,7 +97,7 @@ describe('structural rows', () => { expect(kind(harness.row('1'))).toBe('workPackage'); expect(kind(harness.table.ledger.byKey(groupOccurrenceKey(0, 'sums'))!.element!)).toBe('sums'); expect(kind(harness.totalsRow())).toBe('totals'); - expect(kind(createRow())).toBe('create'); + expect(kind(createRow())).toBe('inlineCreate'); }); it('names placeholder, empty, unexposed and unknown rows', async () => { @@ -110,15 +110,15 @@ describe('structural rows', () => { harness.tbody.before(empty); const unknown = harness.tbody.closest('table')!.createTBody().insertRow(); expect(kind(harness.table.ledger.byKey(placeholderOccurrenceKey())!.element!)).toBe('placeholder'); - expect(kind(emptyRow)).toBe('empty'); + expect(kind(emptyRow)).toBe('emptyResults'); expect(kind(unknown)).toBe('foreign'); unknown.hidden = true; expect(kind(unknown)).toBe('unexposed'); }); - it('treats an occurrence it cannot name as foreign', () => { - const occurrence = { key: 'banner', workPackageId: null } as RenderedOccurrence; - expect(occurrenceRowKind(occurrence)).toBe('foreign'); + it('reads the kind the render pass registered rather than parsing the key', () => { + const occurrence = { key: 'group:0:header', kind: 'workPackage', workPackageId: null } as RenderedOccurrence; + expect(occurrenceRowKind(occurrence)).toBe('workPackage'); }); }); }); 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 index 072f9247e24b..3560e1841106 100644 --- 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 @@ -26,15 +26,15 @@ //++ import type { WorkPackageTable } from '../wp-fast-table'; -import { placeholderOccurrenceKey, type RenderedOccurrence } from '../rendered-occurrence-ledger'; +import type { OccurrenceKind, RenderedOccurrence } from '../rendered-occurrence-ledger'; import { inlineCreateSelector } from '../handlers/state/keyboard/roving-tabindex'; const totalsRowSelector = 'tr.wp-table--sums-row'; -export type OccurrenceRowKind = 'workPackage'|'groupHeader'|'sums'|'placeholder'|'foreign'; +export type OccurrenceRowKind = OccurrenceKind; /** Every body or trailing row of the table; `foreign` is a row no producer declared. */ -export type TableRowKind = OccurrenceRowKind|'totals'|'create'|'empty'|'unexposed'; +export type TableRowKind = OccurrenceRowKind|'foreign'|'totals'|'inlineCreate'|'emptyResults'|'unexposed'; /** The `thead` row of the table, when rendered. */ export function headerRowOf(table:WorkPackageTable):HTMLTableRowElement|null { @@ -60,7 +60,10 @@ 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. */ +/** + * Whether the row is exposed, that is reachable by assistive technology: not `hidden`, not + * `aria-hidden`, and not in an `aria-hidden` section. + */ export function isExposedRow(row:HTMLTableRowElement):boolean { return !row.hidden && row.getAttribute('aria-hidden') !== 'true' @@ -68,18 +71,16 @@ export function isExposedRow(row:HTMLTableRowElement):boolean { } export function occurrenceRowKind(occurrence:RenderedOccurrence):OccurrenceRowKind { - if (occurrence.key === placeholderOccurrenceKey()) return 'placeholder'; - if (occurrence.key.startsWith('group:')) return occurrence.key.endsWith(':sums') ? 'sums' : 'groupHeader'; - return occurrence.workPackageId === null ? 'foreign' : 'workPackage'; + return occurrence.kind; } -/** Rendered occurrences keep their kind even while hidden; other rows count only while exposed. */ +/** Rendered occurrences and structural rows keep their kind even while hidden; an undeclared row counts only while exposed. */ export function classifyRow(table:WorkPackageTable, row:HTMLTableRowElement):TableRowKind { const occurrence = table.ledger.byElement(row); if (occurrence) return occurrenceRowKind(occurrence); - if (!isExposedRow(row)) return 'unexposed'; - if (isEmptyResultsRow(row)) return 'empty'; + if (isEmptyResultsRow(row)) return 'emptyResults'; if (isTotalsRow(table, row)) return 'totals'; - if (isInlineCreateRow(row)) return 'create'; + if (isInlineCreateRow(row)) return 'inlineCreate'; + if (!isExposedRow(row)) return 'unexposed'; return 'foreign'; } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.spec.ts index ae9f30e2ec5d..457dae1094d6 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.spec.ts @@ -40,6 +40,7 @@ import { function draftOccurrence(key:OccurrenceKey, overrides:Partial = {}):DraftOccurrence { return { key, + kind: 'workPackage', classIdentifier: 'wp-row-1', workPackageId: '1', renderType: 'primary', diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.ts index 0b2be5f74316..f02b0f07ff03 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/rendered-occurrence-ledger.ts @@ -33,8 +33,11 @@ declare const ngDevMode:boolean|undefined; export type OccurrenceKey = string; +export type OccurrenceKind = 'workPackage'|'groupHeader'|'sums'|'placeholder'; + export interface RenderedOccurrence { readonly key:OccurrenceKey; + readonly kind:OccurrenceKind; readonly classIdentifier:string; readonly workPackageId:string|null; readonly renderType:'primary'|'relations'|'child_relations'; diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/row-classifier.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/row-classifier.spec.ts new file mode 100644 index 000000000000..bf8563d0747e --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/row-classifier.spec.ts @@ -0,0 +1,132 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { waitFor } from '@testing-library/dom'; +import { WorkPackageViewHierarchiesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-hierarchy.service'; +import { buildTable, TableHarness, TableHarnessOptions } from '../testing/table-harness'; +import type { WorkPackageFixture } from '../testing/work-package-fixture'; +import { ancestorOccurrenceKey, groupOccurrenceKey, wpOccurrenceKey } from '../rendered-occurrence-ledger'; +import type { RowRecord } from './row-classifier'; + +const newStatus = { href: '/api/v3/statuses/1' }; +const parent:WorkPackageFixture = { id: '1' }; + +describe('RowClassifier', () => { + let harness:TableHarness; + + const mount = async (options:Partial = {}) => { + harness = buildTable({ workPackages: [{ id: '1' }, { id: '2' }], productionMarkup: true, ...options }); + await harness.render(); + }; + const rows = () => harness.table.rows.rows(); + const byKey = (key:string):RowRecord => rows().find((row) => row.key === key)!; + + afterEach(() => harness?.destroy()); + + it('lists the header, every rendered occurrence and the trailing rows in document order', async () => { + await mount(); + + expect(rows().map(({ key, state }) => [key, state.kind])).toEqual([ + ['header', 'header'], + [wpOccurrenceKey('1'), 'workPackage'], + [wpOccurrenceKey('2'), 'workPackage'], + ['totals', 'totals'], + ]); + expect(byKey(wpOccurrenceKey('1')).element).toBe(harness.row('1')); + expect(byKey('header').element).toBe(harness.headerRow()); + }); + + it('reports selection only for work package occurrences', async () => { + await mount(); + harness.click('2'); + + expect(byKey(wpOccurrenceKey('1')).state.selected).toBe(false); + expect(byKey(wpOccurrenceKey('2')).state.selected).toBe(true); + expect(byKey('header').state.selected).toBeNull(); + }); + + it('classifies group headers and sums with their expansion', async () => { + await mount({ + workPackages: [{ id: '1', attributes: { status: newStatus } }], + groups: [{ value: 'New', href: newStatus.href, count: 1, sums: { storyPoints: 5 } }], + }); + + expect(byKey(groupOccurrenceKey(0, 'header')).state).toMatchObject({ kind: 'groupHeader', expanded: true }); + expect(byKey(groupOccurrenceKey(0, 'sums')).state).toMatchObject({ kind: 'sums', expanded: null }); + }); + + it('marks hierarchy parents expandable and leaves leaves without expansion', async () => { + await mount({ workPackages: [{ id: '2', ancestors: [parent] }], showHierarchies: true }); + + expect(byKey(ancestorOccurrenceKey('1')).state).toMatchObject({ kind: 'workPackage', expanded: true, level: 1 }); + expect(byKey(wpOccurrenceKey('2')).state).toMatchObject({ kind: 'workPackage', expanded: null, level: 2 }); + }); + + it('exposes no expansion for a parent whose subject column, and so its toggle, is hidden', async () => { + await mount({ workPackages: [{ id: '2', ancestors: [parent] }], showHierarchies: true, columns: ['id', 'status'] }); + + expect(byKey(ancestorOccurrenceKey('1')).state.expanded).toBeNull(); + }); + + it('includes the empty-results row and the inline-create row with fixed keys', async () => { + await mount({ workPackages: [], configuration: { inlineCreateEnabled: true } }); + const emptyBody = harness.tbody.closest('table')!.createTBody(); + emptyBody.classList.add('work-package--empty-tbody'); + const empty = emptyBody.insertRow(); + empty.insertCell().textContent = 'No work packages'; + const create = harness.addInlineCreateRow(); + + expect(byKey('emptyResults').element).toBe(empty); + expect(byKey('inlineCreate')).toMatchObject({ element: create, state: { kind: 'inlineCreate' } }); + }); + + it('hides the totals row while it has no navigable cells', async () => { + await mount(); + + expect(byKey('totals').state.hidden).toBe(false); + expect(byKey('header').state.hidden).toBe(false); + harness.totalsRow().replaceChildren(document.createElement('td')); + + expect(byKey('totals').state.hidden).toBe(true); + }); + + it('hides the occurrences of a collapsed hierarchy parent', async () => { + await mount({ workPackages: [{ id: '2', ancestors: [parent] }], showHierarchies: true }); + harness.injector.get(WorkPackageViewHierarchiesService).collapse('1'); + + await waitFor(() => expect(byKey(wpOccurrenceKey('2')).state.hidden).toBe(true)); + expect(byKey(ancestorOccurrenceKey('1')).state.hidden).toBe(false); + }); + + it('reports a row no producer declared as foreign rather than skipping it', async () => { + await mount(); + const stray = harness.tbody.insertRow(); + stray.insertCell().textContent = 'stray'; + + expect(rows().find((row) => row.element === stray)).toMatchObject({ key: null, state: { kind: 'foreign' } }); + }); +}); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/row-classifier.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/row-classifier.ts new file mode 100644 index 000000000000..d5b579e03cdd --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/row-classifier.ts @@ -0,0 +1,126 @@ +//-- copyright +// OpenProject is an open source project management software. +// Copyright (C) the OpenProject GmbH +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +// +// See COPYRIGHT and LICENSE files for more details. +//++ + +import { Injector } from '@angular/core'; +import { WorkPackageViewHierarchiesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-hierarchy.service'; +import { WorkPackageViewSelectionService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service'; +import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; +import type { RenderedOccurrence } from '../rendered-occurrence-ledger'; +import type { WorkPackageTable } from '../wp-fast-table'; +import { classifyRow, isExposedRow, type TableRowKind } from '../helpers/structural-rows'; +import { expansionStateOf } from '../helpers/row-expansion'; +import { navigableCells } from '../handlers/state/keyboard/roving-tabindex'; + +export type RowRecordKind = TableRowKind|'header'; + +const edgeRowKeys = { + header: 'header', + totals: 'totals', + inlineCreate: 'inlineCreate', + emptyResults: 'emptyResults', +} as const; + +type EdgeRowKind = keyof typeof edgeRowKeys; + +const isEdgeRowKind = (kind:RowRecordKind):kind is EdgeRowKind => kind in edgeRowKeys; + +export interface RowState { + readonly kind:RowRecordKind; + readonly level:number; + readonly expanded:boolean|null; + readonly selected:boolean|null; + /** Kept out of keyboard navigation, and for the totals row also `aria-hidden`. */ + readonly hidden:boolean; +} + +export interface RowRecord { + readonly key:string|null; + readonly element:HTMLTableRowElement; + readonly occurrence:RenderedOccurrence|null; + readonly state:RowState; +} + +export class RowClassifier { + @LazyInject() private readonly wpTableSelection:WorkPackageViewSelectionService; + + @LazyInject() private readonly wpTableHierarchies:WorkPackageViewHierarchiesService; + + public readonly injector:Injector = this.table.injector; + + constructor(private readonly table:WorkPackageTable) {} + + /** Every row of the table, in document order, across all its sections. */ + public rows():RowRecord[] { + const tableElement = this.table.tbody.closest('table'); + if (!tableElement) return []; + return Array.from(tableElement.rows).map((row) => this.record(tableElement, row)); + } + + /** The rows of one table section, in document order. */ + public rowsOf(section:HTMLTableSectionElement):RowRecord[] { + const tableElement = section.closest('table'); + if (!tableElement) return []; + return Array.from(section.rows).map((row) => this.record(tableElement, row)); + } + + private record(tableElement:HTMLTableElement, row:HTMLTableRowElement):RowRecord { + const occurrence = this.table.ledger.byElement(row) ?? null; + const kind = this.kindOf(tableElement, row); + return { + key: occurrence?.key ?? this.edgeKeyOf(kind), + element: row, + occurrence, + state: { + kind, + level: occurrence?.level ?? 1, + expanded: this.expansionOf(occurrence, kind), + selected: occurrence?.workPackageId ? this.wpTableSelection.isSelected(occurrence.workPackageId) : null, + hidden: this.hiddenOf(row, occurrence, kind), + }, + }; + } + + private kindOf(tableElement:HTMLTableElement, row:HTMLTableRowElement):RowRecordKind { + if (row.parentElement === tableElement.tHead) return 'header'; + return classifyRow(this.table, row); + } + + private edgeKeyOf(kind:RowRecordKind):string|null { + return isEdgeRowKind(kind) ? edgeRowKeys[kind] : null; + } + + private hiddenOf(row:HTMLTableRowElement, occurrence:RenderedOccurrence|null, kind:RowRecordKind):boolean { + if (occurrence) return occurrence.kind === 'placeholder' || occurrence.hidden; + if (kind === 'totals') return navigableCells(row).length === 0; + return !isExposedRow(row); + } + + private expansionOf(occurrence:RenderedOccurrence|null, kind:RowRecordKind):boolean|null { + if (!occurrence || (kind !== 'workPackage' && kind !== 'groupHeader')) return null; + return expansionStateOf(this.table, this.wpTableHierarchies, occurrence, kind); + } +} diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/row-state-painter.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/row-state-painter.ts new file mode 100644 index 000000000000..c46905a1ce0e --- /dev/null +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/row-state-painter.ts @@ -0,0 +1,104 @@ +//-- 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 { RenderDraft } from '../rendered-occurrence-ledger'; +import type { WorkPackageTable } from '../wp-fast-table'; +import { TableRowPositions, type RowPositionEvidence } from './table-row-positions'; +import { paintRowRole, paintSectionExposure, setOrRemove, TreegridSemantics } from './treegrid-semantics'; + +/** `'all'` or the keys of the rows whose own attributes changed; row positions are always repainted for the whole table. */ +export type PaintScope = 'all'|ReadonlySet; + +export interface RowReplacement { + readonly key:string; + readonly previous:HTMLTableRowElement|null; + readonly replacement:HTMLTableRowElement; +} + +export interface PaintResult { + /** Why row positions were withdrawn or not offered, or null when they are painted. */ + readonly positionsWithdrawn:string|null; +} + +export class RowStatePainter { + private readonly semantics = new TreegridSemantics(this.table); + + private readonly positions = new TableRowPositions(this.table); + + constructor(private readonly table:WorkPackageTable) {} + + public paint(scope:PaintScope = 'all'):PaintResult { + if (scope === 'all') { + this.semantics.paint(); + } else { + this.semantics.paintKeys(scope); + } + this.paintEdgeRows(); + return { positionsWithdrawn: this.positions.paint() }; + } + + /** The column header, totals, inline-create and empty-results rows live in their own sections, outside the results body. */ + private paintEdgeRows():void { + if (this.table.destroyed) return; + const tableElement = this.table.tbody.closest('table'); + if (tableElement?.tHead) { + setOrRemove(tableElement.tHead, 'role', 'rowgroup'); + Array.from(tableElement.tHead.rows).forEach((row) => paintRowRole(row, 1, 'columnheader')); + } + const sections = Array.from(tableElement?.tBodies ?? []); + sections + .filter((section) => section !== this.table.tbody) + .flatMap((section) => this.table.rows.rowsOf(section)) + .forEach(({ element, state: { kind, hidden } }) => { + if (!['totals', 'inlineCreate', 'emptyResults'].includes(kind)) return; + paintRowRole(element, 1); + if (kind === 'totals') setOrRemove(element, 'aria-hidden', hidden ? 'true' : null); + }); + sections.forEach(paintSectionExposure); + } + + /** The keys of every rendered occurrence of a work package. */ + public keysOf(workPackageId:string):ReadonlySet { + return new Set(this.table.ledger.byWorkPackageId(workPackageId).map((occurrence) => occurrence.key)); + } + + public capturePositions(draft:RenderDraft):RowPositionEvidence { + return this.positions.capture(draft); + } + + public commitPositions(evidence:RowPositionEvidence):void { + this.positions.commit(evidence); + } + + /** Refreshed rows replace their predecessors in place under the same keys, so they inherit those rows' indices instead of repainting positions. */ + public paintRefreshed(replacements:readonly RowReplacement[]):void { + replacements.forEach(({ previous, replacement }) => { + if (previous) this.positions.carry(previous, replacement); + }); + this.semantics.paintKeys(new Set(replacements.map(({ key }) => key))); + } +} 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 index 4f64abea0b3e..88acf7515bee 100644 --- 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 @@ -33,7 +33,6 @@ import { nextTask } from 'core-common/testing/timing'; import { States } from 'core-app/core/states/states.service'; import { groupOccurrenceKey, type RenderDraft } from '../rendered-occurrence-ledger'; import { buildWorkPackage } from '../testing/work-package-fixture'; -import { environment } from '../../../../../../environments/environment'; function deferred() { let resolve!:(value:T) => void; @@ -78,8 +77,8 @@ describe('TableRowPositions', () => { it('reprojects local structure without changing the body coordinates', async () => { await mount(); - harness.totalsRow().setAttribute('aria-hidden', 'true'); - harness.table.rowPositions.paint(); + harness.totalsRow().replaceChildren(); + harness.table.painter.paint(new Set()); expect(harness.totalsRow()).not.toHaveAttribute('aria-rowindex'); expect(harness.row('21')).toHaveAttribute('aria-rowindex', '22'); expect(createRow()).toHaveAttribute('aria-rowindex', '102'); @@ -91,7 +90,7 @@ describe('TableRowPositions', () => { const nested = document.createElement('table'); nested.innerHTML = 'nested'; harness.row('21').cells[0].append(nested); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); expect(nested.rows[0]).toHaveAttribute('aria-rowindex', '7'); expect(table()).toHaveAttribute('aria-rowcount', '103'); }); @@ -100,15 +99,15 @@ describe('TableRowPositions', () => { await mount(); const extra = table().createTBody().insertRow(); extra.insertCell().textContent = 'Unclassified'; - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); harness.expectNoRowPositions(); }); it('reports a foreign row once per table', async () => { await mount(); table().createTBody().insertRow().insertCell().textContent = 'Unclassified'; - harness.table.rowPositions!.paint(); - harness.table.rowPositions!.paint(); + harness.table.painter.paint(new Set()); + harness.table.painter.paint(new Set()); expect(captureMessage).toHaveBeenCalledExactlyOnceWith('Row positions withdrawn: foreign row', 'warning'); }); @@ -117,66 +116,66 @@ describe('TableRowPositions', () => { const grouped = { workPackages: [{ id: '1', attributes: { status } }], pagination: { offset: 1, pageSize: 20, total: 1 }, groups: [{ value: 'G', href: status.href, count: 1 }] }; const hierarchy = { workPackages: [{ id: '1' }, { id: '2', ancestors: [{ id: '1' }] }], showHierarchies: true }; const recapture = (draft:RenderDraft = harness.table.ledger.beginRender()) => { - const positions = harness.table.rowPositions; - positions.commit(positions.capture(draft)); - positions.paint(); + const { painter } = harness.table; + painter.commitPositions(painter.capturePositions(draft)); + painter.paint(new Set()); }; - const withdrawals:[string, () => Promise][] = [ - ['withdrawn: foreign row', async () => { + const withdrawals:[string, string, () => Promise][] = [ + ['withdrawn', 'foreign row', async () => { await mount(); table().createTBody().insertRow().insertCell().textContent = 'Unclassified'; - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); }], - ['withdrawn: untrusted collection href', () => mount({ + ['withdrawn', 'untrusted collection href', () => 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', })], - ['withdrawn: non-monotonic', async () => { + ['withdrawn', 'non-monotonic', async () => { await mount({ workPackages: [{ id: '1' }, { id: '2', ancestors: [{ id: '1' }] }], showHierarchies: true }); harness.row('1').before(harness.row('2')); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); }], - ['withdrawn: group objects differ from collection', async () => { + ['withdrawn', 'group objects differ from collection', async () => { await mount(grouped); harness.querySpace.groups.putValue([...harness.querySpace.groups.value!]); recapture(); }], - ['withdrawn: invalid pagination', async () => { + ['withdrawn', 'invalid pagination', async () => { await mount(hierarchy); harness.querySpace.results.value!.offset = 0; recapture(); }], - ['withdrawn: duplicate occurrence keys', async () => { + ['withdrawn', 'duplicate occurrence keys', async () => { await mount(hierarchy); recapture({ occurrences: [{ key: 'wp:1' }, { key: 'wp:1' }] } as unknown as RenderDraft); }], - ['withdrawn: header rows', async () => { + ['withdrawn', 'header rows', async () => { await mount(); table().tHead!.insertRow(); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); }], - ['withdrawn: unsafe count', async () => { + ['withdrawn', 'unsafe count', async () => { await mount(); const ordinals = new Map([['wp:21', 21], ['wp:22', 22]]); - harness.table.rowPositions.commit({ kind: 'query', source: harness.querySpace.results.value!, body: { count: Number.MAX_SAFE_INTEGER, ordinals } }); - harness.table.rowPositions.paint(); + harness.table.painter.commitPositions({ kind: 'query', source: harness.querySpace.results.value!, body: { count: Number.MAX_SAFE_INTEGER, ordinals } }); + harness.table.painter.paint(new Set()); }], - ['not offered: rows await a matching collection', async () => { + ['not offered', 'rows await a matching collection', async () => { await mount(); harness.table.originalRows.reverse(); recapture(); }], - ['not offered: collection changed before render', async () => { + ['not offered', 'collection changed before render', async () => { await mount(); vi.spyOn(harness.table.uiWork, 'frame').mockImplementation(() => undefined); void harness.render([{ id: '31' }]); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); }], - ['not offered: grouped hierarchy', () => mount({ ...grouped, showHierarchies: true })], - ['not offered: grouped baseline', () => mount({ ...grouped, baseline: true })], - ['not offered: expanded relation columns', async () => { + ['not offered', 'grouped hierarchy', () => mount({ ...grouped, showHierarchies: true })], + ['not offered', 'grouped baseline', () => mount({ ...grouped, baseline: true })], + ['not offered', 'expanded relation columns', async () => { harness = buildTable({ workPackages: [{ id: '21' }], productionMarkup: true, pagination: { offset: 2, pageSize: 20, total: 100 }, @@ -187,21 +186,10 @@ describe('TableRowPositions', () => { }], ]; - const originalProduction = environment.production; - let log:ReturnType; - beforeEach(() => { - environment.production = false; - log = vi.spyOn(console, 'log').mockImplementation(() => undefined); - }); - afterEach(() => { - environment.production = originalProduction; - log.mockRestore(); - }); - - it.each(withdrawals)('logs the reason %s', async (reason, withdraw) => { + it.each(withdrawals)('reports %s positions on later paints: %s', async (_outcome, reason, withdraw) => { await withdraw(); harness.expectNoRowPositions(); - expect(log).toHaveBeenCalledWith(`[DEBUG] Row positions ${reason}`); + expect(harness.table.painter.paint(new Set()).positionsWithdrawn).toBe(reason); }); }); @@ -273,10 +261,10 @@ describe('TableRowPositions', () => { await mountHierarchy(); const restore = harness.row('2').nextSibling; harness.row('1').before(harness.row('2')); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); harness.expectNoRowPositions(); harness.tbody.insertBefore(harness.row('2'), restore); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); harness.expectRowPositions(['1', '22', '23', '24', '25'], '-1'); }); @@ -287,7 +275,7 @@ describe('TableRowPositions', () => { const row = empty.insertRow(); row.insertCell().textContent = 'No results'; harness.tbody.before(empty); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); expect(row).toHaveAttribute('aria-rowindex', '2'); expect(table()).toHaveAttribute('aria-rowcount', '-1'); }); @@ -316,7 +304,7 @@ describe('TableRowPositions', () => { const row = empty.insertRow(); row.insertCell().textContent = 'No results'; harness.tbody.before(empty); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); expect(row).toHaveAttribute('aria-rowindex', '2'); expect(harness.totalsRow()).toHaveAttribute('aria-rowindex', '3'); expect(table()).toHaveAttribute('aria-rowcount', '4'); @@ -329,7 +317,7 @@ describe('TableRowPositions', () => { empty.className = 'work-package--empty-tbody'; empty.insertRow().insertCell().textContent = 'No results'; harness.tbody.before(empty); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); harness.expectNoRowPositions(); }); @@ -341,8 +329,8 @@ describe('TableRowPositions', () => { new Map([['wp:21', 21]]), ]; for (const ordinals of variants) { - harness.table.rowPositions.commit({ kind: 'query', source: harness.querySpace.results.value!, body: { count: 100, ordinals } }); - harness.table.rowPositions.paint(); + harness.table.painter.commitPositions({ kind: 'query', source: harness.querySpace.results.value!, body: { count: 100, ordinals } }); + harness.table.painter.paint(new Set()); harness.expectNoRowPositions(); } }); @@ -352,7 +340,7 @@ describe('TableRowPositions', () => { 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(); + harness.table.painter.paint(new Set()); expect(table()).not.toHaveAttribute('aria-rowcount'); expect(harness.row('21')).not.toHaveAttribute('aria-rowindex'); frames.splice(0).forEach((callback) => callback()); @@ -373,18 +361,18 @@ describe('TableRowPositions', () => { it('finds rendered rows without scanning the ledger', async () => { await mount(); const scanned = vi.spyOn(harness.table.ledger, 'occurrences', 'get'); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); expect(scanned).not.toHaveBeenCalled(); harness.expectRowPositions(['1', '22', '23', '102', '103'], '103'); }); it('skips the row scan while positions stay withdrawn', async () => { await mount(); - harness.table.rowPositions.commit(null); - harness.table.rowPositions.paint(); + harness.table.painter.commitPositions(null); + harness.table.painter.paint(new Set()); harness.expectNoRowPositions(); const scanned = vi.spyOn(table(), 'rows', 'get'); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); expect(scanned).not.toHaveBeenCalled(); }); @@ -397,7 +385,7 @@ describe('TableRowPositions', () => { occurrence.element!.replaceWith(replacement); return replacement; }); - const painted = vi.spyOn(harness.table.rowPositions, 'paint'); + const painted = vi.spyOn(harness.table.painter, 'paint'); for (let refresh = 0; refresh < 50; refresh += 1) { harness.table.refreshRows(harness.table.originalRowIndex[refresh % 2 ? '22' : '21'].object); } @@ -423,7 +411,7 @@ describe('TableRowPositions', () => { await mount(); await harness.destroy(); table().removeAttribute('aria-rowcount'); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); expect(table()).not.toHaveAttribute('aria-rowcount'); }); @@ -684,7 +672,7 @@ describe('TableRowPositions', () => { 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.table.painter.paint(new Set()); harness.expectNoRowPositions(); }); @@ -693,7 +681,7 @@ describe('TableRowPositions', () => { const records:MutationRecord[] = []; const observer = new MutationObserver((batch) => records.push(...batch)); observer.observe(table(), { subtree: true, attributes: true }); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); await nextTask(); observer.disconnect(); expect(records).toEqual([]); @@ -704,7 +692,7 @@ describe('TableRowPositions', () => { const row = harness.row('21'); row.setAttribute('aria-posinset', '21'); row.setAttribute('aria-setsize', '100'); - harness.table.rowPositions.paint(); + harness.table.painter.paint(new Set()); 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 index eb52a20babb4..fbbe73472476 100644 --- 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 @@ -35,7 +35,7 @@ import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorato import { debugLog } from 'core-app/shared/helpers/debug_output'; import { groupOccurrenceKey, placeholderOccurrenceKey, wpOccurrenceKey, type RenderDraft } from '../rendered-occurrence-ledger'; import type { WorkPackageTable } from '../wp-fast-table'; -import { classifyRow } from '../helpers/structural-rows'; +import { classifyRow, isExposedRow } from '../helpers/structural-rows'; import { setOrRemove } from './treegrid-semantics'; import { bodyRowPositions, draftAgrees, type BodyRowPositions, type PositionGroup } from './body-row-positions'; @@ -46,6 +46,18 @@ export interface CapturedRowPositions { readonly body:BodyRowPositions; } +export interface WithdrawnRowPositions { + readonly withdrawn:string; +} + +/** What a render pass learned about row positions; null when there was no collection to check. */ +export type RowPositionEvidence = CapturedRowPositions|WithdrawnRowPositions|null; + +interface Projection { + readonly count:number|null; + readonly indices:Map; +} + // Group counts are computed from the unscoped query results, so grouped positions trust only the query collections: // `/api/v3/work_packages` and `/api/v3/workspaces/:id/work_packages` (the self links // `WorkPackageCollectionFromQueryService` emits), plus the `projects/:id` alias of the latter. @@ -67,9 +79,13 @@ export class TableRowPositions { private foreignRowReported = false; + private withdrawal:string|null = null; + + private committedWithdrawal:WithdrawnRowPositions|null = null; + constructor(private readonly table:WorkPackageTable) {} - public capture(draft:RenderDraft):CapturedRowPositions|null { + public capture(draft:RenderDraft):RowPositionEvidence { const source = this.table.querySpace.results.value; if (!source) return null; if (this.withdrawnByMode()) return this.notOffered('expanded relation columns'); @@ -104,22 +120,15 @@ export class TableRowPositions { return draftAgrees(body, rows, positionGroups) ? { kind: 'query', source, body } : this.withdraw('rows disagree with collection'); } - public commit(snapshot:CapturedRowPositions|null):void { - this.snapshot = snapshot; + public commit(evidence:RowPositionEvidence):void { + this.snapshot = evidence && 'kind' in evidence ? evidence : null; + this.committedWithdrawal = evidence && 'withdrawn' in evidence ? evidence : null; } /** 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 || (!this.currentSnapshot() && !element.hasAttribute('aria-rowcount'))) return; - const rows = Array.from(element.rows); - const projection = this.project(element, rows); - setOrRemove(element, 'aria-rowcount', projection ? String(projection.count ?? -1) : null); - rows.forEach((row) => { - const position = projection?.indices.get(row); - setOrRemove(row, 'aria-rowindex', position === undefined ? null : String(position)); - }); + public paint():string|null { + this.withdrawal = this.paintPositions(); + return this.withdrawal; } /** A refreshed row replaces its predecessor in place under the same occurrence key, so it inherits that row's index. */ @@ -133,7 +142,7 @@ export class TableRowPositions { } /** Contextual ancestors make the query-wide total unknowable, so rendered rows count from the page's first rank. */ - private capturePage(source:WorkPackageCollectionResource, draft:RenderDraft):CapturedRowPositions|null { + private capturePage(source:WorkPackageCollectionResource, draft:RenderDraft):CapturedRowPositions|WithdrawnRowPositions { const { offset, pageSize } = source; const firstRank = (offset - 1) * pageSize; if (!Number.isSafeInteger(offset) || offset < 1 || !Number.isSafeInteger(pageSize) || pageSize < 1 @@ -160,30 +169,51 @@ export class TableRowPositions { && source.elements.every((wp, index) => wp.id === originalRows[index]); } - private project(element:HTMLTableElement, rows:HTMLTableRowElement[]) { - if (!this.snapshot) return null; + private paintPositions():string|null { + if (this.table.destroyed) return null; + const tableElement = this.table.tbody.closest('table'); + if (!tableElement) return null; const snapshot = this.currentSnapshot(); - if (!snapshot) return this.notOffered('collection changed before render'); + if (!snapshot && !tableElement.hasAttribute('aria-rowcount')) return this.unavailable()?.withdrawn ?? null; + + const rows = Array.from(tableElement.rows); + const result = snapshot ? this.project(snapshot, tableElement, rows) : this.unavailable(); + const projection = result && 'indices' in result ? result : null; + setOrRemove(tableElement, 'aria-rowcount', projection ? String(projection.count ?? -1) : null); + rows.forEach((row) => { + const position = projection?.indices.get(row); + setOrRemove(row, 'aria-rowindex', position === undefined ? null : String(position)); + }); + return result && 'withdrawn' in result ? result.withdrawn : null; + } + + /** Without a current snapshot, either the collection moved on since the render pass or that pass withheld positions. */ + private unavailable():WithdrawnRowPositions|null { + return this.snapshot ? this.notOffered('collection changed before render') : this.committedWithdrawal; + } + + private project(snapshot:CapturedRowPositions, tableElement:HTMLTableElement, rows:HTMLTableRowElement[]):Projection|WithdrawnRowPositions { const { body } = snapshot; - const header = element.tHead?.rows; - if (header?.length !== 1) return this.withdraw('header rows'); + const headerRows = tableElement.tHead?.rows; + if (headerRows?.length !== 1) return this.withdraw('header rows'); - const indices = new Map([[header[0], 1]]); + const indices = new Map([[headerRows[0], 1]]); const trailers:HTMLTableRowElement[] = []; - const seen = new Set<'totals'|'create'>(); + const seen = new Set<'totals'|'inlineCreate'>(); let emptySurrogate:HTMLTableRowElement|null = null; for (const row of rows) { - if (row === header[0]) continue; + if (row === headerRows[0]) continue; const kind = classifyRow(this.table, row); if (kind === 'foreign') { this.reportForeignRow(); return this.withdraw('foreign row'); } if (kind === 'placeholder' || kind === 'unexposed') continue; - if (kind === 'empty') { + if ((kind === 'emptyResults' || kind === 'totals' || kind === 'inlineCreate') && !isExposedRow(row)) continue; + if (kind === 'emptyResults') { if (body.count !== 0 || emptySurrogate) return this.withdraw('unexpected empty-results row'); emptySurrogate = row; - } else if (kind === 'totals' || kind === 'create') { + } else if (kind === 'totals' || kind === 'inlineCreate') { if (row.cells.length === 0) continue; if (seen.has(kind)) return this.withdraw(`duplicate ${kind} row`); seen.add(kind); @@ -216,9 +246,9 @@ export class TableRowPositions { return this.snapshot?.source === this.table.querySpace.results.value ? this.snapshot : null; } - private withdraw(inconsistency:string):null { + private withdraw(inconsistency:string):WithdrawnRowPositions { debugLog(`Row positions withdrawn: ${inconsistency}`); - return null; + return { withdrawn: inconsistency }; } private reportForeignRow():void { @@ -227,8 +257,8 @@ export class TableRowPositions { window.ErrorReporter.captureMessage('Row positions withdrawn: foreign row', 'warning'); } - private notOffered(mode:string):null { + private notOffered(mode:string):WithdrawnRowPositions { debugLog(`Row positions not offered: ${mode}`); - return null; + return { withdrawn: mode }; } } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/treegrid-semantics.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/treegrid-semantics.spec.ts index fe9ba399c5bc..70f99251c8c2 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/treegrid-semantics.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/treegrid-semantics.spec.ts @@ -31,6 +31,7 @@ import { map, skip, take } from 'rxjs/operators'; import { States } from 'core-app/core/states/states.service'; import { WorkPackageViewCollapsedGroupsService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-collapsed-groups.service'; import { WorkPackageViewHierarchiesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-hierarchy.service'; +import { WorkPackageViewFocusService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-focus.service'; import { buildQuery, buildTable, initializeViewServices, TableHarness, TableHarnessOptions } from '../testing/table-harness'; import { buildWorkPackage, WorkPackageFixture } from '../testing/work-package-fixture'; import { WorkPackageViewOrderService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-order.service'; @@ -217,11 +218,22 @@ describe('TreegridSemantics', () => { expect(within(harness.row('17')).getByRole('rowheader', { name: 'Alpha' })).toHaveClass('subject'); }); + it('hides the totals row while none of its cells is navigable', async () => { + mount({ productionMarkup: true }); + await harness.render(); + expect(harness.totalsRow()).not.toHaveAttribute('aria-hidden'); + + harness.totalsRow().replaceChildren(document.createElement('td')); + harness.table.painter.paint(new Set()); + + expect(harness.totalsRow()).toHaveAttribute('aria-hidden', 'true'); + }); + it('names only work package cells after repeated paints', async () => { mount({ ...groupedFixtures, productionMarkup: true }); await harness.render(); - harness.table.semantics.paint(); - harness.table.semantics.paintWorkPackage('1'); + harness.table.painter.paint(); + harness.table.painter.paint(harness.table.painter.keysOf('1')); expect(within(harness.row('1')).getByRole('rowheader', { name: 'Work package 1' })).toHaveClass('subject'); [groupHeader(0), groupHeader(1), sumsRow(0), sumsRow(1), harness.totalsRow(), harness.headerRow()] .forEach((row) => { @@ -364,6 +376,20 @@ describe('TreegridSemantics', () => { expect(nested.querySelector('td')).not.toHaveAttribute('role'); }); + it('paints selection and pressed markers as classes and attributes from one place', async () => { + mount(); + await harness.render(); + harness.selection.replaceSelection('2'); + harness.injector.get(WorkPackageViewFocusService).updateFocus('2', false, false); + + expect(harness.row('2')).toHaveAttribute('aria-selected', 'true'); + expect(harness.row('2')).toHaveClass('-checked', '-pressed'); + expect(harness.row('2')).toHaveAttribute('data-pressed'); + expect(harness.row('1')).toHaveAttribute('aria-selected', 'false'); + expect(harness.row('1')).not.toHaveClass('-checked'); + expect(harness.row('1')).not.toHaveAttribute('data-pressed'); + }); + it('exposes selection only on work package occurrences', async () => { mount({ ...groupedFixtures, @@ -534,10 +560,9 @@ describe('TreegridSemantics', () => { const before = harness.tbody.outerHTML; harness.table.refreshRows(workPackage); - const semantics = harness.table.semantics; - semantics.paint(); - semantics.paintWorkPackage('1'); - semantics.paintSelection(); + const { painter } = harness.table; + painter.paint(); + painter.paint(painter.keysOf('1')); expect(row).not.toHaveAttribute('aria-level'); expect(row).toHaveAttribute('aria-selected', 'false'); @@ -600,7 +625,7 @@ describe('TreegridSemantics', () => { await harness.render(); const observer = new MutationObserver(() => undefined); observer.observe(harness.tbody, { subtree: true, attributes: true, attributeFilter: ['aria-expanded', 'aria-hidden'] }); - harness.table.semantics.paint(); + harness.table.painter.paint(); const records = observer.takeRecords(); observer.disconnect(); expect(records).toEqual([]); diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/treegrid-semantics.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/treegrid-semantics.ts index 395e6a2db188..41bd1186975b 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/treegrid-semantics.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/semantics/treegrid-semantics.ts @@ -28,9 +28,11 @@ import { Injector } from '@angular/core'; import { WorkPackageViewHierarchiesService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-hierarchy.service'; import { WorkPackageViewSelectionService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-selection.service'; +import { WorkPackageViewFocusService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-focus.service'; import { isSelectableOccurrence } from 'core-app/features/work-packages/routing/wp-view-base/view-services/rendered-occurrences'; import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; import { baselineStatusName } from '../builders/baseline/baseline-column-builder'; +import { paintRowSelection } from '../builders/rows/row-selection-painter'; import { expansionStateOf } from '../helpers/row-expansion'; import { occurrenceRowKind, type OccurrenceRowKind } from '../helpers/structural-rows'; import type { RenderedOccurrence } from '../rendered-occurrence-ledger'; @@ -50,17 +52,17 @@ export function paintRowRole( cellRole:'gridcell'|'columnheader' = 'gridcell', nameIdPrefix:string|null = null, ):void { - row.setAttribute('role', 'row'); - row.setAttribute('aria-level', String(level)); + setOrRemove(row, 'role', 'row'); + setOrRemove(row, 'aria-level', String(level)); let header:HTMLTableCellElement|undefined; Array.from(row.cells).forEach((cell) => { if (cellRole === 'gridcell' && cell.matches('th[scope="row"]')) { header = cell; - cell.removeAttribute('role'); - cell.removeAttribute('aria-labelledby'); - cell.setAttribute('aria-label', cell.dataset.rowHeaderName!); + setOrRemove(cell, 'role', null); + setOrRemove(cell, 'aria-labelledby', null); + setOrRemove(cell, 'aria-label', cell.dataset.rowHeaderName ?? null); } else { - cell.setAttribute('role', cellRole); + setOrRemove(cell, 'role', cellRole); } }); const named = header && nameIdPrefix !== null ? header : null; @@ -74,8 +76,12 @@ function assignId(element:HTMLElement, id:string):string { return id; } +/** + * Marks the section as a `rowgroup` and hides it from assistive technology (`aria-hidden`) when + * every row in it is `aria-hidden`. + */ export function paintSectionExposure(section:HTMLTableSectionElement):void { - section.setAttribute('role', 'rowgroup'); + setOrRemove(section, 'role', 'rowgroup'); const exposed = Array.from(section.rows).some((row) => row.getAttribute('aria-hidden') !== 'true'); setOrRemove(section, 'aria-hidden', exposed ? null : 'true'); } @@ -85,6 +91,8 @@ export class TreegridSemantics { @LazyInject() private readonly wpTableHierarchies:WorkPackageViewHierarchiesService; + @LazyInject() private readonly wpTableFocus:WorkPackageViewFocusService; + public readonly injector:Injector = this.table.injector; constructor(private readonly table:WorkPackageTable) {} @@ -95,14 +103,12 @@ export class TreegridSemantics { paintSectionExposure(this.table.tbody); } - public paintWorkPackage(id:string):void { - if (this.table.destroyed) return; - this.table.ledger.byWorkPackageId(id).forEach((occurrence) => this.paintOccurrence(occurrence)); - } - - public paintSelection():void { + public paintKeys(keys:ReadonlySet):void { if (this.table.destroyed) return; - this.table.ledger.occurrences.forEach((occurrence) => this.paintSelected(occurrence)); + keys.forEach((key) => { + const occurrence = this.table.ledger.byKey(key); + if (occurrence) this.paintOccurrence(occurrence); + }); } private paintOccurrence(occurrence:RenderedOccurrence):void { @@ -121,13 +127,17 @@ export class TreegridSemantics { } private paintSelected(occurrence:RenderedOccurrence):void { - if (occurrence.element && isSelectableOccurrence(occurrence)) { - occurrence.element.setAttribute('aria-selected', String(this.wpTableSelection.isSelected(occurrence.workPackageId))); - } + const row = occurrence.element; + if (!row || !isSelectableOccurrence(occurrence)) return; + const selected = this.wpTableSelection.isSelected(occurrence.workPackageId); + const pressed = this.wpTableFocus.isFocused(occurrence.workPackageId); + setOrRemove(row, 'aria-selected', String(selected)); + setOrRemove(row, 'data-pressed', pressed ? '' : null); + paintRowSelection(row, { selected, pressed }); } private expansionOf(occurrence:RenderedOccurrence, kind:Exclude):string|null { - const expanded = kind === 'foreign' ? null : expansionStateOf(this.table, this.wpTableHierarchies, occurrence, kind); + const expanded = expansionStateOf(this.table, this.wpTableHierarchies, occurrence, kind); return expanded === null ? null : String(expanded); } } diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table-lifecycle.spec.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table-lifecycle.spec.ts index eed4d682d1a7..68dc575551b2 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table-lifecycle.spec.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/wp-fast-table-lifecycle.spec.ts @@ -455,6 +455,22 @@ describe('WorkPackageTable lifecycle', () => { await waitFor(() => expect(build).toHaveBeenLastCalledWith(latest, undefined)); }); + it('repaints the rows whose drag handle cells it swaps in', async () => { + const harness = await mount({ workPackages: [{ id: '1' }], configuration: { dragAndDropEnabled: true } }); + const positions = deferred(); + vi.spyOn(harness.injector.get(WorkPackageViewOrderService), 'positionsFor').mockReturnValue(positions.promise); + harness.table.redrawTableAndTimeline(); + await nextFrame(); + const before = harness.row('1').firstElementChild; + const paint = vi.spyOn(harness.table.painter, 'paint'); + + positions.resolve({ '1': 100 }); + + await waitFor(() => expect(harness.row('1').firstElementChild).not.toBe(before)); + expect(paint).toHaveBeenCalledWith(new Set(['wp:1'])); + expect(harness.row('1').firstElementChild).toHaveAttribute('role', 'gridcell'); + }); + it('registers the drag-and-drop placeholder without rendering it on the timeline', async () => { const harness = await mount({ workPackages: [], 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 f35c68da386c..7cb5fa88c1d4 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,13 +50,14 @@ import { WorkPackageTableEditingContext } from './wp-table-editing'; import type { WorkPackageTableRow } from './wp-table.interfaces'; import type { KeyboardNavigation } from './handlers/state/keyboard/keyboard-navigation-transformer'; import type { TableSelectionFeedback } from './handlers/state/selection-feedback'; -import { TableRowPositions, type CapturedRowPositions } from './semantics/table-row-positions'; -import { paintSectionExposure, TreegridSemantics } from './semantics/treegrid-semantics'; +import type { RowPositionEvidence } from './semantics/table-row-positions'; +import { RowStatePainter, type RowReplacement } from './semantics/row-state-painter'; +import { RowClassifier } from './semantics/row-classifier'; interface PendingRender { readonly pass:PrimaryRenderPass; readonly timeline:boolean; - readonly positions:CapturedRowPositions|null; + readonly positions:RowPositionEvidence; } export class WorkPackageTable { @@ -103,9 +104,9 @@ export class WorkPackageTable { public selectionFeedback:TableSelectionFeedback|null = null; - public readonly semantics = new TreegridSemantics(this); + public readonly rows = new RowClassifier(this); - public readonly rowPositions = new TableRowPositions(this); + public readonly painter = new RowStatePainter(this); private readonly rowsRefreshedSubject = new Subject(); @@ -243,32 +244,30 @@ export class WorkPackageTable { } this.keyboardNavigation?.captureOwnership(); - let replaced = 0; + const replacements:RowReplacement[] = []; this.ledger.byWorkPackageId(workPackage.id!).forEach((occurrence) => { const previous = occurrence.element; const replacement = pass.refresh(occurrence, workPackage); if (replacement) { this.ledger.replaceElement(occurrence.key, replacement); - if (previous) this.rowPositions.carry(previous, replacement); - replaced += 1; + replacements.push({ key: occurrence.key, previous, replacement }); } }); - if (replaced > 0) { - this.semantics.paintWorkPackage(workPackage.id!); + if (replacements.length > 0) { + this.painter.paintRefreshed(replacements); this.rowsRefreshedSubject.next(workPackage.id!); } } public notifyRowsRefreshed(key:string):void { - this.rowPositions.paint(); + this.painter.paint(new Set([key])); this.rowsRefreshedSubject.next(key); } - /** Re-exposes a structural section whose rows changed outside the render pass. */ - public reconcileSection(section:HTMLTableSectionElement):void { - paintSectionExposure(section); - this.rowPositions.paint(); + /** Repaints the structural rows and sections after rows changed outside the render pass. */ + public reconcileSection():void { + this.painter.paint(new Set()); } /** @@ -285,7 +284,7 @@ export class WorkPackageTable { this.editing.reset(); const promoted = timeline || this.pendingRender?.timeline === true; const pass = this.rowBuilder.buildRows({ timeline: promoted }); - const pending:PendingRender = { pass, timeline: promoted, positions: this.rowPositions.capture(pass.draft) }; + const pending:PendingRender = { pass, timeline: promoted, positions: this.painter.capturePositions(pass.draft) }; this.pendingRender = pending; this.uiWork.frame(() => this.commitRender(pending)); } @@ -300,14 +299,13 @@ export class WorkPackageTable { this.timelineBody.replaceChildren(timelinePass.timelineBody); } this.ledger.commit(pass.draft); - this.rowPositions.commit(pending.positions); + this.painter.commitPositions(pending.positions); this.lastRenderPass = pass; this.publishRendered(); } private publishRendered():void { - this.semantics.paint(); - this.rowPositions.paint(); + this.painter.paint(); this.querySpace.tableRendered.putValue(this.ledger.snapshot()); } diff --git a/frontend/src/app/features/work-packages/components/wp-inline-create/inline-create-row-builder.ts b/frontend/src/app/features/work-packages/components/wp-inline-create/inline-create-row-builder.ts index 0bf7bd1446a9..4fa8e45f3aa9 100644 --- a/frontend/src/app/features/work-packages/components/wp-inline-create/inline-create-row-builder.ts +++ b/frontend/src/app/features/work-packages/components/wp-inline-create/inline-create-row-builder.ts @@ -33,7 +33,6 @@ import { WorkPackageViewSelectionService } from 'core-app/features/work-packages import { WorkPackageViewColumnsService } from 'core-app/features/work-packages/routing/wp-view-base/view-services/wp-view-columns.service'; import { QueryColumn } from 'core-app/features/work-packages/components/wp-query/query-column'; import { tdClassName } from 'core-app/features/work-packages/components/wp-fast-table/builders/cell-builder'; -import { paintRowRole } from 'core-app/features/work-packages/components/wp-fast-table/semantics/treegrid-semantics'; import { internalContextMenuColumn } from 'core-app/features/work-packages/components/wp-fast-table/builders/internal-sort-columns'; import { EditForm } from 'core-app/shared/components/fields/edit/edit-form/edit-form'; import { LazyInject } from 'core-app/shared/helpers/angular/lazy-inject.decorator'; @@ -87,7 +86,7 @@ export class InlineCreateRowBuilder extends SingleRowBuilder { // Get any existing edit state for this work package const [row, hidden] = this.buildEmpty(workPackage); - return [this.paintStructuralRow(row), hidden]; + return [row, hidden]; } public buildEmpty(workPackage:WorkPackageResource):[HTMLTableRowElement, boolean] { @@ -101,7 +100,7 @@ export class InlineCreateRowBuilder extends SingleRowBuilder { const refreshed = super.refreshRow(workPackage, row); this.placeCancelLink(refreshed); - return this.paintStructuralRow(refreshed); + return refreshed; } private placeCancelLink(row:HTMLTableRowElement):void { @@ -156,12 +155,6 @@ export class InlineCreateRowBuilder extends SingleRowBuilder { return tr; } - private paintStructuralRow(row:HTMLTableRowElement):HTMLTableRowElement { - paintRowRole(row, 1); - - return row; - } - protected buildCancelButton() { const td = document.createElement('td'); td.classList.add(tdClassName, 'wp-table--cancel-create-td'); diff --git a/frontend/src/app/features/work-packages/components/wp-inline-create/wp-inline-create.component.html b/frontend/src/app/features/work-packages/components/wp-inline-create/wp-inline-create.component.html index 3f3219637937..9258d079f7b4 100644 --- a/frontend/src/app/features/work-packages/components/wp-inline-create/wp-inline-create.component.html +++ b/frontend/src/app/features/work-packages/components/wp-inline-create/wp-inline-create.component.html @@ -2,13 +2,10 @@ @if (mode === 'inactive') { @if (canAdd) {