From 85457fef0e1e22f044742d4e84850fcc20fd771e Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Tue, 12 Nov 2024 08:50:28 -0600 Subject: [PATCH 1/6] Update `renderCell` tests --- test/browser/renderers.test.tsx | 50 ++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/test/browser/renderers.test.tsx b/test/browser/renderers.test.tsx index 071cd5aac4..355dd2f019 100644 --- a/test/browser/renderers.test.tsx +++ b/test/browser/renderers.test.tsx @@ -3,6 +3,7 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import DataGrid, { + Cell, DataGridDefaultRenderersProvider, renderSortIcon, SelectColumn @@ -39,19 +40,11 @@ const columns: readonly Column[] = [ ]; function globalCellRenderer(key: React.Key, props: CellRendererProps) { - return ( -
- {props.row[props.column.key as keyof Row]} -
- ); + return ; } -function localCellRenderer(key: React.Key) { - return ( -
- local -
- ); +function localCellRenderer(key: React.Key, props: CellRendererProps) { + return ; } function NoRowsFallback() { @@ -133,7 +126,7 @@ test('fallback defined using both provider and renderers with no rows', () => { test('fallback defined using renderers prop with a row', () => { setup({ columns, - rows: [{ id: 1, col1: 'col 1 value', col2: 'col 2 value' }], + rows: [{ id: 1, col1: 'value 1', col2: 'value 2' }], renderers: { noRowsFallback: } }); @@ -142,7 +135,7 @@ test('fallback defined using renderers prop with a row', () => { }); test('fallback defined using provider with a row', () => { - setupProvider({ columns, rows: [{ id: 1, col1: 'col 1 value', col2: 'col 2 value' }] }); + setupProvider({ columns, rows: [{ id: 1, col1: 'value 1', col2: 'value 2' }] }); expect(getRows()).toHaveLength(1); expect(screen.queryByText('Global no rows fallback')).not.toBeInTheDocument(); @@ -151,7 +144,7 @@ test('fallback defined using provider with a row', () => { test('fallback defined using both provider and renderers with a row', () => { setupProvider({ columns, - rows: [{ id: 1, col1: 'col 1 value', col2: 'col 2 value' }], + rows: [{ id: 1, col1: 'value 1', col2: 'value 2' }], renderers: { noRowsFallback: } }); @@ -215,22 +208,35 @@ test('sortPriority defined using both providers and renderers', async () => { }); test('renderCell defined using provider', () => { - setupProvider({ columns, rows: [{ id: 1, col1: 'col 1 value', col2: 'col 2 value' }] }); + setupProvider({ columns, rows: [{ id: 1, col1: 'value 1', col2: 'value 2' }] }); const [, cell1, cell2] = getCells(); - expect(cell1).toHaveTextContent('col 1 value'); - expect(cell2).toHaveTextContent('col 2 value'); + expect(cell1).toHaveTextContent('value 1'); + expect(cell1).toHaveClass('global'); + expect(cell1).not.toHaveClass('local'); + expect(cell1).toHaveStyle({ fontStyle: 'italic' }); + + expect(cell2).toHaveTextContent('value 2'); + expect(cell2).toHaveClass('global'); + expect(cell2).not.toHaveClass('local'); + expect(cell2).toHaveStyle({ fontStyle: 'italic' }); }); test('renderCell defined using both providers and renderers', () => { setupProvider({ columns, - rows: [{ id: 1, col1: 'col 1 value', col2: 'col 2 value' }], + rows: [{ id: 1, col1: 'value 1', col2: 'value 2' }], renderers: { renderCell: localCellRenderer } }); - const [selectCell, cell1, cell2] = getCells(); - expect(selectCell).toHaveTextContent('local'); - expect(cell1).toHaveTextContent('local'); - expect(cell2).toHaveTextContent('local'); + const [, cell1, cell2] = getCells(); + expect(cell1).toHaveTextContent('value 1'); + expect(cell1).toHaveClass('local'); + expect(cell1).not.toHaveClass('global'); + expect(cell1).toHaveStyle({ fontStyle: 'normal' }); + + expect(cell2).toHaveTextContent('value 2'); + expect(cell2).toHaveClass('local'); + expect(cell2).not.toHaveClass('global'); + expect(cell2).toHaveStyle({ fontStyle: 'normal' }); }); From 5e39950088fc9b203c8fb982c9efe1c4ebb2f44a Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Tue, 12 Nov 2024 09:08:39 -0600 Subject: [PATCH 2/6] Test `renderRow` --- test/browser/renderers.test.tsx | 39 +++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/test/browser/renderers.test.tsx b/test/browser/renderers.test.tsx index 355dd2f019..823a8aa621 100644 --- a/test/browser/renderers.test.tsx +++ b/test/browser/renderers.test.tsx @@ -5,6 +5,7 @@ import userEvent from '@testing-library/user-event'; import DataGrid, { Cell, DataGridDefaultRenderersProvider, + Row as DefaultRow, renderSortIcon, SelectColumn } from '../../src'; @@ -12,6 +13,7 @@ import type { CellRendererProps, Column, DataGridProps, + RenderRowProps, RenderSortStatusProps, SortColumn } from '../../src'; @@ -39,14 +41,22 @@ const columns: readonly Column[] = [ } ]; -function globalCellRenderer(key: React.Key, props: CellRendererProps) { +function globalRenderCell(key: React.Key, props: CellRendererProps) { return ; } -function localCellRenderer(key: React.Key, props: CellRendererProps) { +function localRenderCell(key: React.Key, props: CellRendererProps) { return ; } +function globalRenderRow(key: React.Key, props: RenderRowProps) { + return ; +} + +function localRenderRow(key: React.Key, props: RenderRowProps) { + return ; +} + function NoRowsFallback() { return
Local no rows fallback
; } @@ -94,7 +104,8 @@ function setupProvider(props: DataGridProps, renderCheckbox: globalRenderCheckbox, renderSortStatus: globalSortStatus, - renderCell: globalCellRenderer + renderCell: globalRenderCell, + renderRow: globalRenderRow }} > @@ -226,7 +237,7 @@ test('renderCell defined using both providers and renderers', () => { setupProvider({ columns, rows: [{ id: 1, col1: 'value 1', col2: 'value 2' }], - renderers: { renderCell: localCellRenderer } + renderers: { renderCell: localRenderCell } }); const [, cell1, cell2] = getCells(); @@ -240,3 +251,23 @@ test('renderCell defined using both providers and renderers', () => { expect(cell2).not.toHaveClass('global'); expect(cell2).toHaveStyle({ fontStyle: 'normal' }); }); + +test('renderRow defined using provider', () => { + setupProvider({ columns, rows: [{ id: 1, col1: 'value 1', col2: 'value 2' }] }); + + const [row] = getRows(); + expect(row).toHaveClass('global'); + expect(row).not.toHaveClass('local'); +}); + +test('renderRow defined using both providers and renderers', () => { + setupProvider({ + columns, + rows: [{ id: 1, col1: 'value 1', col2: 'value 2' }], + renderers: { renderRow: localRenderRow } + }); + + const [row] = getRows(); + expect(row).toHaveClass('local'); + expect(row).not.toHaveClass('global'); +}); From 8ffe50725dba6e83427257edb192a5d139f09d0f Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Tue, 12 Nov 2024 10:32:56 -0600 Subject: [PATCH 3/6] Use `flushSync` to close editor --- src/DataGrid.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/DataGrid.tsx b/src/DataGrid.tsx index 593ed97154..33982c5bb5 100644 --- a/src/DataGrid.tsx +++ b/src/DataGrid.tsx @@ -913,6 +913,11 @@ function DataGrid( ); } + function closeEditor(shouldFocusCell: boolean) { + setShouldFocusCell(shouldFocusCell); + setSelectedPosition(({ idx, rowIdx }) => ({ idx, rowIdx, mode: 'SELECT' })); + } + function getCellEditor(rowIdx: number) { if (selectedPosition.rowIdx !== rowIdx || selectedPosition.mode === 'SELECT') return; @@ -920,9 +925,10 @@ function DataGrid( const column = columns[idx]; const colSpan = getColSpan(column, lastFrozenColumnIndex, { type: 'ROW', row }); - const closeEditor = (shouldFocusCell: boolean) => { - setShouldFocusCell(shouldFocusCell); - setSelectedPosition(({ idx, rowIdx }) => ({ idx, rowIdx, mode: 'SELECT' })); + const onEditorClose = (shouldFocusCell: boolean) => { + flushSync(() => { + closeEditor(shouldFocusCell); + }); }; const onRowChange = (row: R, commitChanges: boolean, shouldFocusCell: boolean) => { @@ -953,7 +959,7 @@ function DataGrid( row={row} rowIdx={rowIdx} onRowChange={onRowChange} - closeEditor={closeEditor} + closeEditor={onEditorClose} onKeyDown={onCellKeyDown} navigate={navigate} /> From 8f3a3c6c01fedf037c5827584628dc97ebc33dd6 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Tue, 12 Nov 2024 10:57:43 -0600 Subject: [PATCH 4/6] Revert "Use `flushSync` to close editor" This reverts commit 8ffe50725dba6e83427257edb192a5d139f09d0f. --- src/DataGrid.tsx | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/DataGrid.tsx b/src/DataGrid.tsx index 33982c5bb5..593ed97154 100644 --- a/src/DataGrid.tsx +++ b/src/DataGrid.tsx @@ -913,11 +913,6 @@ function DataGrid( ); } - function closeEditor(shouldFocusCell: boolean) { - setShouldFocusCell(shouldFocusCell); - setSelectedPosition(({ idx, rowIdx }) => ({ idx, rowIdx, mode: 'SELECT' })); - } - function getCellEditor(rowIdx: number) { if (selectedPosition.rowIdx !== rowIdx || selectedPosition.mode === 'SELECT') return; @@ -925,10 +920,9 @@ function DataGrid( const column = columns[idx]; const colSpan = getColSpan(column, lastFrozenColumnIndex, { type: 'ROW', row }); - const onEditorClose = (shouldFocusCell: boolean) => { - flushSync(() => { - closeEditor(shouldFocusCell); - }); + const closeEditor = (shouldFocusCell: boolean) => { + setShouldFocusCell(shouldFocusCell); + setSelectedPosition(({ idx, rowIdx }) => ({ idx, rowIdx, mode: 'SELECT' })); }; const onRowChange = (row: R, commitChanges: boolean, shouldFocusCell: boolean) => { @@ -959,7 +953,7 @@ function DataGrid( row={row} rowIdx={rowIdx} onRowChange={onRowChange} - closeEditor={onEditorClose} + closeEditor={closeEditor} onKeyDown={onCellKeyDown} navigate={navigate} /> From 88bd5e372a64c4967950017e85dd8bc55a883805 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 13 Nov 2024 12:31:42 -0600 Subject: [PATCH 5/6] Set `screenshotFailures` to false in CI --- vitest.workspace.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vitest.workspace.ts b/vitest.workspace.ts index bf43e7a3cd..7c3a30f323 100644 --- a/vitest.workspace.ts +++ b/vitest.workspace.ts @@ -27,7 +27,8 @@ export default defineWorkspace([ provider: 'playwright', commands: { resizeColumn }, viewport: { width: 1920, height: 1080 }, - headless: true + headless: true, + screenshotFailures: process.env.CI !== 'true' } } }, From dbc0be3cf73d25860466ea0957ab5264c2da9e5e Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Fri, 15 Nov 2024 10:58:09 -0600 Subject: [PATCH 6/6] Rename --- test/browser/renderers.test.tsx | 38 ++++++++++++++++----------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/test/browser/renderers.test.tsx b/test/browser/renderers.test.tsx index 823a8aa621..faec77f48e 100644 --- a/test/browser/renderers.test.tsx +++ b/test/browser/renderers.test.tsx @@ -41,19 +41,19 @@ const columns: readonly Column[] = [ } ]; -function globalRenderCell(key: React.Key, props: CellRendererProps) { +function renderGlobalCell(key: React.Key, props: CellRendererProps) { return ; } -function localRenderCell(key: React.Key, props: CellRendererProps) { +function renderLocalCell(key: React.Key, props: CellRendererProps) { return ; } -function globalRenderRow(key: React.Key, props: RenderRowProps) { +function renderGlobalRow(key: React.Key, props: RenderRowProps) { return ; } -function localRenderRow(key: React.Key, props: RenderRowProps) { +function renderLocalRow(key: React.Key, props: RenderRowProps) { return ; } @@ -61,19 +61,19 @@ function NoRowsFallback() { return
Local no rows fallback
; } -function GlobalNoRowsFallback() { +function NoRowsGlobalFallback() { return
Global no rows fallback
; } -function localRenderCheckbox() { +function renderLocalCheckbox() { return
Local checkbox
; } -function globalRenderCheckbox() { +function renderGlobalCheckbox() { return
Global checkbox
; } -function globalSortStatus({ sortDirection, priority }: RenderSortStatusProps) { +function renderGlobalSortStatus({ sortDirection, priority }: RenderSortStatusProps) { return ( <> {renderSortIcon({ sortDirection })} @@ -82,7 +82,7 @@ function globalSortStatus({ sortDirection, priority }: RenderSortStatusProps) { ); } -function renderSortStatus({ sortDirection, priority }: RenderSortStatusProps) { +function renderLocalSortStatus({ sortDirection, priority }: RenderSortStatusProps) { return ( <> {renderSortIcon({ sortDirection })} @@ -101,11 +101,11 @@ function setupProvider(props: DataGridProps, - renderCheckbox: globalRenderCheckbox, - renderSortStatus: globalSortStatus, - renderCell: globalRenderCell, - renderRow: globalRenderRow + noRowsFallback: , + renderCheckbox: renderGlobalCheckbox, + renderSortStatus: renderGlobalSortStatus, + renderCell: renderGlobalCell, + renderRow: renderGlobalRow }} > @@ -165,7 +165,7 @@ test('fallback defined using both provider and renderers with a row', () => { }); test('checkbox defined using renderers prop', () => { - setup({ columns, rows: noRows, renderers: { renderCheckbox: localRenderCheckbox } }); + setup({ columns, rows: noRows, renderers: { renderCheckbox: renderLocalCheckbox } }); expect(getRows()).toHaveLength(0); expect(screen.getByText('Local checkbox')).toBeInTheDocument(); @@ -179,7 +179,7 @@ test('checkbox defined using provider', () => { }); test('checkbox defined using both provider and renderers', () => { - setupProvider({ columns, rows: noRows, renderers: { renderCheckbox: localRenderCheckbox } }); + setupProvider({ columns, rows: noRows, renderers: { renderCheckbox: renderLocalCheckbox } }); expect(getRows()).toHaveLength(0); expect(screen.getByText('Local checkbox')).toBeInTheDocument(); @@ -203,7 +203,7 @@ test('sortPriority defined using both providers', async () => { }); test('sortPriority defined using both providers and renderers', async () => { - setupProvider({ columns, rows: noRows, renderers: { renderSortStatus } }); + setupProvider({ columns, rows: noRows, renderers: { renderSortStatus: renderLocalSortStatus } }); const [, headerCell2, headerCell3] = getHeaderCells(); const user = userEvent.setup(); @@ -237,7 +237,7 @@ test('renderCell defined using both providers and renderers', () => { setupProvider({ columns, rows: [{ id: 1, col1: 'value 1', col2: 'value 2' }], - renderers: { renderCell: localRenderCell } + renderers: { renderCell: renderLocalCell } }); const [, cell1, cell2] = getCells(); @@ -264,7 +264,7 @@ test('renderRow defined using both providers and renderers', () => { setupProvider({ columns, rows: [{ id: 1, col1: 'value 1', col2: 'value 2' }], - renderers: { renderRow: localRenderRow } + renderers: { renderRow: renderLocalRow } }); const [row] = getRows();