diff --git a/.changeset/giant-mayflies-wash.md b/.changeset/giant-mayflies-wash.md new file mode 100644 index 00000000000..b1f8e5e57ff --- /dev/null +++ b/.changeset/giant-mayflies-wash.md @@ -0,0 +1,5 @@ +--- +'@itwin/itwinui-react': patch +--- + +Fixed `scrollToRow` in un-virtualized `Table`. In virtualized `Table`, `scrollToRow` now scrolls to the top for consistent behavior. diff --git a/apps/react-workshop/cypress-visual-screenshots/baseline/Table.test.ts-Scroll To Row.png b/apps/react-workshop/cypress-visual-screenshots/baseline/Table.test.ts-Scroll To Row.png new file mode 100755 index 00000000000..26f0dd05943 Binary files /dev/null and b/apps/react-workshop/cypress-visual-screenshots/baseline/Table.test.ts-Scroll To Row.png differ diff --git a/apps/react-workshop/src/Table.test.ts b/apps/react-workshop/src/Table.test.ts index 38b68f2f856..135c82a64c9 100644 --- a/apps/react-workshop/src/Table.test.ts +++ b/apps/react-workshop/src/Table.test.ts @@ -26,6 +26,7 @@ describe('Table', () => { 'Localized', 'No Data', 'Resizable Columns', + 'Scroll To Row', 'Selectable Multi', 'Selectable Single', 'Sortable', @@ -81,6 +82,9 @@ describe('Table', () => { cy.get('button').first().click({ force: true }); // force because the button is hidden break; } + case 'Scroll To Row': { + cy.wait(100); + } } }); diff --git a/packages/itwinui-css/src/table/base.scss b/packages/itwinui-css/src/table/base.scss index 27578785a76..12b8cdb7455 100644 --- a/packages/itwinui-css/src/table/base.scss +++ b/packages/itwinui-css/src/table/base.scss @@ -152,6 +152,9 @@ min-inline-size: 100%; display: flex; + // To prevent the row from being overlapped by the tallest header when scrollToRow is used. + scroll-margin-block-start: calc(var(--_iui-table-header-size) + var(--iui-size-xl)); + .iui-table-header & { flex-grow: 1; } diff --git a/packages/itwinui-react/src/core/Table/Table.test.tsx b/packages/itwinui-react/src/core/Table/Table.test.tsx index 2f868a5c979..ef0c12c2f65 100644 --- a/packages/itwinui-react/src/core/Table/Table.test.tsx +++ b/packages/itwinui-react/src/core/Table/Table.test.tsx @@ -3291,28 +3291,6 @@ it('should render selectable rows without select column', async () => { expect(onRowClick).toHaveBeenCalledTimes(3); }); -it('should scroll to selected item in non-virtualized table', async () => { - let scrolledElement: HTMLElement | null = null; - vi.spyOn(HTMLElement.prototype, 'scrollIntoView').mockImplementation( - function (this: HTMLElement) { - // eslint-disable-next-line @typescript-eslint/no-this-alias - scrolledElement = this; - }, - ); - - const data = mockedData(50); - renderComponent({ - data, - scrollToRow: (rows) => rows.findIndex((row) => row.original === data[25]), - }); - - expect(scrolledElement).toBeTruthy(); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - expect(scrolledElement!.querySelector('.iui-table-cell')?.textContent).toBe( - data[25].name, - ); -}); - it('should render sticky columns correctly', () => { vi.spyOn(HTMLDivElement.prototype, 'scrollWidth', 'get').mockReturnValue(900); vi.spyOn(HTMLDivElement.prototype, 'clientWidth', 'get').mockReturnValue(500); diff --git a/packages/itwinui-react/src/core/Table/Table.tsx b/packages/itwinui-react/src/core/Table/Table.tsx index f3f2ae3c75c..f34e64b0ddd 100644 --- a/packages/itwinui-react/src/core/Table/Table.tsx +++ b/packages/itwinui-react/src/core/Table/Table.tsx @@ -824,7 +824,7 @@ export const Table = < useLayoutEffect(() => { if (scrollToIndex) { - virtualizer.scrollToIndex(scrollToIndex, { align: 'center' }); + virtualizer.scrollToIndex(scrollToIndex, { align: 'start' }); } }, [virtualizer, scrollToIndex]); diff --git a/testing/e2e/app/routes/Table/route.tsx b/testing/e2e/app/routes/Table/route.tsx index e3b1a7a0848..92f1781d402 100644 --- a/testing/e2e/app/routes/Table/route.tsx +++ b/testing/e2e/app/routes/Table/route.tsx @@ -61,7 +61,7 @@ function EverythingElse() { const maxWidths = searchParams.getAll('maxWidth'); const minWidths = searchParams.getAll('minWidth'); const isSelectable = searchParams.get('isSelectable') === 'true'; - const subRows = searchParams.get('subRows') === 'true'; + const rows = searchParams.get('rows'); const filter = searchParams.get('filter') === 'true'; const selectSubRows = !(searchParams.get('selectSubRows') === 'false'); const enableVirtualization = searchParams.get('virtualization') === 'true'; @@ -88,7 +88,16 @@ function EverythingElse() { return arr; }, [oneRow, empty]); - const data = subRows ? dataWithSubrows : baseData; + const data = (() => { + switch (rows) { + case 'subRows': + return dataWithSubrows; + case 'large': + return largeData; + default: + return baseData; + } + })(); const isRowDisabled = React.useCallback( (rowData: Record) => { @@ -136,7 +145,7 @@ function EverythingElse() { columnResizeMode={columnResizeMode as 'fit' | 'expand' | undefined} selectSubRows={selectSubRows} enableVirtualization={enableVirtualization} - style={enableVirtualization ? { maxHeight: '90vh' } : undefined} + style={{ maxHeight: '90vh' }} scrollToRow={ scroll ? (rows, data) => @@ -184,6 +193,13 @@ const baseData = [ }, ]; +const largeData = new Array(100).fill(0).map((_, i) => ({ + index: i, + name: `Name${i}`, + description: `Description${i}`, + id: i, +})); + const dataWithSubrows = [ { index: 1, diff --git a/testing/e2e/app/routes/Table/spec.ts b/testing/e2e/app/routes/Table/spec.ts index ed4ec53ae90..1c36083175e 100644 --- a/testing/e2e/app/routes/Table/spec.ts +++ b/testing/e2e/app/routes/Table/spec.ts @@ -1,5 +1,15 @@ import { test, expect, type Page } from '@playwright/test'; +test('un-virtualized table should scroll to provided row', async ({ page }) => { + await page.goto('/Table?rows=large&scroll=true&scrollRow=50'); + + await expect(page.getByText('Name47')).not.toBeInViewport(); + await expect(page.getByText('Name48')).toBeInViewport(); + await expect(page.getByText('Name49')).toBeInViewport(); + await expect(page.getByText('Name50')).toBeInViewport(); + await expect(page.getByText('Name51')).toBeInViewport(); +}); + test.describe('Table sorting', () => { test('should work with keyboard', async ({ page }) => { await page.goto('/Table'); @@ -156,7 +166,7 @@ test.describe('Table resizing', () => { test('should resize correctly when column width is set lower than default min width', async ({ page, }) => { - await page.goto('/Table?subRows=true'); + await page.goto('/Table?rows=subRows'); // resize first column { @@ -227,7 +237,7 @@ test.describe('Table row selection', () => { test('Should select all sub rows when parent row is selected', async ({ page, }) => { - await page.goto('/Table?isSelectable=true&subRows=true'); + await page.goto('/Table?isSelectable=true&rows=subRows'); const row2 = page.getByText('2Name2Description2'); const row2SubRowExpander = row2.getByLabel('Toggle sub row'); @@ -253,7 +263,7 @@ test.describe('Table row selection', () => { test('Should show indeterminate when sub rows are disabled', async ({ page, }) => { - await page.goto('/Table?isSelectable=true&subRows=true'); + await page.goto('/Table?isSelectable=true&rows=subRows'); const row3 = page.getByText('3Name3Description3'); const row3SubRowExpander = row3.getByLabel('Toggle sub row'); @@ -275,7 +285,7 @@ test.describe('Table row selection', () => { test('Should show indeterminate when some sub rows are filtered out', async ({ page, }) => { - await page.goto('/Table?isSelectable=true&subRows=true&filter=true'); + await page.goto('/Table?isSelectable=true&rows=subRows&filter=true'); await filter(page); const row2 = page.getByText('2Name2Description2'); @@ -318,7 +328,7 @@ test.describe('Table row selection', () => { test('parent checkbox state should become indeterminate when some filtered sub rows are deselected', async ({ page, }) => { - await page.goto('/Table?isSelectable=true&subRows=true&filter=true'); + await page.goto('/Table?isSelectable=true&rows=subRows&filter=true'); //expand subRows const row2 = page.getByText('2Name2Description2'); @@ -354,7 +364,7 @@ test.describe('Table row selection', () => { page, }) => { await page.goto( - '/Table?isSelectable=true&subRows=true&selectSubRows=false', + '/Table?isSelectable=true&rows=subRows&selectSubRows=false', ); const row2 = page @@ -384,7 +394,7 @@ test.describe('Table row selection', () => { page, }) => { await page.goto( - '/Table?isSelectable=true&subRows=true&selectSubRows=false&stateReducer=true', + '/Table?isSelectable=true&rows=subRows&selectSubRows=false&stateReducer=true', ); const row2 = page @@ -485,10 +495,10 @@ test.describe('Virtual Scroll Tests', () => { const rows = page.getByRole('rowgroup').getByRole('row'); const row50NameCell = page.getByText('Name50'); - expect((await rows.all()).length).toBe(14); - await expect(rows.nth(0)).toContainText('Name43'); - await expect(rows.nth(7)).toContainText('Name50'); - await expect(rows.nth(13)).toContainText('Name56'); + expect((await rows.all()).length).toBe(13); + await expect(rows.nth(0)).toContainText('Name49'); + await expect(rows.nth(1)).toContainText('Name50'); + await expect(rows.nth(12)).toContainText('Name61'); await expect(row50NameCell).toBeInViewport(); });