Extract shared TablePagination (dedupe DataTable + Media mobile)
Pull the duplicated pagination footer into a single shared component at frontend/src/components/ui/table-pagination.tsx. Both the desktop DataTable (which had an internal DataTablePagination driven by a TanStack table instance) and the Media mobile card list (which had a standalone MediaMobilePagination driven by raw PaginationState) now consume it. The shared component takes the raw primitives (pageIndex, pageSize, pageCount, totalRows, pageSizeOptions, onPaginationChange, optional className) so it backs both an adapter view (DataTable extracts state from its table instance and passes table.setPagination) and a direct state view (Media passes its pagination state directly). Includes the 44px mobile-touch-target on prev/next buttons (previously only on the Media mobile variant). Removes ~90 lines of duplication across data-table.tsx and Media.tsx; adds the focused 122-line shared component. The DataTable Select imports are dropped (now unused). 122 tests pass; lint/build green. Refs openspec/changes/mobile-responsive-parity/verify-report.md residual risk #5.
This commit is contained in:
@@ -6,7 +6,6 @@ import {
|
||||
type OnChangeFn,
|
||||
type PaginationState,
|
||||
type RowSelectionState,
|
||||
type Table as TableInstance,
|
||||
type VisibilityState,
|
||||
flexRender,
|
||||
getCoreRowModel,
|
||||
@@ -18,6 +17,7 @@ import { Columns3 } from "lucide-react";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Checkbox } from "@/components/ui/checkbox";
|
||||
import { TablePagination } from "@/components/ui/table-pagination";
|
||||
import {
|
||||
Table,
|
||||
TableBody,
|
||||
@@ -34,13 +34,6 @@ import {
|
||||
DropdownMenuSeparator,
|
||||
DropdownMenuTrigger,
|
||||
} from "@/components/ui/dropdown-menu";
|
||||
import {
|
||||
Select,
|
||||
SelectContent,
|
||||
SelectItem,
|
||||
SelectTrigger,
|
||||
SelectValue,
|
||||
} from "@/components/ui/select";
|
||||
|
||||
export interface DataTableProps<TData, TValue = unknown> {
|
||||
columns: ColumnDef<TData, TValue>[];
|
||||
@@ -253,90 +246,19 @@ export function DataTable<TData, TValue = unknown>({
|
||||
</div>
|
||||
|
||||
{enablePagination && (
|
||||
<DataTablePagination
|
||||
table={table}
|
||||
<TablePagination
|
||||
pageIndex={table.getState().pagination.pageIndex}
|
||||
pageSize={table.getState().pagination.pageSize}
|
||||
pageSizeOptions={pageSizeOptions}
|
||||
totalRows={manualPagination ? (rowCount ?? 0) : table.getRowModel().rows.length}
|
||||
pageCount={pageCount}
|
||||
manual={manualPagination}
|
||||
rowCount={rowCount}
|
||||
onPaginationChange={table.setPagination}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
interface PaginationProps<TData> {
|
||||
table: TableInstance<TData>;
|
||||
pageSizeOptions: number[];
|
||||
pageCount: number;
|
||||
manual: boolean;
|
||||
rowCount?: number;
|
||||
}
|
||||
|
||||
function DataTablePagination<TData>({
|
||||
table,
|
||||
pageSizeOptions,
|
||||
pageCount,
|
||||
manual,
|
||||
rowCount,
|
||||
}: PaginationProps<TData>) {
|
||||
const pageIndex = table.getState().pagination.pageIndex;
|
||||
const pageSize = table.getState().pagination.pageSize;
|
||||
const visibleRows = table.getRowModel().rows.length;
|
||||
const totalRows = manual ? (rowCount ?? 0) : visibleRows;
|
||||
|
||||
return (
|
||||
<div className="flex flex-wrap items-center justify-between gap-3 text-sm">
|
||||
<div className="text-muted-foreground">
|
||||
{`${totalRows} row${totalRows === 1 ? "" : "s"}`}
|
||||
</div>
|
||||
<div className="flex items-center gap-3">
|
||||
<div className="flex items-center gap-1.5">
|
||||
<span className="text-muted-foreground">Rows per page</span>
|
||||
<Select
|
||||
value={String(pageSize)}
|
||||
onValueChange={(value) => table.setPageSize(Number(value))}
|
||||
>
|
||||
<SelectTrigger
|
||||
size="sm"
|
||||
className="w-[70px]"
|
||||
aria-label="Rows per page"
|
||||
>
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{pageSizeOptions.map((option) => (
|
||||
<SelectItem key={option} value={String(option)}>
|
||||
{option}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
</div>
|
||||
<span className="text-muted-foreground">
|
||||
Page {pageIndex + 1} of {pageCount}
|
||||
</span>
|
||||
<div className="flex items-center gap-1">
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
onClick={() => table.previousPage()}
|
||||
disabled={!table.getCanPreviousPage()}
|
||||
aria-label="Previous page"
|
||||
>
|
||||
Previous
|
||||
</Button>
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
onClick={() => table.nextPage()}
|
||||
disabled={!table.getCanNextPage()}
|
||||
aria-label="Next page"
|
||||
>
|
||||
Next
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
// DataTablePagination was extracted into the shared TablePagination component
|
||||
// (frontend/src/components/ui/table-pagination.tsx). Both the desktop DataTable
|
||||
// and the Media mobile card list consume it.
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
import type { OnChangeFn } from "@tanstack/react-table";
|
||||
import type { PaginationState } from "@tanstack/react-table";
|
||||
|
||||
import { cn } from "@/lib/utils";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import {
|
||||
Select,
|
||||
SelectContent,
|
||||
SelectItem,
|
||||
SelectTrigger,
|
||||
SelectValue,
|
||||
} from "@/components/ui/select";
|
||||
|
||||
/**
|
||||
* Shared pagination footer for table-style views.
|
||||
*
|
||||
* Renders the rows count, page-size select, page indicator, and prev/next
|
||||
* buttons. Works off the raw {@link PaginationState} primitives so it can back
|
||||
* both a TanStack `Table` instance (via a thin adapter) and standalone card
|
||||
* layouts that drive pagination directly (e.g. MediaMobilePagination).
|
||||
*
|
||||
* The Desktop DataTable and the Media mobile card list both consume this to
|
||||
* avoid the duplication flagged in
|
||||
* `openspec/changes/mobile-responsive-parity/verify-report.md` residual risk #5.
|
||||
*/
|
||||
export interface TablePaginationProps {
|
||||
pageIndex: number;
|
||||
pageSize: number;
|
||||
pageSizeOptions: number[];
|
||||
totalRows: number;
|
||||
pageCount: number;
|
||||
onPaginationChange: OnChangeFn<PaginationState>;
|
||||
/** Optional extra className on the outer container (e.g. "p-4"). */
|
||||
className?: string;
|
||||
}
|
||||
|
||||
export function TablePagination({
|
||||
pageIndex,
|
||||
pageSize,
|
||||
pageSizeOptions,
|
||||
totalRows,
|
||||
pageCount,
|
||||
onPaginationChange,
|
||||
className,
|
||||
}: TablePaginationProps) {
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
"flex flex-wrap items-center justify-between gap-3 text-sm",
|
||||
className,
|
||||
)}
|
||||
>
|
||||
<div className="text-muted-foreground">
|
||||
{`${totalRows} row${totalRows === 1 ? "" : "s"}`}
|
||||
</div>
|
||||
<div className="flex items-center gap-3">
|
||||
<div className="flex items-center gap-1.5">
|
||||
<span className="text-muted-foreground">Rows per page</span>
|
||||
<Select
|
||||
value={String(pageSize)}
|
||||
onValueChange={(value) =>
|
||||
onPaginationChange(() => ({
|
||||
pageIndex: 0,
|
||||
pageSize: Number(value),
|
||||
}))
|
||||
}
|
||||
>
|
||||
<SelectTrigger
|
||||
size="sm"
|
||||
className="w-[70px]"
|
||||
aria-label="Rows per page"
|
||||
>
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{pageSizeOptions.map((option) => (
|
||||
<SelectItem key={option} value={String(option)}>
|
||||
{option}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
</div>
|
||||
<span className="text-muted-foreground">
|
||||
Page {pageIndex + 1} of {pageCount}
|
||||
</span>
|
||||
<div className="flex items-center gap-1">
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
className="mobile-touch-target"
|
||||
onClick={() =>
|
||||
onPaginationChange((prev) => ({
|
||||
...prev,
|
||||
pageIndex: Math.max(0, prev.pageIndex - 1),
|
||||
}))
|
||||
}
|
||||
disabled={pageIndex <= 0}
|
||||
aria-label="Previous page"
|
||||
>
|
||||
Previous
|
||||
</Button>
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
className="mobile-touch-target"
|
||||
onClick={() =>
|
||||
onPaginationChange((prev) => ({
|
||||
...prev,
|
||||
pageIndex: prev.pageIndex + 1,
|
||||
}))
|
||||
}
|
||||
disabled={pageIndex >= pageCount - 1}
|
||||
aria-label="Next page"
|
||||
>
|
||||
Next
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
MobileCardRow,
|
||||
type MobileCardField,
|
||||
} from "@/components/ui/mobile-card";
|
||||
import { TablePagination } from "@/components/ui/table-pagination";
|
||||
import { Alert, AlertDescription } from "@/components/ui/alert";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Card, CardContent } from "@/components/ui/card";
|
||||
@@ -100,97 +101,8 @@ const mediaCardFields: MobileCardField<MediaItem>[] = [
|
||||
},
|
||||
];
|
||||
|
||||
// Standalone pagination for the mobile card layout. The DataTable renders its
|
||||
// own pagination internally; this mirrors that UI (rows count, page-size
|
||||
// select, page indicator, prev/next) but works off the raw pagination state
|
||||
// instead of a TanStack table instance. See spec R3.3.
|
||||
function MediaMobilePagination({
|
||||
pageIndex,
|
||||
pageSize,
|
||||
pageSizeOptions,
|
||||
totalRows,
|
||||
pageCount,
|
||||
onPaginationChange,
|
||||
}: {
|
||||
pageIndex: number;
|
||||
pageSize: number;
|
||||
pageSizeOptions: number[];
|
||||
totalRows: number;
|
||||
pageCount: number;
|
||||
onPaginationChange: OnChangeFn<PaginationState>;
|
||||
}) {
|
||||
return (
|
||||
<div className="flex flex-wrap items-center justify-between gap-3 p-4 text-sm">
|
||||
<div className="text-muted-foreground">
|
||||
{`${totalRows} row${totalRows === 1 ? "" : "s"}`}
|
||||
</div>
|
||||
<div className="flex items-center gap-3">
|
||||
<div className="flex items-center gap-1.5">
|
||||
<span className="text-muted-foreground">Rows per page</span>
|
||||
<Select
|
||||
value={String(pageSize)}
|
||||
onValueChange={(value) =>
|
||||
onPaginationChange(() => ({
|
||||
pageIndex: 0,
|
||||
pageSize: Number(value),
|
||||
}))
|
||||
}
|
||||
>
|
||||
<SelectTrigger
|
||||
size="sm"
|
||||
className="w-[70px]"
|
||||
aria-label="Rows per page"
|
||||
>
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
{pageSizeOptions.map((option) => (
|
||||
<SelectItem key={option} value={String(option)}>
|
||||
{option}
|
||||
</SelectItem>
|
||||
))}
|
||||
</SelectContent>
|
||||
</Select>
|
||||
</div>
|
||||
<span className="text-muted-foreground">
|
||||
Page {pageIndex + 1} of {pageCount}
|
||||
</span>
|
||||
<div className="flex items-center gap-1">
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
onClick={() =>
|
||||
onPaginationChange((prev) => ({
|
||||
...prev,
|
||||
pageIndex: Math.max(0, prev.pageIndex - 1),
|
||||
}))
|
||||
}
|
||||
disabled={pageIndex <= 0}
|
||||
aria-label="Previous page"
|
||||
className="mobile-touch-target"
|
||||
>
|
||||
Previous
|
||||
</Button>
|
||||
<Button
|
||||
variant="outline"
|
||||
size="sm"
|
||||
onClick={() =>
|
||||
onPaginationChange((prev) => ({
|
||||
...prev,
|
||||
pageIndex: prev.pageIndex + 1,
|
||||
}))
|
||||
}
|
||||
disabled={pageIndex >= pageCount - 1}
|
||||
className="mobile-touch-target"
|
||||
aria-label="Next page"
|
||||
>
|
||||
Next
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
// Mobile pagination uses the shared TablePagination component
|
||||
// (frontend/src/components/ui/table-pagination.tsx).
|
||||
|
||||
const MEDIA_TAB_STATE_KEY = "manage.media.tabState";
|
||||
const SMALL_BREAKPOINT = "(max-width: 900px)";
|
||||
@@ -662,13 +574,14 @@ export function Media() {
|
||||
/>
|
||||
</div>
|
||||
{queryResult && (
|
||||
<MediaMobilePagination
|
||||
<TablePagination
|
||||
pageIndex={pageIndex}
|
||||
pageSize={pageSize}
|
||||
pageSizeOptions={[50, 100, 200]}
|
||||
totalRows={total}
|
||||
pageCount={totalPages}
|
||||
onPaginationChange={handlePaginationChange}
|
||||
className="p-4"
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user