From 8d58e0bf81f2804b0ca494223eb0917680d5df4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Falc=C3=B3n?= Date: Tue, 11 Aug 2026 14:37:23 +0200 Subject: [PATCH] refactor(settings): dedupe the categories and labels pages (#770) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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`. --- .../shared/row-edit-delete-actions.tsx | 141 +++++++++++ .../js/components/shared/settings-table.tsx | 68 ++++++ resources/js/pages/settings/categories.tsx | 224 ++++-------------- resources/js/pages/settings/labels.tsx | 200 +++------------- 4 files changed, 281 insertions(+), 352 deletions(-) create mode 100644 resources/js/components/shared/row-edit-delete-actions.tsx create mode 100644 resources/js/components/shared/settings-table.tsx diff --git a/resources/js/components/shared/row-edit-delete-actions.tsx b/resources/js/components/shared/row-edit-delete-actions.tsx new file mode 100644 index 00000000..3c088bd3 --- /dev/null +++ b/resources/js/components/shared/row-edit-delete-actions.tsx @@ -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 ( + <> + + + + + + {__('Actions')} + setEditOpen(true)}> + {__('Edit')} + + setDeleteOpen(true)} + variant="destructive" + > + {__('Delete')} + + + + + {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({ + row, + renderEditDialog, + renderDeleteDialog, +}: { row: Row } & EditDeleteDialogs) { + const [editOpen, setEditOpen] = useState(false); + const [deleteOpen, setDeleteOpen] = useState(false); + const [contextMenuOpen, setContextMenuOpen] = useState(false); + + return ( + <> + + + + {row + .getVisibleCells() + .map((cell: Cell) => ( + + {flexRender( + cell.column.columnDef.cell, + cell.getContext(), + )} + + ))} + + + + {__('Actions')} + setEditOpen(true)}> + {__('Edit')} + + setDeleteOpen(true)} + variant="destructive" + > + {__('Delete')} + + + + + {renderEditDialog({ open: editOpen, onOpenChange: setEditOpen })} + {renderDeleteDialog({ + open: deleteOpen, + onOpenChange: setDeleteOpen, + })} + + ); +} diff --git a/resources/js/components/shared/settings-table.tsx b/resources/js/components/shared/settings-table.tsx new file mode 100644 index 00000000..b8b12038 --- /dev/null +++ b/resources/js/components/shared/settings-table.tsx @@ -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({ + table, + emptyMessage, + renderRow, +}: { + table: TableType; + emptyMessage: string; + renderRow: (row: Row) => ReactNode; +}) { + const rows = table.getRowModel().rows; + + return ( +
+ + + {table.getHeaderGroups().map((headerGroup) => ( + + {headerGroup.headers.map((header) => ( + + {header.isPlaceholder + ? null + : flexRender( + header.column.columnDef.header, + header.getContext(), + )} + + ))} + + ))} + + + {rows.length ? ( + rows.map(renderRow) + ) : ( + + + {emptyMessage} + + + )} + +
+
+ ); +} diff --git a/resources/js/pages/settings/categories.tsx b/resources/js/pages/settings/categories.tsx index 6730f09a..32470759 100644 --- a/resources/js/pages/settings/categories.tsx +++ b/resources/js/pages/settings/categories.tsx @@ -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) => ( + {}} + {...control} + /> + ), + renderDeleteDialog: (control: DialogControl) => ( + {}} + {...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 ( - <> - - - - - - {__('Actions')} - setEditOpen(true)}> - {__('Edit')} - - setDeleteOpen(true)} - variant="destructive" - > - {__('Delete')} - - - - - {}} - /> - - {}} - /> - - ); + return ; } function CategoryRow({ @@ -130,66 +91,11 @@ function CategoryRow({ row: Row; categories: Category[]; }) { - const category = row.original; - const [editOpen, setEditOpen] = useState(false); - const [deleteOpen, setDeleteOpen] = useState(false); - const [contextMenuOpen, setContextMenuOpen] = useState(false); - return ( - <> - - - - {row - .getVisibleCells() - .map((cell: Cell) => ( - - {flexRender( - cell.column.columnDef.cell, - cell.getContext(), - )} - - ))} - - - - {__('Actions')} - setEditOpen(true)}> - {__('Edit')} - - setDeleteOpen(true)} - variant="destructive" - > - {__('Delete')} - - - - - {}} - /> - - {}} - /> - + ); } @@ -462,59 +368,17 @@ export default function Categories() { /> -
- - - {table - .getHeaderGroups() - .map((headerGroup) => ( - - {headerGroup.headers.map( - (header) => { - return ( - - {header.isPlaceholder - ? null - : flexRender( - header - .column - .columnDef - .header, - header.getContext(), - )} - - ); - }, - )} - - ))} - - - {table.getRowModel().rows?.length ? ( - table - .getRowModel() - .rows.map((row) => ( - - )) - ) : ( - - - {__('No categories found.')} - - - )} - -
-
+ ( + + )} + />
diff --git a/resources/js/pages/settings/labels.tsx b/resources/js/pages/settings/labels.tsx index 75d6f65f..862902c5 100644 --- a/resources/js/pages/settings/labels.tsx +++ b/resources/js/pages/settings/labels.tsx @@ -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 ( - <> - - - - - - {__('Actions')} - setEditOpen(true)}> - {__('Edit')} - - setDeleteOpen(true)} - variant="destructive" - > - {__('Delete')} - - - - - {}} - /> - +function labelDialogs(label: Label) { + return { + renderEditDialog: (control: DialogControl) => ( + {}} {...control} /> + ), + renderDeleteDialog: (control: DialogControl) => ( {}} + {...control} /> - - ); + ), + }; +} + +function LabelActions({ label }: { label: Label }) { + return ; } function LabelRow({ row }: { row: Row
-
- - - {table - .getHeaderGroups() - .map((headerGroup) => ( - - {headerGroup.headers.map( - (header) => { - return ( - - {header.isPlaceholder - ? null - : flexRender( - header - .column - .columnDef - .header, - header.getContext(), - )} - - ); - }, - )} - - ))} - - - {table.getRowModel().rows?.length ? ( - table - .getRowModel() - .rows.map((row) => ( - - )) - ) : ( - - - {__('No labels found.')} - - - )} - -
-
+ ( + + )} + />