From 5957299383e470385f54e2e6b42d97171b1a04ae Mon Sep 17 00:00:00 2001 From: Karl Seamon Date: Fri, 27 Dec 2024 13:16:18 -0500 Subject: [PATCH] perf(cdk/table): Use afterNextRender for sticky styling. Fixes a performance regression dating back to #28393 and removes need for coalesced sticky styler. --- src/cdk/table/sticky-styler.ts | 283 +++++++++++++++++++-------------- src/cdk/table/table.ts | 15 +- 2 files changed, 166 insertions(+), 132 deletions(-) diff --git a/src/cdk/table/sticky-styler.ts b/src/cdk/table/sticky-styler.ts index 286cabacc6be..ea2a5f237a33 100644 --- a/src/cdk/table/sticky-styler.ts +++ b/src/cdk/table/sticky-styler.ts @@ -10,8 +10,8 @@ * Directions that can be used when setting sticky positioning. * @docs-private */ +import {afterNextRender, Injector} from '@angular/core'; import {Direction} from '@angular/cdk/bidi'; -import {_CoalescedStyleScheduler} from './coalesced-style-scheduler'; import {StickyPositioningListener} from './sticky-position-listener'; export type StickyDirection = 'top' | 'bottom' | 'left' | 'right'; @@ -41,6 +41,7 @@ export class StickyStyler { private _stickyColumnsReplayTimeout: number | null = null; private _cachedCellWidths: number[] = []; private readonly _borderCellCss: Readonly<{[d in StickyDirection]: string}>; + private _destroyed = false; /** * @param _isNativeHtmlTable Whether the sticky logic should be based on a table @@ -60,10 +61,10 @@ export class StickyStyler { private _isNativeHtmlTable: boolean, private _stickCellCss: string, public direction: Direction, - private _coalescedStyleScheduler: _CoalescedStyleScheduler, private _isBrowser = true, private readonly _needsPositionStickyOnElement = true, private readonly _positionListener?: StickyPositioningListener, + private readonly _tableInjector?: Injector, ) { this._borderCellCss = { 'top': `${_stickCellCss}-border-elem-top`, @@ -92,18 +93,20 @@ export class StickyStyler { continue; } - elementsToClear.push(row); - for (let i = 0; i < row.children.length; i++) { - elementsToClear.push(row.children[i] as HTMLElement); - } + elementsToClear.push(row, ...(Array.from(row.children) as HTMLElement[])); } // Coalesce with sticky row/column updates (and potentially other changes like column resize). - this._coalescedStyleScheduler.schedule(() => { - for (const element of elementsToClear) { - this._removeStickyStyle(element, stickyDirections); - } - }); + afterNextRender( + { + write: () => { + for (const element of elementsToClear) { + this._removeStickyStyle(element, stickyDirections); + } + }, + }, + {injector: this._tableInjector}, + ); } /** @@ -147,54 +150,67 @@ export class StickyStyler { } // Coalesce with sticky row updates (and potentially other changes like column resize). - this._coalescedStyleScheduler.schedule(() => { - const firstRow = rows[0]; - const numCells = firstRow.children.length; - const cellWidths: number[] = this._getCellWidths(firstRow, recalculateCellWidths); - - const startPositions = this._getStickyStartColumnPositions(cellWidths, stickyStartStates); - const endPositions = this._getStickyEndColumnPositions(cellWidths, stickyEndStates); - - const lastStickyStart = stickyStartStates.lastIndexOf(true); - const firstStickyEnd = stickyEndStates.indexOf(true); - - const isRtl = this.direction === 'rtl'; - const start = isRtl ? 'right' : 'left'; - const end = isRtl ? 'left' : 'right'; - - for (const row of rows) { - for (let i = 0; i < numCells; i++) { - const cell = row.children[i] as HTMLElement; - if (stickyStartStates[i]) { - this._addStickyStyle(cell, start, startPositions[i], i === lastStickyStart); + const firstRow = rows[0]; + const numCells = firstRow.children.length; + + const isRtl = this.direction === 'rtl'; + const start = isRtl ? 'right' : 'left'; + const end = isRtl ? 'left' : 'right'; + + const lastStickyStart = stickyStartStates.lastIndexOf(true); + const firstStickyEnd = stickyEndStates.indexOf(true); + + let cellWidths: number[]; + let startPositions: number[]; + let endPositions: number[]; + + afterNextRender( + { + earlyRead: () => { + cellWidths = this._getCellWidths(firstRow, recalculateCellWidths); + + startPositions = this._getStickyStartColumnPositions(cellWidths, stickyStartStates); + endPositions = this._getStickyEndColumnPositions(cellWidths, stickyEndStates); + }, + write: () => { + for (const row of rows) { + for (let i = 0; i < numCells; i++) { + const cell = row.children[i] as HTMLElement; + if (stickyStartStates[i]) { + this._addStickyStyle(cell, start, startPositions[i], i === lastStickyStart); + } + + if (stickyEndStates[i]) { + this._addStickyStyle(cell, end, endPositions[i], i === firstStickyEnd); + } + } } - if (stickyEndStates[i]) { - this._addStickyStyle(cell, end, endPositions[i], i === firstStickyEnd); + if (this._positionListener) { + this._positionListener.stickyColumnsUpdated({ + sizes: + lastStickyStart === -1 + ? [] + : cellWidths + .slice(0, lastStickyStart + 1) + .map((width, index) => (stickyStartStates[index] ? width : null)), + }); + this._positionListener.stickyEndColumnsUpdated({ + sizes: + firstStickyEnd === -1 + ? [] + : cellWidths + .slice(firstStickyEnd) + .map((width, index) => + stickyEndStates[index + firstStickyEnd] ? width : null, + ) + .reverse(), + }); } - } - } - - if (this._positionListener) { - this._positionListener.stickyColumnsUpdated({ - sizes: - lastStickyStart === -1 - ? [] - : cellWidths - .slice(0, lastStickyStart + 1) - .map((width, index) => (stickyStartStates[index] ? width : null)), - }); - this._positionListener.stickyEndColumnsUpdated({ - sizes: - firstStickyEnd === -1 - ? [] - : cellWidths - .slice(firstStickyEnd) - .map((width, index) => (stickyEndStates[index + firstStickyEnd] ? width : null)) - .reverse(), - }); - } - }); + }, + }, + {injector: this._tableInjector}, + ); } /** @@ -214,64 +230,70 @@ export class StickyStyler { return; } - // Coalesce with other sticky row updates (top/bottom), sticky columns updates - // (and potentially other changes like column resize). - this._coalescedStyleScheduler.schedule(() => { - // If positioning the rows to the bottom, reverse their order when evaluating the sticky - // position such that the last row stuck will be "bottom: 0px" and so on. Note that the - // sticky states need to be reversed as well. - const rows = position === 'bottom' ? rowsToStick.slice().reverse() : rowsToStick; - const states = position === 'bottom' ? stickyStates.slice().reverse() : stickyStates; - - // Measure row heights all at once before adding sticky styles to reduce layout thrashing. - const stickyOffsets: number[] = []; - const stickyCellHeights: (number | undefined)[] = []; - const elementsToStick: HTMLElement[][] = []; - - for (let rowIndex = 0, stickyOffset = 0; rowIndex < rows.length; rowIndex++) { - if (!states[rowIndex]) { - continue; - } + // If positioning the rows to the bottom, reverse their order when evaluating the sticky + // position such that the last row stuck will be "bottom: 0px" and so on. Note that the + // sticky states need to be reversed as well. + const rows = position === 'bottom' ? rowsToStick.slice().reverse() : rowsToStick; + const states = position === 'bottom' ? stickyStates.slice().reverse() : stickyStates; - stickyOffsets[rowIndex] = stickyOffset; - const row = rows[rowIndex]; - elementsToStick[rowIndex] = this._isNativeHtmlTable - ? (Array.from(row.children) as HTMLElement[]) - : [row]; - - const height = this._retrieveElementSize(row).height; - stickyOffset += height; - stickyCellHeights[rowIndex] = height; - } + // Measure row heights all at once before adding sticky styles to reduce layout thrashing. + const stickyOffsets: number[] = []; + const stickyCellHeights: (number | undefined)[] = []; + const elementsToStick: HTMLElement[][] = []; - const borderedRowIndex = states.lastIndexOf(true); - - for (let rowIndex = 0; rowIndex < rows.length; rowIndex++) { - if (!states[rowIndex]) { - continue; - } - - const offset = stickyOffsets[rowIndex]; - const isBorderedRowIndex = rowIndex === borderedRowIndex; - for (const element of elementsToStick[rowIndex]) { - this._addStickyStyle(element, position, offset, isBorderedRowIndex); - } - } + // Coalesce with other sticky row updates (top/bottom), sticky columns updates + // (and potentially other changes like column resize). + afterNextRender( + { + earlyRead: () => { + for (let rowIndex = 0, stickyOffset = 0; rowIndex < rows.length; rowIndex++) { + if (!states[rowIndex]) { + continue; + } + + stickyOffsets[rowIndex] = stickyOffset; + const row = rows[rowIndex]; + elementsToStick[rowIndex] = this._isNativeHtmlTable + ? (Array.from(row.children) as HTMLElement[]) + : [row]; + + const height = this._retrieveElementSize(row).height; + stickyOffset += height; + stickyCellHeights[rowIndex] = height; + } + }, + write: () => { + const borderedRowIndex = states.lastIndexOf(true); + + for (let rowIndex = 0; rowIndex < rows.length; rowIndex++) { + if (!states[rowIndex]) { + continue; + } + + const offset = stickyOffsets[rowIndex]; + const isBorderedRowIndex = rowIndex === borderedRowIndex; + for (const element of elementsToStick[rowIndex]) { + this._addStickyStyle(element, position, offset, isBorderedRowIndex); + } + } - if (position === 'top') { - this._positionListener?.stickyHeaderRowsUpdated({ - sizes: stickyCellHeights, - offsets: stickyOffsets, - elements: elementsToStick, - }); - } else { - this._positionListener?.stickyFooterRowsUpdated({ - sizes: stickyCellHeights, - offsets: stickyOffsets, - elements: elementsToStick, - }); - } - }); + if (position === 'top') { + this._positionListener?.stickyHeaderRowsUpdated({ + sizes: stickyCellHeights, + offsets: stickyOffsets, + elements: elementsToStick, + }); + } else { + this._positionListener?.stickyFooterRowsUpdated({ + sizes: stickyCellHeights, + offsets: stickyOffsets, + elements: elementsToStick, + }); + } + }, + }, + {injector: this._tableInjector}, + ); } /** @@ -286,17 +308,30 @@ export class StickyStyler { } // Coalesce with other sticky updates (and potentially other changes like column resize). - this._coalescedStyleScheduler.schedule(() => { - const tfoot = tableElement.querySelector('tfoot')!; - - if (tfoot) { - if (stickyStates.some(state => !state)) { - this._removeStickyStyle(tfoot, ['bottom']); - } else { - this._addStickyStyle(tfoot, 'bottom', 0, false); - } - } - }); + afterNextRender( + { + write: () => { + const tfoot = tableElement.querySelector('tfoot')!; + + if (tfoot) { + if (stickyStates.some(state => !state)) { + this._removeStickyStyle(tfoot, ['bottom']); + } else { + this._addStickyStyle(tfoot, 'bottom', 0, false); + } + } + }, + }, + {injector: this._tableInjector}, + ); + } + + destroy() { + if (this._stickyColumnsReplayTimeout) { + clearTimeout(this._stickyColumnsReplayTimeout); + } + + this._destroyed = true; } /** @@ -516,6 +551,10 @@ export class StickyStyler { } this._stickyColumnsReplayTimeout = setTimeout(() => { + if (this._destroyed) { + return; + } + for (const update of this._updatedStickyColumnsParamsToReplay) { this.updateStickyColumns( update.rows, diff --git a/src/cdk/table/table.ts b/src/cdk/table/table.ts index 56c416ab560d..be45abf8f0f7 100644 --- a/src/cdk/table/table.ts +++ b/src/cdk/table/table.ts @@ -48,7 +48,6 @@ import { ViewEncapsulation, booleanAttribute, inject, - afterNextRender, Injector, HostAttributeToken, } from '@angular/core'; @@ -654,6 +653,8 @@ export class CdkTable } ngOnDestroy() { + this._stickyStyler?.destroy(); + [ this._rowOutlet?.viewContainer, this._headerRowOutlet?.viewContainer, @@ -727,14 +728,8 @@ export class CdkTable this._updateNoDataRow(); - afterNextRender( - () => { - this.updateStickyColumnStyles(); - }, - {injector: this._injector}, - ); - this.contentChanged.next(); + this.updateStickyColumnStyles(); } /** Adds a column definition that was not included as part of the content children. */ @@ -1201,7 +1196,7 @@ export class CdkTable /** Adds the sticky column styles for the rows according to the columns' stick states. */ private _addStickyColumnStyles(rows: HTMLElement[], rowDef: BaseRowDef) { - const columnDefs = Array.from(rowDef.columns || []).map(columnName => { + const columnDefs = Array.from(rowDef?.columns || []).map(columnName => { const columnDef = this._columnDefsByName.get(columnName); if (!columnDef && (typeof ngDevMode === 'undefined' || ngDevMode)) { throw getTableUnknownColumnError(columnName); @@ -1392,10 +1387,10 @@ export class CdkTable this._isNativeHtmlTable, this.stickyCssClass, direction, - this._coalescedStyleScheduler, this._platform.isBrowser, this.needsPositionStickyOnElement, this._stickyPositioningListener, + this._injector, ); (this._dir ? this._dir.change : observableOf()) .pipe(takeUntil(this._onDestroy))