From e854e3c63ea2089fa7baa8b9d6fe9f1f6161fad9 Mon Sep 17 00:00:00 2001 From: Lukas Harbarth Date: Fri, 13 Nov 2026 12:37:33 +0100 Subject: [PATCH] refactor(AnalyticalTable): drop useAnimationFrameWithResizeObserver after react-virtual update (v3.14.26) Remove `useAnimationFrameWithResizeObserver: true` from the row virtualizer: virtual-core 3.19.9 fixes the ResizeObserver loop natively (defers observing newly mounted rows while measuring them synchronously), making the flag redundant; dropping it restores synchronous row measurement. Add a Playwright RO-loop regression test (variable-height subcomponents + scroll). --- packages/main/package.json | 2 +- .../AnalyticalTable/AnalyticalTable.cy.tsx | 51 -------- .../src/components/AnalyticalTable/index.tsx | 1 - .../test/AnalyticalTable.gallery.tsx | 93 ++++++++++++++ .../test/AnalyticalTable.spec.tsx | 118 +++++++++++++++++- yarn.lock | 20 +-- 6 files changed, 220 insertions(+), 65 deletions(-) diff --git a/packages/main/package.json b/packages/main/package.json index 2bb9f157e79..0f737734391 100644 --- a/packages/main/package.json +++ b/packages/main/package.json @@ -864,7 +864,7 @@ "watch:css": "yarn build:css --watch" }, "dependencies": { - "@tanstack/react-virtual": "3.14.23", + "@tanstack/react-virtual": "3.14.26", "@ui5/webcomponents-react-base": "workspace:~", "clsx": "2.1.1" }, diff --git a/packages/main/src/components/AnalyticalTable/AnalyticalTable.cy.tsx b/packages/main/src/components/AnalyticalTable/AnalyticalTable.cy.tsx index 643a953d98b..89fe3008b13 100644 --- a/packages/main/src/components/AnalyticalTable/AnalyticalTable.cy.tsx +++ b/packages/main/src/components/AnalyticalTable/AnalyticalTable.cy.tsx @@ -1849,57 +1849,6 @@ describe('AnalyticalTable', () => { }); }); - it('first virtual row offset matches scrollTop after loading cycle', () => { - // Guards the layout-effect in `AnalyticalTable/index.tsx` that re-syncs the virtualizer's cached `scrollOffset` with the DOM after a data swap clamps `scrollTop` (dispatching a scroll event). - // Without it, the first row renders at a stale `translateY` and leaves a whitespace gap at the top. - // `minRows={20}` + a filter result smaller than `minRows` keeps `itemCount = Math.max(minRows, rows.length, ...)` constant across the empty-then-refilled cycle, so the effect can only re-fire via the `rows.length` dep — guarding both deps in one shot. - const filterData = new Array(500).fill('').map((_, index) => ({ name: `Row-${index}`, age: index })); - const TestComp = () => { - const [tableData, setTableData] = useState(filterData); - const [loading, setLoading] = useState(false); - const reactTableOptions = useMemo(() => ({ manualFilters: true }), []); - const triggerFilter = () => { - setTableData([]); - setLoading(true); - setTimeout(() => { - setTableData(filterData.filter((item) => item.age < 5)); - setLoading(false); - }, 100); - }; - return ( - <> - - - - ); - }; - cy.mount(); - cy.get('[data-component-name="AnalyticalTableBody"]').as('body'); - cy.get('@body').scrollTo(0, 4000); - cy.findByTestId('filter').click(); - // `.should(callback)` retries until both assertions pass — implicitly waits for the loading cycle to finish and rows to be rendered. - cy.get('@body').should(($body) => { - const scrollTop = $body[0].scrollTop; - const scrollContainer = $body[0].querySelector('[data-component-name="AnalyticalTableBodyScrollableContainer"]'); - const firstRow = scrollContainer?.children?.[0] as HTMLElement | undefined; - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - expect(firstRow, 'first body row should exist').to.exist; - const match = firstRow.style.transform?.match(/translateY\(([\d.]+)px\)/); - const translateY = match ? parseFloat(match[1]) : 0; - // Bounded by the overscan window when in sync; far off (~3000+px) when stale. - expect(Math.abs(translateY - scrollTop), 'first row translateY should be close to scrollTop').to.be.lessThan(500); - }); - }); - it('InfiniteScroll', () => { const data = new Array(500).fill('').map((_, index) => ({ name: `Name${index}` })); const TestComp = (props: Omit) => { diff --git a/packages/main/src/components/AnalyticalTable/index.tsx b/packages/main/src/components/AnalyticalTable/index.tsx index 9188d8b48e4..9e9c21dddae 100644 --- a/packages/main/src/components/AnalyticalTable/index.tsx +++ b/packages/main/src/components/AnalyticalTable/index.tsx @@ -811,7 +811,6 @@ const AnalyticalTable = forwardRef ( ); + +// Virtualized, scrollable table with variable-height row subcomponents — drives the #6923 RO-loop regression spec. +const subComponentData = Array.from({ length: 200 }, (_, i) => ({ + name: `Name-${i}`, + age: i, + friend: { name: `Friend-${i}`, age: i + 100 }, +})); + +const subComponentColumns: AnalyticalTableColumnDefinition[] = [ + { Header: 'Name', accessor: 'name' }, + { Header: 'Age', accessor: 'age' }, + { Header: 'Friend Name', accessor: 'friend.name' }, +]; + +export const SubComponentResizeHarness = ({ + subComponentsBehavior = AnalyticalTableSubComponentsBehavior.Visible, +}: { + subComponentsBehavior?: AnalyticalTablePropTypes['subComponentsBehavior']; +}) => { + const renderRowSubComponent = useCallback((row) => { + // Vary heights so re-measurement moves rows; one row has no subcomponent. + if (row.index % 7 === 2) { + return null; + } + const height = row.index % 3 === 0 ? 240 : row.index % 3 === 1 ? 120 : 60; + return ( +
+ {`subcomponent ${row.id} — ${height}px`} +
+ ); + }, []); + return ( + + ); +}; + +// Scrolled near the end, then an async filter empties and refills with fewer rows than minRows: the DOM clamps +// scrollTop with no scroll event, and minRows (20) > result keeps itemCount constant so the re-sync effect can only +// re-fire via its rows.length dep. Drives the scrollOffset regression test (origin PR #8631). +export const ScrollOffsetAfterFilterHarness = () => { + const full = useMemo(() => Array.from({ length: 500 }, (_, i) => ({ name: `Row-${i}`, age: i })), []); + const [tableData, setTableData] = useState(full); + const [loading, setLoading] = useState(false); + const columns = useMemo( + () => [ + { Header: 'Name', accessor: 'name' }, + { Header: 'Age', accessor: 'age' }, + ], + [], + ); + const reactTableOptions = useMemo(() => ({ manualFilters: true }), []); + const onTableScroll = useCallback(() => { + const w = window as unknown as { __tableScrollCount?: number }; + w.__tableScrollCount = (w.__tableScrollCount ?? 0) + 1; + }, []); + return ( + <> + + + + ); +}; diff --git a/packages/main/src/components/AnalyticalTable/test/AnalyticalTable.spec.tsx b/packages/main/src/components/AnalyticalTable/test/AnalyticalTable.spec.tsx index f3f50b2fe59..4e31d031a1e 100644 --- a/packages/main/src/components/AnalyticalTable/test/AnalyticalTable.spec.tsx +++ b/packages/main/src/components/AnalyticalTable/test/AnalyticalTable.spec.tsx @@ -1,9 +1,14 @@ -import type { Locator, Page } from '@playwright/test'; +import type { ConsoleMessage, Locator, Page } from '@playwright/test'; import { expect, test } from '../../../../../../playwright/fixtures/gallery-fixtures.js'; import { AnalyticalTableScaleWidthMode } from '../../../enums/AnalyticalTableScaleWidthMode.js'; import { AnalyticalTableSelectionMode } from '../../../enums/AnalyticalTableSelectionMode.js'; +import { AnalyticalTableSubComponentsBehavior } from '../../../enums/AnalyticalTableSubComponentsBehavior.js'; import type { AnalyticalTableColumnDefinition } from '../index.js'; -import type { StickyHarness } from './AnalyticalTable.gallery.js'; +import type { + ScrollOffsetAfterFilterHarness, + StickyHarness, + SubComponentResizeHarness, +} from './AnalyticalTable.gallery.js'; // 5×150 = 750px total; container 500px → guaranteed horizontal overflow. const wideCols: AnalyticalTableColumnDefinition[] = [ @@ -496,3 +501,112 @@ test.describe('AnalyticalTable Smart scaleWidthMode — shrink to fit', () => { expect(await boxWidth(columnHeader(page, 'friend.name'))).toBeGreaterThanOrEqual(100); }); }); + +test.describe('AnalyticalTable — scroll offset', () => { + // Guards the scrollOffset re-sync effect (index.tsx) after a filter clamps scrollTop. onTableScroll must fire: + // on current virtual-core the native range-clamp already fixes the render, so the translateY check alone no longer + // detects removal — keep both. translateY vs scrollTop: <500 is within the overscan window, stale is ≈3000px. + test('re-syncs scroll offset after an async filter shrinks the rows', async ({ mount, page }) => { + await mount('AnalyticalTable/ScrollOffsetAfterFilterHarness', {}); + const body = page.locator('[data-component-name="AnalyticalTableBody"]'); + await expect(body).toBeVisible(); + + await body.evaluate((el) => el.scrollTo(0, 4000)); + await expect.poll(() => body.evaluate((el) => el.scrollTop)).toBeGreaterThan(0); + await page.waitForTimeout(250); + await page.evaluate(() => ((window as unknown as { __tableScrollCount: number }).__tableScrollCount = 0)); + + await page.getByTestId('filter').click(); + + await expect + .poll(() => page.evaluate(() => (window as unknown as { __tableScrollCount?: number }).__tableScrollCount ?? 0)) + .toBeGreaterThan(0); + + await expect + .poll(() => + body.evaluate((el) => { + const container = el.querySelector('[data-component-name="AnalyticalTableBodyScrollableContainer"]'); + const firstRow = container?.children?.[0] as HTMLElement | undefined; + if (!firstRow) { + return Number.POSITIVE_INFINITY; + } + const match = firstRow.style.transform?.match(/translateY\(([\d.]+)px\)/); + const translateY = match ? parseFloat(match[1]) : 0; + return Math.abs(translateY - el.scrollTop); + }), + ) + .toBeLessThan(500); + }); +}); + +const RO_LOOP = /ResizeObserver loop/i; + +// Collect RO-loop reports from every channel: the window `error` event (where Chromium dispatches it), console, and pageerror. +async function trackResizeObserverLoop(page: Page): Promise<() => Promise> { + const fromListeners: string[] = []; + page.on('console', (msg: ConsoleMessage) => { + if (RO_LOOP.test(msg.text())) { + fromListeners.push(msg.text()); + } + }); + page.on('pageerror', (err) => { + if (RO_LOOP.test(err.message)) { + fromListeners.push(err.message); + } + }); + await page.addInitScript(() => { + (window as unknown as { __roErrors: string[] }).__roErrors = []; + window.addEventListener('error', (e: ErrorEvent) => { + if (/ResizeObserver loop/i.test(e?.message ?? '')) { + (window as unknown as { __roErrors: string[] }).__roErrors.push(e.message); + } + }); + }); + return async () => { + const fromWindow = await page.evaluate(() => (window as unknown as { __roErrors?: string[] }).__roErrors ?? []); + return [...fromWindow, ...fromListeners]; + }; +} + +// Scroll so virtualized rows and their subcomponents mount/unmount while being measured. +async function scrollStress(page: Page) { + const body = page.locator('[data-component-name="AnalyticalTableBody"]'); + await expect(body).toBeVisible(); + for (const offset of [600, 1800, 400, 2400, 0, 1200]) { + await body.evaluate((el, top) => el.scrollTo(0, top), offset); + await page.waitForTimeout(80); + } +} + +// Regression guard for #6923: dynamic subcomponent heights must not trigger a ResizeObserver loop during scroll. +test.describe('AnalyticalTable subcomponents — ResizeObserver loop', () => { + test('no RO loop while scrolling with always-visible variable-height subcomponents', async ({ mount, page }) => { + const getErrors = await trackResizeObserverLoop(page); + await mount('AnalyticalTable/SubComponentResizeHarness', { + subComponentsBehavior: AnalyticalTableSubComponentsBehavior.Visible, + }); + await expect(page.getByRole('grid')).toBeVisible(); + await expect(page.getByTestId('subcomponent-0')).toBeVisible(); + + await scrollStress(page); + + expect(await getErrors()).toEqual([]); + }); + + test('no RO loop while expanding/collapsing subcomponents during scroll', async ({ mount, page }) => { + const getErrors = await trackResizeObserverLoop(page); + await mount('AnalyticalTable/SubComponentResizeHarness', { + subComponentsBehavior: AnalyticalTableSubComponentsBehavior.IncludeHeightExpandable, + }); + await expect(page.getByRole('grid')).toBeVisible(); + + const expander = () => page.locator('[title="Expand Node"]').first().locator('[ui5-button]'); + for (let i = 0; i < 4; i++) { + await expander().click(); + await scrollStress(page); + await expander().click(); + } + + expect(await getErrors()).toEqual([]); + }); +}); diff --git a/yarn.lock b/yarn.lock index f9f47391be1..0b610c2e983 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4408,22 +4408,22 @@ __metadata: languageName: node linkType: hard -"@tanstack/react-virtual@npm:3.14.23": - version: 3.14.23 - resolution: "@tanstack/react-virtual@npm:3.14.23" +"@tanstack/react-virtual@npm:3.14.26": + version: 3.14.26 + resolution: "@tanstack/react-virtual@npm:3.14.26" dependencies: - "@tanstack/virtual-core": "npm:3.19.8" + "@tanstack/virtual-core": "npm:3.19.11" peerDependencies: react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 react-dom: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 - checksum: 10c0/cd39e337ba5d450be5cade885dd0a043ef9149abd95331c533b588a626573af4a48b0d4e78d2bcf36b4ffa1267921dfae7203dec8aa8aa7f128014accd2e4ff5 + checksum: 10c0/71291177ac4c4b067a40351d46c69af0d5fc9dfcc6cb328cb87717b29209501a7193e92e75e599273ee423fff6c55c88157bd238684846b0d926dfa73b5d6dac languageName: node linkType: hard -"@tanstack/virtual-core@npm:3.19.8": - version: 3.19.8 - resolution: "@tanstack/virtual-core@npm:3.19.8" - checksum: 10c0/eac7a1edd843d8c0f1b07343a7804ac393fb152cb046581331c1c912b7dc518f484c5bca69cae2637739f447cffc86f779b3f4ca7b227872ed7cd5dc8fe5d28b +"@tanstack/virtual-core@npm:3.19.11": + version: 3.19.11 + resolution: "@tanstack/virtual-core@npm:3.19.11" + checksum: 10c0/61558882fd09f64020104ecd94bacaa2a6ccbb0745247610ede7683d709cf8d134cc3f6d1ee2d22203cb881e531bc4a3bb92e4657c08edf923ab6d5c39a958ec languageName: node linkType: hard @@ -5786,7 +5786,7 @@ __metadata: version: 0.0.0-use.local resolution: "@ui5/webcomponents-react@workspace:packages/main" dependencies: - "@tanstack/react-virtual": "npm:3.14.23" + "@tanstack/react-virtual": "npm:3.14.26" "@ui5/webcomponents-react-base": "workspace:~" clsx: "npm:2.1.1" peerDependencies: