mirror of
https://github.com/vxcontrol/pentagi.git
synced 2026-07-10 14:17:11 +00:00
fix(frontend): sync URL on pageSize change, label knowledge columns
- DataTable: route handlePageSizeChange through handlePaginationChange so onPageChange fires when picking "All" on a high page drops pageIndex to 0. Without this, the URL kept a stale ?page=N while the display correctly clamped to "Page 1 of 1". - DataTable: reconcile effect in controlled mode now compares externalPageIndex (the URL source of truth) against safePageIndex instead of the internal mirror, which can drop to 0 via handlePageSizeChange and hide the URL mismatch from the previous comparison. - knowledges: add meta.columnMenuLabel to docType and question columns so the Columns dropdown reads "Type" / "Question" — same labels as the table headers — aligning with the Flags/Preview entries. - data-table.test: cover both pathways — out-of-range controlled pageIndex on mount, and pageSize=All from page 2. - vitest.setup: polyfill hasPointerCapture / setPointerCapture / releasePointerCapture so Radix Select interactions don't crash in jsdom. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -206,6 +206,61 @@ describe('DataTable — sorting state persists to storage', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('DataTable — controlled pageIndex reconciliation', () => {
|
||||
it('asks the parent to clamp an out-of-range controlled pageIndex on mount', () => {
|
||||
const onPageChange = vi.fn();
|
||||
|
||||
render(
|
||||
<DataTable<Row>
|
||||
columns={COLUMNS}
|
||||
data={ROWS}
|
||||
onPageChange={onPageChange}
|
||||
pageIndex={9}
|
||||
/>,
|
||||
{ wrapper: Wrapper },
|
||||
);
|
||||
|
||||
// 3 rows / default pageSize 10 → 1 page. The URL-driven pageIndex 9
|
||||
// is out of range, so the parent must be told to drop to 0 so the
|
||||
// canonical URL no longer points past the dataset.
|
||||
expect(onPageChange).toHaveBeenCalledWith(0);
|
||||
});
|
||||
|
||||
it('clears the URL page when picking "All" on a high page in controlled mode', async () => {
|
||||
const user = userEvent.setup();
|
||||
// 15 rows × default pageSize 10 → 2 pages; start on page 2.
|
||||
const manyRows: Row[] = Array.from({ length: 15 }, (_, index) => ({
|
||||
id: String(index + 1),
|
||||
name: `Row ${index + 1}`,
|
||||
}));
|
||||
const onPageChange = vi.fn();
|
||||
|
||||
render(
|
||||
<DataTable<Row>
|
||||
columns={COLUMNS}
|
||||
data={manyRows}
|
||||
onPageChange={onPageChange}
|
||||
pageIndex={1}
|
||||
/>,
|
||||
{ wrapper: Wrapper },
|
||||
);
|
||||
|
||||
// Initial mirror of the controlled pageIndex shouldn't trigger a
|
||||
// reconcile — page 2 of 2 is in range.
|
||||
expect(onPageChange).not.toHaveBeenCalled();
|
||||
|
||||
// Open the rows-per-page select and pick "All".
|
||||
const trigger = screen.getByRole('combobox');
|
||||
await user.click(trigger);
|
||||
const option = await screen.findByRole('option', { name: 'All' });
|
||||
await user.click(option);
|
||||
|
||||
// "All" collapses the dataset to a single page; the parent has to
|
||||
// hear about pageIndex 0 so `?page=2` is dropped from the URL.
|
||||
expect(onPageChange).toHaveBeenCalledWith(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('DataTable — empty results', () => {
|
||||
it('does not render "Page 1 of 0" when there are no rows', () => {
|
||||
render(
|
||||
|
||||
@@ -289,10 +289,6 @@ function DataTable<TData, TValue = unknown>({
|
||||
}
|
||||
}, [externalPageIndex]);
|
||||
|
||||
const handlePageSizeChange = useCallback((newPageSize: number) => {
|
||||
setPagination({ pageIndex: 0, pageSize: newPageSize });
|
||||
}, []);
|
||||
|
||||
const paginationReference = useLatestRef(pagination);
|
||||
|
||||
const handlePaginationChange = useCallback(
|
||||
@@ -312,6 +308,18 @@ function DataTable<TData, TValue = unknown>({
|
||||
[onPageChange, paginationReference],
|
||||
);
|
||||
|
||||
// Route page-size changes through the same channel as TanStack's
|
||||
// pagination mutations so `onPageChange` fires when the URL-driven
|
||||
// pageIndex needs to drop to 0. Without this, picking "All" on a high
|
||||
// page leaves `?page=5` stranded in the URL even though the display
|
||||
// correctly clamps to "Page 1 of 1".
|
||||
const handlePageSizeChange = useCallback(
|
||||
(newPageSize: number) => {
|
||||
handlePaginationChange({ pageIndex: 0, pageSize: newPageSize });
|
||||
},
|
||||
[handlePaginationChange],
|
||||
);
|
||||
|
||||
const table = useReactTable({
|
||||
autoResetPageIndex: false,
|
||||
columns,
|
||||
@@ -370,21 +378,29 @@ function DataTable<TData, TValue = unknown>({
|
||||
// pipeline, which React forbids during render. The early return makes the
|
||||
// effect a no-op on every render where the URL is already in range, so
|
||||
// the cost on the happy path is one comparison.
|
||||
//
|
||||
// Controlled mode compares the URL (`externalPageIndex`) — the canonical
|
||||
// source of truth — rather than the internal mirror, because internal
|
||||
// state can drop to a clamped value via `handlePageSizeChange` while the
|
||||
// URL still points at the stale page. Uncontrolled mode keeps comparing
|
||||
// the internal pageIndex since there's no other source.
|
||||
useEffect(() => {
|
||||
if (pageCount === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (pagination.pageIndex === safePageIndex) {
|
||||
if (isPageControlled) {
|
||||
if (externalPageIndex !== undefined && externalPageIndex !== safePageIndex) {
|
||||
onPageChange?.(safePageIndex);
|
||||
}
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
if (isPageControlled) {
|
||||
onPageChange?.(safePageIndex);
|
||||
} else {
|
||||
if (pagination.pageIndex !== safePageIndex) {
|
||||
setPagination((previous) => ({ ...previous, pageIndex: safePageIndex }));
|
||||
}
|
||||
}, [isPageControlled, onPageChange, pageCount, pagination.pageIndex, safePageIndex]);
|
||||
}, [externalPageIndex, isPageControlled, onPageChange, pageCount, pagination.pageIndex, safePageIndex]);
|
||||
|
||||
return (
|
||||
<div className="w-full">
|
||||
|
||||
@@ -173,6 +173,7 @@ const Knowledges = () => {
|
||||
},
|
||||
header: ({ column }) => <SortableColumnHeader column={column} label="Type" />,
|
||||
maxSize: 180,
|
||||
meta: { columnMenuLabel: 'Type' },
|
||||
minSize: 110,
|
||||
size: 130,
|
||||
},
|
||||
@@ -209,6 +210,7 @@ const Knowledges = () => {
|
||||
);
|
||||
},
|
||||
header: ({ column }) => <SortableColumnHeader column={column} label="Question" />,
|
||||
meta: { columnMenuLabel: 'Question' },
|
||||
minSize: 180,
|
||||
size: 280,
|
||||
},
|
||||
|
||||
@@ -31,6 +31,21 @@ if (typeof globalThis.ResizeObserver === 'undefined') {
|
||||
globalThis.ResizeObserver = NoopResizeObserver as unknown as typeof ResizeObserver;
|
||||
}
|
||||
|
||||
// Radix Select gates pointer events behind `hasPointerCapture` / `setPointerCapture`,
|
||||
// which jsdom doesn't implement. Without these no-ops, `userEvent.click` on a
|
||||
// SelectTrigger throws "target.hasPointerCapture is not a function" mid-render.
|
||||
if (typeof Element !== 'undefined' && !Element.prototype.hasPointerCapture) {
|
||||
Element.prototype.hasPointerCapture = function hasPointerCapture() {
|
||||
return false;
|
||||
};
|
||||
Element.prototype.setPointerCapture = function setPointerCapture() {
|
||||
/* no-op for jsdom */
|
||||
};
|
||||
Element.prototype.releasePointerCapture = function releasePointerCapture() {
|
||||
/* no-op for jsdom */
|
||||
};
|
||||
}
|
||||
|
||||
// React Testing Library leaves rendered nodes attached to `document.body`
|
||||
// after each test. Without this, tests would leak DOM state into each other
|
||||
// — e.g. two `render(<X />)` calls would both end up on screen at once.
|
||||
|
||||
Reference in New Issue
Block a user