refactor(settings): dedupe the categories and labels pages (#770)

## Why

`settings/categories.tsx` and `settings/labels.tsx` were the last big
duplication cluster after the landing page (#768): 6 clone pairs between
and within them, ~100 duplicated lines.

They are the same page with a different record type:

- the trailing `...` dropdown with Edit / Delete
- the right-click context menu on each row, with the same two items
- the pair of dialog mounts wired to two `useState` flags — written
**twice per page**, once for the dropdown and once for the context menu
- 39 lines of table markup (header groups, body, empty row) differing
only in the row component and the empty message

## What changed

Three shared pieces:

| component | replaces |
|---|---|
| `RowActionsDropdown` | the `...` cell + its two dialogs, in both pages
|
| `RowWithActionsContextMenu` | the row + context menu + its two
dialogs, in both pages |
| `SettingsTable` | the table markup, in both pages |

The dialogs come in as render props (`renderEditDialog` /
`renderDeleteDialog`), so each page keeps its own dialog components and
their extra props — `categories` needs the full category list, labels
needs nothing — while the menus own the open state.

`categories.tsx` −86 lines, `labels.tsx` −100 lines.

### Why not the existing DataTable

`components/ui/data-table.tsx` already renders a table, but it is
**virtualized**: its `renderRow` hands back a `VirtualItem` and the
virtualizer, and rows are expected to attach `measureElement`. These
settings lists are short and their rows are wrapped in a
`ContextMenuTrigger asChild`, so adopting it would mean threading
measurement through the context menu to gain nothing. `SettingsTable` is
the plain version, and `accounts.tsx` / `automation-rules.tsx` (which
repeat the same markup) can move onto it next.

## Metrics

Cumulative with the other refactor PRs in flight; measured against
`main`:

| | before | after |
|---|---|---|
| duplicated lines (tsx) | 5.82% | 5.40% |
| duplicated lines (total) | 5.34% | 5.11% |
| clones | 322 | 314 |

## Testing

ESLint and `tsc --noEmit` clean. No behaviour change: same markup, same
handlers, same dialogs — only their location moved.

The browser suite is what actually exercises these pages
(`CategoriesTest` covers viewing, creating, filtering, the empty state
and cell alignment). I could not run it locally without a production
asset build, so it is verified here by CI's `browser-tests-matrix`.
This commit is contained in:
Víctor Falcón 2026-08-11 14:37:23 +02:00 committed by GitHub
parent 952ca41eb1
commit 8d58e0bf81
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 281 additions and 352 deletions

View File

@ -0,0 +1,141 @@
import { Button } from '@/components/ui/button';
import {
ContextMenu,
ContextMenuContent,
ContextMenuItem,
ContextMenuLabel,
ContextMenuTrigger,
} from '@/components/ui/context-menu';
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuLabel,
DropdownMenuTrigger,
} from '@/components/ui/dropdown-menu';
import { TableCell, TableRow } from '@/components/ui/table';
import { __ } from '@/utils/i18n';
import { type Cell, flexRender, type Row } from '@tanstack/react-table';
import { MoreHorizontal } from 'lucide-react';
import { type ReactNode, useState } from 'react';
/** The open state a menu hands to the dialog it triggers. */
export interface DialogControl {
open: boolean;
onOpenChange: (open: boolean) => void;
}
/**
* Renders a dialog whose open state is owned by the menu that triggers it, so
* each settings page keeps its own edit/delete dialogs and props.
*/
type DialogRenderer = (control: DialogControl) => ReactNode;
interface EditDeleteDialogs {
renderEditDialog: DialogRenderer;
renderDeleteDialog: DialogRenderer;
}
/**
* The trailing "..." cell of a settings table: edit and delete, each opening
* the dialog the page passed in.
*/
export function RowActionsDropdown({
renderEditDialog,
renderDeleteDialog,
}: EditDeleteDialogs) {
const [editOpen, setEditOpen] = useState(false);
const [deleteOpen, setDeleteOpen] = useState(false);
return (
<>
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button variant="ghost" className="h-8 w-8 p-0">
<span className="sr-only">{__('Open menu')}</span>
<MoreHorizontal className="h-4 w-4" />
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end">
<DropdownMenuLabel>{__('Actions')}</DropdownMenuLabel>
<DropdownMenuItem onClick={() => setEditOpen(true)}>
{__('Edit')}
</DropdownMenuItem>
<DropdownMenuItem
onClick={() => setDeleteOpen(true)}
variant="destructive"
>
{__('Delete')}
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
{renderEditDialog({ open: editOpen, onOpenChange: setEditOpen })}
{renderDeleteDialog({
open: deleteOpen,
onOpenChange: setDeleteOpen,
})}
</>
);
}
/**
* A settings table row that offers the same edit and delete actions on
* right-click, staying highlighted while its menu is open.
*/
export function RowWithActionsContextMenu<TData>({
row,
renderEditDialog,
renderDeleteDialog,
}: { row: Row<TData> } & EditDeleteDialogs) {
const [editOpen, setEditOpen] = useState(false);
const [deleteOpen, setDeleteOpen] = useState(false);
const [contextMenuOpen, setContextMenuOpen] = useState(false);
return (
<>
<ContextMenu onOpenChange={setContextMenuOpen}>
<ContextMenuTrigger asChild>
<TableRow
data-state={
(row.getIsSelected() || contextMenuOpen) &&
'selected'
}
>
{row
.getVisibleCells()
.map((cell: Cell<TData, unknown>) => (
<TableCell
key={cell.id}
className="align-middle"
>
{flexRender(
cell.column.columnDef.cell,
cell.getContext(),
)}
</TableCell>
))}
</TableRow>
</ContextMenuTrigger>
<ContextMenuContent>
<ContextMenuLabel>{__('Actions')}</ContextMenuLabel>
<ContextMenuItem onClick={() => setEditOpen(true)}>
{__('Edit')}
</ContextMenuItem>
<ContextMenuItem
onClick={() => setDeleteOpen(true)}
variant="destructive"
>
{__('Delete')}
</ContextMenuItem>
</ContextMenuContent>
</ContextMenu>
{renderEditDialog({ open: editOpen, onOpenChange: setEditOpen })}
{renderDeleteDialog({
open: deleteOpen,
onOpenChange: setDeleteOpen,
})}
</>
);
}

View File

@ -0,0 +1,68 @@
import {
Table,
TableBody,
TableCell,
TableHead,
TableHeader,
TableRow,
} from '@/components/ui/table';
import {
flexRender,
type Row,
type Table as TableType,
} from '@tanstack/react-table';
import { type ReactNode } from 'react';
/**
* The bordered table the settings pages list their records in. Deliberately not
* the virtualized DataTable: these lists are short, and each page renders its
* own row component (they carry a per-row context menu).
*/
export function SettingsTable<TData>({
table,
emptyMessage,
renderRow,
}: {
table: TableType<TData>;
emptyMessage: string;
renderRow: (row: Row<TData>) => ReactNode;
}) {
const rows = table.getRowModel().rows;
return (
<div className="overflow-hidden rounded-md border">
<Table>
<TableHeader>
{table.getHeaderGroups().map((headerGroup) => (
<TableRow key={headerGroup.id}>
{headerGroup.headers.map((header) => (
<TableHead key={header.id}>
{header.isPlaceholder
? null
: flexRender(
header.column.columnDef.header,
header.getContext(),
)}
</TableHead>
))}
</TableRow>
))}
</TableHeader>
<TableBody>
{rows.length ? (
rows.map(renderRow)
) : (
<TableRow>
<TableCell
colSpan={table.getAllColumns().length}
className="h-24 text-center align-middle"
>
{emptyMessage}
</TableCell>
</TableRow>
)}
</TableBody>
</Table>
</div>
);
}

View File

@ -1,10 +1,8 @@
import { __ } from '@/utils/i18n';
import { Head, usePage } from '@inertiajs/react';
import {
Cell,
ColumnDef,
ColumnFiltersState,
flexRender,
getCoreRowModel,
getFilteredRowModel,
getSortedRowModel,
@ -14,7 +12,7 @@ import {
VisibilityState,
} from '@tanstack/react-table';
import * as Icons from 'lucide-react';
import { ArrowDown, ArrowUp, ArrowUpDown, MoreHorizontal } from 'lucide-react';
import { ArrowDown, ArrowUp, ArrowUpDown } from 'lucide-react';
import { useMemo, useState } from 'react';
import { index as categoriesIndex } from '@/actions/App/Http/Controllers/Settings/CategoryController';
@ -22,31 +20,15 @@ import { CreateCategoryDialog } from '@/components/categories/create-category-di
import { DeleteCategoryDialog } from '@/components/categories/delete-category-dialog';
import { EditCategoryDialog } from '@/components/categories/edit-category-dialog';
import HeadingSmall from '@/components/heading-small';
import {
type DialogControl,
RowActionsDropdown,
RowWithActionsContextMenu,
} from '@/components/shared/row-edit-delete-actions';
import { SettingsTable } from '@/components/shared/settings-table';
import { Badge } from '@/components/ui/badge';
import { Button } from '@/components/ui/button';
import {
ContextMenu,
ContextMenuContent,
ContextMenuItem,
ContextMenuLabel,
ContextMenuTrigger,
} from '@/components/ui/context-menu';
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuLabel,
DropdownMenuTrigger,
} from '@/components/ui/dropdown-menu';
import { Input } from '@/components/ui/input';
import {
Table,
TableBody,
TableCell,
TableHead,
TableHeader,
TableRow,
} from '@/components/ui/table';
import {
Tooltip,
TooltipContent,
@ -71,6 +53,27 @@ const breadcrumbs: BreadcrumbItem[] = [
},
];
function categoryDialogs(category: Category, categories: Category[]) {
return {
renderEditDialog: (control: DialogControl) => (
<EditCategoryDialog
category={category}
categories={categories}
onSuccess={() => {}}
{...control}
/>
),
renderDeleteDialog: (control: DialogControl) => (
<DeleteCategoryDialog
category={category}
categories={categories}
onSuccess={() => {}}
{...control}
/>
),
};
}
function CategoryActions({
category,
categories,
@ -78,49 +81,7 @@ function CategoryActions({
category: Category;
categories: Category[];
}) {
const [editOpen, setEditOpen] = useState(false);
const [deleteOpen, setDeleteOpen] = useState(false);
return (
<>
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button variant="ghost" className="h-8 w-8 p-0">
<span className="sr-only">{__('Open menu')}</span>
<MoreHorizontal className="h-4 w-4" />
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end">
<DropdownMenuLabel>{__('Actions')}</DropdownMenuLabel>
<DropdownMenuItem onClick={() => setEditOpen(true)}>
{__('Edit')}
</DropdownMenuItem>
<DropdownMenuItem
onClick={() => setDeleteOpen(true)}
variant="destructive"
>
{__('Delete')}
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
<EditCategoryDialog
category={category}
categories={categories}
open={editOpen}
onOpenChange={setEditOpen}
onSuccess={() => {}}
/>
<DeleteCategoryDialog
category={category}
categories={categories}
open={deleteOpen}
onOpenChange={setDeleteOpen}
onSuccess={() => {}}
/>
</>
);
return <RowActionsDropdown {...categoryDialogs(category, categories)} />;
}
function CategoryRow({
@ -130,66 +91,11 @@ function CategoryRow({
row: Row<Category>;
categories: Category[];
}) {
const category = row.original;
const [editOpen, setEditOpen] = useState(false);
const [deleteOpen, setDeleteOpen] = useState(false);
const [contextMenuOpen, setContextMenuOpen] = useState(false);
return (
<>
<ContextMenu onOpenChange={setContextMenuOpen}>
<ContextMenuTrigger asChild>
<TableRow
data-state={
(row.getIsSelected() || contextMenuOpen) &&
'selected'
}
>
{row
.getVisibleCells()
.map((cell: Cell<Category, unknown>) => (
<TableCell
key={cell.id}
className="align-middle"
>
{flexRender(
cell.column.columnDef.cell,
cell.getContext(),
)}
</TableCell>
))}
</TableRow>
</ContextMenuTrigger>
<ContextMenuContent>
<ContextMenuLabel>{__('Actions')}</ContextMenuLabel>
<ContextMenuItem onClick={() => setEditOpen(true)}>
{__('Edit')}
</ContextMenuItem>
<ContextMenuItem
onClick={() => setDeleteOpen(true)}
variant="destructive"
>
{__('Delete')}
</ContextMenuItem>
</ContextMenuContent>
</ContextMenu>
<EditCategoryDialog
category={category}
categories={categories}
open={editOpen}
onOpenChange={setEditOpen}
onSuccess={() => {}}
/>
<DeleteCategoryDialog
category={category}
categories={categories}
open={deleteOpen}
onOpenChange={setDeleteOpen}
onSuccess={() => {}}
/>
</>
<RowWithActionsContextMenu
row={row}
{...categoryDialogs(row.original, categories)}
/>
);
}
@ -462,59 +368,17 @@ export default function Categories() {
/>
</div>
<div className="overflow-hidden rounded-md border">
<Table>
<TableHeader>
{table
.getHeaderGroups()
.map((headerGroup) => (
<TableRow key={headerGroup.id}>
{headerGroup.headers.map(
(header) => {
return (
<TableHead
key={header.id}
>
{header.isPlaceholder
? null
: flexRender(
header
.column
.columnDef
.header,
header.getContext(),
)}
</TableHead>
);
},
)}
</TableRow>
))}
</TableHeader>
<TableBody>
{table.getRowModel().rows?.length ? (
table
.getRowModel()
.rows.map((row) => (
<CategoryRow
key={row.id}
row={row}
categories={categories}
/>
))
) : (
<TableRow>
<TableCell
colSpan={columns.length}
className="h-24 text-center align-middle"
>
{__('No categories found.')}
</TableCell>
</TableRow>
)}
</TableBody>
</Table>
</div>
<SettingsTable
table={table}
emptyMessage={__('No categories found.')}
renderRow={(row) => (
<CategoryRow
key={row.id}
row={row}
categories={categories}
/>
)}
/>
<div className="flex items-center justify-end">
<div className="text-sm text-muted-foreground">

View File

@ -1,10 +1,8 @@
import { __ } from '@/utils/i18n';
import { Head, usePage } from '@inertiajs/react';
import {
Cell,
ColumnDef,
ColumnFiltersState,
flexRender,
getCoreRowModel,
getFilteredRowModel,
getSortedRowModel,
@ -13,7 +11,7 @@ import {
useReactTable,
VisibilityState,
} from '@tanstack/react-table';
import { ArrowUpDown, MoreHorizontal, Tag } from 'lucide-react';
import { ArrowUpDown, Tag } from 'lucide-react';
import { useState } from 'react';
import { index as labelsIndex } from '@/actions/App/Http/Controllers/Settings/LabelController';
@ -21,31 +19,15 @@ import HeadingSmall from '@/components/heading-small';
import { CreateLabelDialog } from '@/components/labels/create-label-dialog';
import { DeleteLabelDialog } from '@/components/labels/delete-label-dialog';
import { EditLabelDialog } from '@/components/labels/edit-label-dialog';
import {
type DialogControl,
RowActionsDropdown,
RowWithActionsContextMenu,
} from '@/components/shared/row-edit-delete-actions';
import { SettingsTable } from '@/components/shared/settings-table';
import { Badge } from '@/components/ui/badge';
import { Button } from '@/components/ui/button';
import {
ContextMenu,
ContextMenuContent,
ContextMenuItem,
ContextMenuLabel,
ContextMenuTrigger,
} from '@/components/ui/context-menu';
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuLabel,
DropdownMenuTrigger,
} from '@/components/ui/dropdown-menu';
import { Input } from '@/components/ui/input';
import {
Table,
TableBody,
TableCell,
TableHead,
TableHeader,
TableRow,
} from '@/components/ui/table';
import AppLayout from '@/layouts/app-layout';
import SettingsLayout from '@/layouts/settings/layout';
import { type BreadcrumbItem } from '@/types';
@ -58,109 +40,28 @@ const breadcrumbs: BreadcrumbItem[] = [
},
];
function LabelActions({ label }: { label: Label }) {
const [editOpen, setEditOpen] = useState(false);
const [deleteOpen, setDeleteOpen] = useState(false);
return (
<>
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button variant="ghost" className="h-8 w-8 p-0">
<span className="sr-only">{__('Open menu')}</span>
<MoreHorizontal className="h-4 w-4" />
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end">
<DropdownMenuLabel>{__('Actions')}</DropdownMenuLabel>
<DropdownMenuItem onClick={() => setEditOpen(true)}>
{__('Edit')}
</DropdownMenuItem>
<DropdownMenuItem
onClick={() => setDeleteOpen(true)}
variant="destructive"
>
{__('Delete')}
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
<EditLabelDialog
label={label}
open={editOpen}
onOpenChange={setEditOpen}
onSuccess={() => {}}
/>
function labelDialogs(label: Label) {
return {
renderEditDialog: (control: DialogControl) => (
<EditLabelDialog label={label} onSuccess={() => {}} {...control} />
),
renderDeleteDialog: (control: DialogControl) => (
<DeleteLabelDialog
label={label}
open={deleteOpen}
onOpenChange={setDeleteOpen}
onSuccess={() => {}}
{...control}
/>
</>
);
),
};
}
function LabelActions({ label }: { label: Label }) {
return <RowActionsDropdown {...labelDialogs(label)} />;
}
function LabelRow({ row }: { row: Row<Label> }) {
const label = row.original;
const [editOpen, setEditOpen] = useState(false);
const [deleteOpen, setDeleteOpen] = useState(false);
const [contextMenuOpen, setContextMenuOpen] = useState(false);
return (
<>
<ContextMenu onOpenChange={setContextMenuOpen}>
<ContextMenuTrigger asChild>
<TableRow
data-state={
(row.getIsSelected() || contextMenuOpen) &&
'selected'
}
>
{row
.getVisibleCells()
.map((cell: Cell<Label, unknown>) => (
<TableCell
key={cell.id}
className="align-middle"
>
{flexRender(
cell.column.columnDef.cell,
cell.getContext(),
)}
</TableCell>
))}
</TableRow>
</ContextMenuTrigger>
<ContextMenuContent>
<ContextMenuLabel>{__('Actions')}</ContextMenuLabel>
<ContextMenuItem onClick={() => setEditOpen(true)}>
{__('Edit')}
</ContextMenuItem>
<ContextMenuItem
onClick={() => setDeleteOpen(true)}
variant="destructive"
>
{__('Delete')}
</ContextMenuItem>
</ContextMenuContent>
</ContextMenu>
<EditLabelDialog
label={label}
open={editOpen}
onOpenChange={setEditOpen}
onSuccess={() => {}}
/>
<DeleteLabelDialog
label={label}
open={deleteOpen}
onOpenChange={setDeleteOpen}
onSuccess={() => {}}
/>
</>
<RowWithActionsContextMenu row={row} {...labelDialogs(row.original)} />
);
}
@ -274,58 +175,13 @@ export default function Labels() {
<CreateLabelDialog onSuccess={() => {}} />
</div>
<div className="overflow-hidden rounded-md border">
<Table>
<TableHeader>
{table
.getHeaderGroups()
.map((headerGroup) => (
<TableRow key={headerGroup.id}>
{headerGroup.headers.map(
(header) => {
return (
<TableHead
key={header.id}
>
{header.isPlaceholder
? null
: flexRender(
header
.column
.columnDef
.header,
header.getContext(),
)}
</TableHead>
);
},
)}
</TableRow>
))}
</TableHeader>
<TableBody>
{table.getRowModel().rows?.length ? (
table
.getRowModel()
.rows.map((row) => (
<LabelRow
key={row.id}
row={row}
/>
))
) : (
<TableRow>
<TableCell
colSpan={columns.length}
className="h-24 text-center"
>
{__('No labels found.')}
</TableCell>
</TableRow>
)}
</TableBody>
</Table>
</div>
<SettingsTable
table={table}
emptyMessage={__('No labels found.')}
renderRow={(row) => (
<LabelRow key={row.id} row={row} />
)}
/>
<div className="flex items-center justify-end">
<div className="text-sm text-muted-foreground">