From 387cb35494406414979db3ce29d984fa2df03d61 Mon Sep 17 00:00:00 2001 From: harsh-aghara Date: Tue, 5 May 2026 17:06:48 +0530 Subject: [PATCH] refactor(ui): simplify table selection and clarify mobile label rendering Addresses PR feedback: - Removes over-abstracted useTableSelection hook, inlines selection logic - Replaces triple-condition with explicit hideMobileLabel bool flag - Removes unsafe Record casting Fixes #3602 --- .../src/Components/design-elements/Table.tsx | 7 +- client/src/Hooks/useTableSelection.ts | 74 ------------------- .../Monitors/Components/MonitorsTable.tsx | 29 +++++--- .../Components/UptimeMonitorsTable.tsx | 29 +++++--- 4 files changed, 41 insertions(+), 98 deletions(-) delete mode 100644 client/src/Hooks/useTableSelection.ts diff --git a/client/src/Components/design-elements/Table.tsx b/client/src/Components/design-elements/Table.tsx index c6628a611..a02f9be8f 100644 --- a/client/src/Components/design-elements/Table.tsx +++ b/client/src/Components/design-elements/Table.tsx @@ -36,6 +36,7 @@ export type Header = { id: number | string; content: React.ReactNode; mobileLabel?: React.ReactNode; + hideMobileLabel?: boolean; align?: "left" | "center" | "right" | "justify" | "inherit"; onClick?: (event: React.MouseEvent, row: T) => void; render: (row: T) => React.ReactNode; @@ -111,7 +112,7 @@ export function DataTable< key={key} > {headers.map((header) => { - if (header.mobileLabel === null) { + if (header.hideMobileLabel) { return ( - {header.mobileLabel !== undefined - ? header.mobileLabel - : header.content} + {header.mobileLabel ?? header.content} void; - handleSelectRow: (itemId: string, checked: boolean) => void; - isRowSelected: (itemId: string) => boolean; -} - -export const useTableSelection = ( - items: SelectableItem[], - selectedIds: string[], - onSelectionChange?: (selected: string[]) => void -): UseTableSelectionReturn => { - const selectedSet = useMemo(() => new Set(selectedIds), [selectedIds]); - - const isAllSelected = useMemo( - () => - items.length > 0 && - items.every((item) => selectedSet.has(item.id)) && - selectedIds.length >= items.length, - [items, selectedSet, selectedIds.length] - ); - - const isSomeSelected = useMemo( - () => selectedIds.length > 0 && !isAllSelected, - [selectedIds.length, isAllSelected] - ); - - const handleSelectAll = useCallback( - (checked: boolean) => { - if (onSelectionChange) { - if (checked) { - const allIds = items.map((item) => item.id); - onSelectionChange(allIds); - } else { - onSelectionChange([]); - } - } - }, - [items, onSelectionChange] - ); - - const handleSelectRow = useCallback( - (itemId: string, checked: boolean) => { - if (onSelectionChange) { - if (checked) { - onSelectionChange([...selectedIds, itemId]); - } else { - onSelectionChange(selectedIds.filter((id) => id !== itemId)); - } - } - }, - [selectedIds, onSelectionChange] - ); - - const isRowSelected = useCallback( - (itemId: string) => selectedSet.has(itemId), - [selectedSet] - ); - - return { - isAllSelected, - isSomeSelected, - handleSelectAll, - handleSelectRow, - isRowSelected, - }; -}; diff --git a/client/src/Pages/Infrastructure/Monitors/Components/MonitorsTable.tsx b/client/src/Pages/Infrastructure/Monitors/Components/MonitorsTable.tsx index dbae1b1e9..e6458ea34 100644 --- a/client/src/Pages/Infrastructure/Monitors/Components/MonitorsTable.tsx +++ b/client/src/Pages/Infrastructure/Monitors/Components/MonitorsTable.tsx @@ -14,7 +14,6 @@ import { usePost } from "@/Hooks/UseApi"; import type { Monitor } from "@/Types/Monitor"; import { Checkbox } from "@/Components/inputs"; -import { useTableSelection } from "@/Hooks/useTableSelection"; interface InfraMonitorsTableProps { monitors: Monitor[]; @@ -55,13 +54,23 @@ export const InfraMonitorsTable = ({ const navigate = useNavigate(); const { post } = usePost, Monitor>(); - const { - isAllSelected, - isSomeSelected, - handleSelectAll, - handleSelectRow, - isRowSelected, - } = useTableSelection(monitors, selectedRows, onSelectionChange); + const selectedSet = new Set(selectedRows); + const isAllSelected = + monitors.length > 0 && monitors.every((m) => selectedSet.has(m.id)); + const isSomeSelected = selectedRows.length > 0 && !isAllSelected; + + const handleSelectAll = (checked: boolean) => { + onSelectionChange?.(checked ? monitors.map((m) => m.id) : []); + }; + + const handleSelectRow = (id: string, checked: boolean) => { + if (!onSelectionChange) return; + onSelectionChange( + checked ? [...selectedRows, id] : selectedRows.filter((rowId) => rowId !== id) + ); + }; + + const isRowSelected = (id: string) => selectedSet.has(id); const handlePageChange = ( _e: React.MouseEvent | null, @@ -121,7 +130,7 @@ export const InfraMonitorsTable = ({ ? t("common.buttons.resume") : t("common.buttons.pause"), action: async () => { - await post(`/monitors/pause/${monitor.id}`, {} as Record); + await post(`/monitors/pause/${monitor.id}`, {}); refetch(); }, closeMenu: true, @@ -168,7 +177,7 @@ export const InfraMonitorsTable = ({ onChange={(e) => handleSelectAll(e.target.checked)} /> ), - mobileLabel: null, + hideMobileLabel: true, render: (row) => ( state.ui?.chartType ?? "histogram"); const { post } = usePost, Monitor>(); - const { - isAllSelected, - isSomeSelected, - handleSelectAll, - handleSelectRow, - isRowSelected, - } = useTableSelection(monitors, selectedRows, onSelectionChange); + const selectedSet = new Set(selectedRows); + const isAllSelected = + monitors.length > 0 && monitors.every((m) => selectedSet.has(m.id)); + const isSomeSelected = selectedRows.length > 0 && !isAllSelected; + + const handleSelectAll = (checked: boolean) => { + onSelectionChange?.(checked ? monitors.map((m) => m.id) : []); + }; + + const handleSelectRow = (id: string, checked: boolean) => { + if (!onSelectionChange) return; + onSelectionChange( + checked ? [...selectedRows, id] : selectedRows.filter((rowId) => rowId !== id) + ); + }; + + const isRowSelected = (id: string) => selectedSet.has(id); const handleSort = (e: React.MouseEvent, field: string) => { e.preventDefault(); @@ -129,7 +138,7 @@ export const MonitorTable = ({ ? t("pages.common.monitors.actions.resume") : t("pages.common.monitors.actions.pause"), action: async () => { - await post(`/monitors/pause/${monitor.id}`, {} as Record); + await post(`/monitors/pause/${monitor.id}`, {}); refetch(); }, closeMenu: true, @@ -175,7 +184,7 @@ export const MonitorTable = ({ onChange={(e) => handleSelectAll(e.target.checked)} /> ), - mobileLabel: null, + hideMobileLabel: true, render: (row) => (