mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-05-06 08:47:18 +00:00
refactor(ui): unify DataTable pagination into a single callback (#10863)
Co-authored-by: Pablo F.G <pablo.fernandez@prowler.com>
This commit is contained in:
committed by
GitHub
parent
85d38b5f71
commit
4fb5272362
@@ -27,6 +27,7 @@ All notable changes to the **Prowler UI** are documented in this file.
|
||||
- Mutelist improvements: table now supports name/reason search and visual count badges for finding targets [(#10846)](https://github.com/prowler-cloud/prowler/pull/10846)
|
||||
- Resources now use batch-applied filters, render metadata JSON with syntax highlighting, and more [(#10861)](https://github.com/prowler-cloud/prowler/pull/10861)
|
||||
- Table pagination controls now keep their arrows visible on hover in light theme, and more UI improvements [(#10862)](https://github.com/prowler-cloud/prowler/pull/10862)
|
||||
- Fix rows-per-page selector silently ignoring changes in URL-driven tables by unifying `DataTable` pagination into a single `onPaginationChange` callback [(#10863)](https://github.com/prowler-cloud/prowler/pull/10863)
|
||||
|
||||
---
|
||||
|
||||
|
||||
+3
-23
@@ -3,7 +3,6 @@
|
||||
import { ColumnDef } from "@tanstack/react-table";
|
||||
import { Check, Minus } from "lucide-react";
|
||||
import { usePathname, useRouter, useSearchParams } from "next/navigation";
|
||||
import { useRef } from "react";
|
||||
|
||||
import {
|
||||
RadioGroup,
|
||||
@@ -244,15 +243,6 @@ export const ScanListTable = ({ scans }: ScanListTableProps) => {
|
||||
const endIndex = startIndex + pageSize;
|
||||
const paginatedScans = scans.slice(startIndex, endIndex);
|
||||
|
||||
// TODO(#10863): remove this workaround (ref + split handlers + pushWithParams)
|
||||
// once the DataTable unified-pagination-callback refactor in PR #10863 lands.
|
||||
// The underlying issue is that DataTablePagination's controlled mode fires
|
||||
// onPageSizeChange and onPageChange(1) back-to-back in the same tick, so the
|
||||
// second router.push reads a stale searchParams snapshot and silently reverts
|
||||
// the page-size change. Replace both handlers with a single
|
||||
// onPaginationChange handler after that PR merges.
|
||||
const suppressNextPageResetRef = useRef(false);
|
||||
|
||||
const pushWithParams = (nextParams: Record<string, string>) => {
|
||||
const params = new URLSearchParams(searchParams.toString());
|
||||
|
||||
@@ -267,18 +257,9 @@ export const ScanListTable = ({ scans }: ScanListTableProps) => {
|
||||
pushWithParams({ scanId });
|
||||
};
|
||||
|
||||
const handlePageChange = (page: number) => {
|
||||
if (suppressNextPageResetRef.current && page === 1) {
|
||||
suppressNextPageResetRef.current = false;
|
||||
return;
|
||||
}
|
||||
pushWithParams({ scanPage: page.toString() });
|
||||
};
|
||||
|
||||
const handlePageSizeChange = (nextPageSize: number) => {
|
||||
suppressNextPageResetRef.current = true;
|
||||
const handlePaginationChange = (nextPage: number, nextPageSize: number) => {
|
||||
pushWithParams({
|
||||
scanPage: "1",
|
||||
scanPage: nextPage.toString(),
|
||||
scanPageSize: nextPageSize.toString(),
|
||||
});
|
||||
};
|
||||
@@ -295,8 +276,7 @@ export const ScanListTable = ({ scans }: ScanListTableProps) => {
|
||||
metadata={buildMetadata(scans.length, currentPage, totalPages)}
|
||||
controlledPage={currentPage}
|
||||
controlledPageSize={pageSize}
|
||||
onPageChange={handlePageChange}
|
||||
onPageSizeChange={handlePageSizeChange}
|
||||
onPaginationChange={handlePaginationChange}
|
||||
onRowClick={(row) => {
|
||||
if (row.original.attributes.graph_data_ready) {
|
||||
handleSelectScan(row.original.id);
|
||||
|
||||
@@ -402,14 +402,10 @@ export const ResourceDetailContent = ({
|
||||
}}
|
||||
controlledPage={currentPage}
|
||||
controlledPageSize={pageSize}
|
||||
onPageChange={(page) => {
|
||||
onPaginationChange={(nextPage, nextPageSize) => {
|
||||
setRowSelection({});
|
||||
setCurrentPage(page);
|
||||
}}
|
||||
onPageSizeChange={(size) => {
|
||||
setRowSelection({});
|
||||
setCurrentPage(1);
|
||||
setPageSize(size);
|
||||
setCurrentPage(nextPage);
|
||||
setPageSize(nextPageSize);
|
||||
}}
|
||||
isLoading={findingsLoading}
|
||||
/>
|
||||
|
||||
@@ -28,14 +28,13 @@ interface DataTablePaginationProps {
|
||||
paramPrefix?: string;
|
||||
|
||||
/*
|
||||
* Controlled mode: Use these props to manage pagination via React state
|
||||
* instead of URL params. Useful for tables in drawers/modals to avoid
|
||||
* triggering page re-renders when paginating.
|
||||
* Controlled mode: receive all three props together (parent contract is
|
||||
* enforced at the DataTable boundary). Useful for tables in drawers/modals
|
||||
* to avoid triggering page re-renders when paginating.
|
||||
*/
|
||||
controlledPage?: number;
|
||||
controlledPageSize?: number;
|
||||
onPageChange?: (page: number) => void;
|
||||
onPageSizeChange?: (pageSize: number) => void;
|
||||
onPaginationChange?: (page: number, pageSize: number) => void;
|
||||
}
|
||||
|
||||
const NAV_BUTTON_STYLES = {
|
||||
@@ -51,22 +50,26 @@ export function DataTablePagination({
|
||||
paramPrefix = "",
|
||||
controlledPage,
|
||||
controlledPageSize,
|
||||
onPageChange,
|
||||
onPageSizeChange,
|
||||
onPaginationChange,
|
||||
}: DataTablePaginationProps) {
|
||||
const pathname = usePathname();
|
||||
const searchParams = useSearchParams();
|
||||
const router = useRouter();
|
||||
|
||||
// Determine if we're in controlled mode
|
||||
const isControlled = controlledPage !== undefined && onPageChange;
|
||||
// Determine if we're in controlled mode. The discriminated union on
|
||||
// `DataTable`'s ControlledPaginationProps guarantees `controlledPageSize`
|
||||
// is defined here whenever the other two are.
|
||||
const isControlled =
|
||||
controlledPage !== undefined &&
|
||||
controlledPageSize !== undefined &&
|
||||
onPaginationChange !== undefined;
|
||||
|
||||
// Determine param names based on prefix
|
||||
const pageParam = paramPrefix ? `${paramPrefix}Page` : "page";
|
||||
const pageSizeParam = paramPrefix ? `${paramPrefix}PageSize` : "pageSize";
|
||||
|
||||
const initialPageSize = isControlled
|
||||
? String(controlledPageSize ?? 10)
|
||||
? String(controlledPageSize)
|
||||
: (searchParams.get(pageSizeParam) ?? "10");
|
||||
|
||||
const [selectedPageSize, setSelectedPageSize] = useState(initialPageSize);
|
||||
@@ -112,7 +115,7 @@ export function DataTablePagination({
|
||||
// Handle page navigation for controlled mode
|
||||
const handlePageChange = (pageNumber: number) => {
|
||||
if (isControlled) {
|
||||
onPageChange(pageNumber);
|
||||
onPaginationChange(pageNumber, controlledPageSize);
|
||||
} else {
|
||||
const url = createPageUrl(pageNumber);
|
||||
if (disableScroll) {
|
||||
@@ -141,8 +144,7 @@ export function DataTablePagination({
|
||||
setSelectedPageSize(value);
|
||||
|
||||
if (isControlled) {
|
||||
onPageSizeChange?.(parseInt(value, 10));
|
||||
onPageChange(1); // Reset to first page
|
||||
onPaginationChange(1, parseInt(value, 10));
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -41,7 +41,25 @@ import { FilterOption, MetaDataProps } from "@/types";
|
||||
*/
|
||||
const DEFAULT_COLUMN_SIZE = 150;
|
||||
|
||||
interface DataTableProviderProps<TData, TValue> {
|
||||
/*
|
||||
* Controlled pagination: pass all three props together or none. Modeled as a
|
||||
* discriminated union so TypeScript prevents passing `onPaginationChange`
|
||||
* without `controlledPageSize`, which would otherwise silently emit a default
|
||||
* page size on every navigation.
|
||||
*/
|
||||
type ControlledPaginationProps =
|
||||
| {
|
||||
controlledPage: number;
|
||||
controlledPageSize: number;
|
||||
onPaginationChange: (page: number, pageSize: number) => void;
|
||||
}
|
||||
| {
|
||||
controlledPage?: undefined;
|
||||
controlledPageSize?: undefined;
|
||||
onPaginationChange?: undefined;
|
||||
};
|
||||
|
||||
type DataTableProviderProps<TData, TValue> = {
|
||||
columns: ColumnDef<TData, TValue>[];
|
||||
data: TData[];
|
||||
metadata?: MetaDataProps;
|
||||
@@ -67,28 +85,6 @@ interface DataTableProviderProps<TData, TValue> {
|
||||
/** Prefix for URL params to avoid conflicts (e.g., "findings" -> "findingsPage") */
|
||||
paramPrefix?: string;
|
||||
|
||||
/*
|
||||
* Controlled Mode Props
|
||||
* ---------------------
|
||||
* By default, DataTable uses URL params for pagination/search (via paramPrefix).
|
||||
* This causes Next.js page re-renders on every interaction.
|
||||
*
|
||||
* For tables inside drawers/modals, use controlled mode instead:
|
||||
* - Pass controlledPage, controlledPageSize, controlledSearch as state values
|
||||
* - Pass onPageChange, onPageSizeChange, onSearchChange as state setters
|
||||
* - This keeps state local, avoiding URL changes and unnecessary page re-renders
|
||||
*
|
||||
* Example:
|
||||
* const [page, setPage] = useState(1);
|
||||
* const [search, setSearch] = useState("");
|
||||
* <DataTable
|
||||
* controlledPage={page}
|
||||
* onPageChange={setPage}
|
||||
* controlledSearch={search}
|
||||
* onSearchChange={setSearch}
|
||||
* isLoading={isLoading}
|
||||
* />
|
||||
*/
|
||||
controlledSearch?: string;
|
||||
onSearchChange?: (value: string) => void;
|
||||
/**
|
||||
@@ -96,10 +92,6 @@ interface DataTableProviderProps<TData, TValue> {
|
||||
* Use this alongside onSearchChange to implement "search on Enter" behavior.
|
||||
*/
|
||||
onSearchCommit?: (value: string) => void;
|
||||
controlledPage?: number;
|
||||
controlledPageSize?: number;
|
||||
onPageChange?: (page: number) => void;
|
||||
onPageSizeChange?: (pageSize: number) => void;
|
||||
/** Show loading state with opacity overlay (for controlled mode) */
|
||||
isLoading?: boolean;
|
||||
/** Custom placeholder text for the search input */
|
||||
@@ -114,7 +106,7 @@ interface DataTableProviderProps<TData, TValue> {
|
||||
header?: ReactNode;
|
||||
/** Optional content rendered in the toolbar before the total entries count. */
|
||||
toolbarRightContent?: ReactNode;
|
||||
}
|
||||
} & ControlledPaginationProps;
|
||||
|
||||
export function DataTable<TData, TValue>({
|
||||
columns,
|
||||
@@ -137,8 +129,7 @@ export function DataTable<TData, TValue>({
|
||||
onSearchCommit,
|
||||
controlledPage,
|
||||
controlledPageSize,
|
||||
onPageChange,
|
||||
onPageSizeChange,
|
||||
onPaginationChange,
|
||||
isLoading = false,
|
||||
searchPlaceholder,
|
||||
renderAfterRow,
|
||||
@@ -353,8 +344,7 @@ export function DataTable<TData, TValue>({
|
||||
paramPrefix={paramPrefix}
|
||||
controlledPage={controlledPage}
|
||||
controlledPageSize={controlledPageSize}
|
||||
onPageChange={onPageChange}
|
||||
onPageSizeChange={onPageSizeChange}
|
||||
onPaginationChange={onPaginationChange}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user