diff --git a/src/components/DataTable/DataTableControls.tsx b/src/components/DataTable/DataTableControls.tsx index 0e48ced..3314664 100644 --- a/src/components/DataTable/DataTableControls.tsx +++ b/src/components/DataTable/DataTableControls.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; import type { RowData, Table } from '@tanstack/table-core'; @@ -22,14 +22,22 @@ export const DataTableControls = ({ const { t } = useTranslation(); + // Held in a ref so that the effect below responds to the search value alone. Call sites normally + // write `onSearchChange` inline, so it is a new function on every render; keying the effect on it + // replayed the handler on every render of the consumer, and a handler that writes filter state + // then reset the table's page via Tanstack's `autoResetPageIndex`. + const onSearchChangeRef = useRef(onSearchChange); + onSearchChangeRef.current = onSearchChange; + useEffect(() => { - if (onSearchChange) { + const handleSearchChange = onSearchChangeRef.current; + if (handleSearchChange) { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument - onSearchChange(searchValue, table); + handleSearchChange(searchValue, table); } else { setGlobalFilter(searchValue || undefined); } - }, [onSearchChange, searchValue]); + }, [searchValue]); return (
diff --git a/src/components/DataTable/__tests__/DataTable.spec.tsx b/src/components/DataTable/__tests__/DataTable.spec.tsx index befd703..0e16373 100644 --- a/src/components/DataTable/__tests__/DataTable.spec.tsx +++ b/src/components/DataTable/__tests__/DataTable.spec.tsx @@ -1,3 +1,5 @@ +import { useState } from 'react'; + import { faker } from '@faker-js/faker'; import type { ColumnDef } from '@tanstack/table-core'; import { fireEvent, render, screen, waitFor } from '@testing-library/react'; @@ -6,6 +8,8 @@ import { describe, expect, it } from 'vitest'; import { DataTable } from '../DataTable.tsx'; +import type { DataTableSearchChangeHandler } from '../types.ts'; + type PaymentStatus = 'failed' | 'pending' | 'processing' | 'success'; type Payment = { @@ -66,3 +70,85 @@ describe('DataTable', () => { await waitFor(() => expect(screen.getAllByTestId('data-table-row').length).toBe(10)); }); }); + +describe('DataTable pagination', () => { + // Deterministic, so a row can be identified by the page it belongs to + const paginatedData: Payment[] = range(25).map((i) => ({ + amount: i, + email: `user-${i + 1}@example.com`, + id: String(i + 1), + status: 'pending' + })); + + const firstRow = () => screen.getAllByTestId('data-table-row')[0]!; + + const goToSecondPage = async () => { + fireEvent.click(screen.getByRole('button', { name: '2' })); + await waitFor(() => expect(firstRow()).toHaveTextContent('user-11@example.com')); + }; + + /** + * Records the clicked row, the way a consumer would in order to highlight it. The point is that + * this is an ordinary `setState` in the component that owns the table: it re-renders the + * consumer, but changes nothing about the table's contents. + */ + const ClickToSelect = ({ onSearchChange }: { onSearchChange?: DataTableSearchChangeHandler }) => { + const [clickedRowId, setClickedRowId] = useState(null); + return ( +
+ {clickedRowId ?? 'none'} + setClickedRowId(payment.id)} + onSearchChange={onSearchChange} + /> +
+ ); + }; + + it('should stay on the current page when a row click re-renders the consumer', async () => { + render(); + await goToSecondPage(); + + fireEvent.click(firstRow()); + await waitFor(() => expect(screen.getByTestId('clicked-row-id')).toHaveTextContent('11')); + + expect(firstRow()).toHaveTextContent('user-11@example.com'); + }); + + // `DataTableControls` runs `onSearchChange` from an effect keyed on the handler itself, so an + // inline handler -- the normal way to write one -- re-runs it on every consumer render, not only + // when the search value changes. A handler that writes filter state then hands Tanstack a new + // `columnFilters` identity, whose `autoResetPageIndex` returns the table to the first page. + it('should stay on the current page when an inline onSearchChange writes filter state', async () => { + render( + { + table.getColumn('status')!.setFilterValue(() => [...statuses]); + }} + /> + ); + await goToSecondPage(); + + fireEvent.click(firstRow()); + await waitFor(() => expect(screen.getByTestId('clicked-row-id')).toHaveTextContent('11')); + + expect(firstRow()).toHaveTextContent('user-11@example.com'); + }); + + // Same reset, seen from the search box: the filter is dropped from the table state while the + // search input keeps its text, so the controls end up contradicting the rows + it('should keep the search applied when a row click re-renders the consumer', async () => { + render(); + const searchBar = screen.getByTestId('data-table-search-bar').querySelector('input')!; + fireEvent.change(searchBar, { target: { value: 'user-11@example.com' } }); + await waitFor(() => expect(screen.getAllByTestId('data-table-row').length).toBe(1)); + + fireEvent.click(firstRow()); + await waitFor(() => expect(screen.getByTestId('clicked-row-id')).toHaveTextContent('11')); + + expect(searchBar).toHaveValue('user-11@example.com'); + expect(screen.getAllByTestId('data-table-row').length).toBe(1); + }); +}); diff --git a/src/components/DataTable/store.ts b/src/components/DataTable/store.ts index de4cd66..baa5b36 100644 --- a/src/components/DataTable/store.ts +++ b/src/components/DataTable/store.ts @@ -13,6 +13,7 @@ import { applyUpdater, calculateColumnSizing, defineMemoizedHandle, + getColumnPinningWithActions, getColumnsWithActions, getTanstackTableState } from './utils.tsx'; @@ -190,30 +191,29 @@ export function createDataTableStore(params: DataTableStoreParams) { if (updatedParams.mode === 'server') { _serverOnPaginationChange = updatedParams.onPaginationChange; _serverOnSortingChange = updatedParams.onSortingChange; - table.setOptions((options) => ({ - ...options, - columns: getColumnsWithActions(updatedParams), - data: updatedParams.data, - meta: { - ...updatedParams.meta, - [ROW_ACTIONS_METADATA_KEY]: updatedParams.rowActions, - [TABLE_NAME_METADATA_KEY]: updatedParams.tableName - }, - pageCount: updatedParams.pageCount - })); - } else { - table.setOptions((options) => ({ - ...options, - columns: getColumnsWithActions(updatedParams), - data: updatedParams.data, - meta: { - ...updatedParams.meta, - [ROW_ACTIONS_METADATA_KEY]: updatedParams.rowActions, - [TABLE_NAME_METADATA_KEY]: updatedParams.tableName - }, - state: getTanstackTableState(updatedParams) - })); } + table.setOptions((options) => ({ + ...options, + columns: getColumnsWithActions(updatedParams), + data: updatedParams.data, + meta: { + ...updatedParams.meta, + [ROW_ACTIONS_METADATA_KEY]: updatedParams.rowActions, + [TABLE_NAME_METADATA_KEY]: updatedParams.tableName + }, + ...(updatedParams.mode === 'server' && { pageCount: updatedParams.pageCount }) + })); + + // `initialState` is applied once, when the store is created. Everything in the live table + // state -- pagination, sorting, column filters, the global filter, column sizing -- is + // owned by the user, not by props, so rebuilding it here would send them back to page one + // (losing their sort and search along with it) every time `data` or `columns` changed + // identity. The only piece derived from props is the actions column's pinning, so + // reconcile just that. + setTableState('columnPinning', (columnPinning) => + getColumnPinningWithActions(columnPinning, updatedParams.rowActions) + ); + updateColumnSizing(); updateStyle(); invalidateHandles(); diff --git a/src/components/DataTable/utils.tsx b/src/components/DataTable/utils.tsx index dd796b6..69171ba 100644 --- a/src/components/DataTable/utils.tsx +++ b/src/components/DataTable/utils.tsx @@ -1,4 +1,12 @@ -import type { ColumnDef, ColumnSizingState, RowData, Table, TableState, Updater } from '@tanstack/table-core'; +import type { + ColumnDef, + ColumnPinningState, + ColumnSizingState, + RowData, + Table, + TableState, + Updater +} from '@tanstack/table-core'; import { sum } from 'lodash-es'; import { ACTIONS_COLUMN_ID, MEMOIZED_HANDLE_ID } from './constants.ts'; @@ -7,6 +15,7 @@ import { DataTableRowActionCell } from './DataTableRowActionCell.tsx'; import type { BaseDataTableStoreParams, DataTableColumnBreakpoints, + DataTableRowAction, DataTableStoreParams, MemoizedHandle } from './types.ts'; @@ -110,6 +119,22 @@ function getColumnsWithActions({ ]; } +/** + * The given column pinning, with the actions column pinned to the right if (and only if) the table + * has row actions. Pinning is the one part of the table state derived from props, so it has to be + * reconcilable on a props update without disturbing the rest of the state. + */ +function getColumnPinningWithActions( + columnPinning: ColumnPinningState, + rowActions: DataTableRowAction[] | undefined +): ColumnPinningState { + const right = (columnPinning.right ?? []).filter((id) => id !== ACTIONS_COLUMN_ID); + return { + ...columnPinning, + right: rowActions ? [...right, ACTIONS_COLUMN_ID] : right + }; +} + function getTanstackTableState({ initialState, rowActions }: DataTableStoreParams): TableState { const { columnFilters = [], columnPinning = {}, sorting = [] } = initialState ?? {}; const state: TableState = { @@ -137,12 +162,7 @@ function getTanstackTableState({ initialState, rowActions }: DataTableStorePa rowSelection: {}, sorting }; - if (rowActions) { - state.columnPinning = { - ...state.columnPinning, - right: [...(state.columnPinning.right ?? []), ACTIONS_COLUMN_ID] - }; - } + state.columnPinning = getColumnPinningWithActions(state.columnPinning, rowActions); return state; } @@ -155,6 +175,7 @@ export { calculateColumnSizing, defineMemoizedHandle, flexRender, + getColumnPinningWithActions, getColumnsWithActions, getTanstackTableState };