feat(ui): compare network monitor targets within the same system (#2477)

Co-authored-by: henrygd <hank@henrygd.me>
2026-10-02 06:17:47 +02:00 · 2026-09-30 01:56:56 +02:00
parent 3cc19602c1
commit e01e2a20e9
6 changed files with 516 additions and 81 deletions
--- a/internal/site/src/components/network-monitors-table/monitor-dialog.tsx
+++ b/internal/site/src/components/network-monitors-table/monitor-dialog.tsx
@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useRef, useState } from "react"
+import { type ReactNode, useCallback, useEffect, useRef, useState } from "react"
 import { Trans, useLingui } from "@lingui/react/macro"
 import { useStore } from "@nanostores/react"
 import { pb } from "@/lib/api"
@@ -23,10 +23,11 @@ import { Input } from "@/components/ui/input"
 import { Label } from "@/components/ui/label"
 import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"
 import { Textarea } from "@/components/ui/textarea"
-import { ChevronDownIcon, ListIcon, PlusIcon, SearchIcon, ServerIcon } from "lucide-react"
+import { ChevronDownIcon, GlobeIcon, ListIcon, type LucideIcon, PlusIcon, SearchIcon, ServerIcon } from "lucide-react"
 import { useToast } from "@/components/ui/use-toast"
 import { $systems } from "@/lib/stores"
 import { cn, supportsNetworkMonitors } from "@/lib/utils"
+import { getMonitorTarget } from "@/lib/network-monitor-utils"
 import type { NetworkMonitorRecord } from "@/types"
 import * as v from "valibot"

@@ -202,6 +203,7 @@ export function SystemMultiSelect({
 	className,
 	systemIds,
 	placeholder,
+	canSelectMore,
 }: {
 	id: string
 	selectedSystemIds: Set<string>
@@ -211,8 +213,107 @@ export function SystemMultiSelect({
 	/** Limit the options to these systems. Defaults to all systems that support network monitors. */
 	systemIds?: string[]
 	placeholder?: string
+	canSelectMore?: boolean
 }) {
 	const systems = useStore($systems)
+	const { t } = useLingui()
+	const options = systems
+		.filter((system) => (systemIds ? systemIds.includes(system.id) : supportsNetworkMonitors(system)))
+		.map((system) => ({ id: system.id, label: system.name }))
+	return (
+		<MultiSelect
+			id={id}
+			options={options}
+			selectedIds={selectedSystemIds}
+			onChange={onChange}
+			disabled={disabled}
+			className={className}
+			icon={ServerIcon}
+			canSelectMore={canSelectMore}
+			placeholder={placeholder ?? t`Select systems`}
+			searchPlaceholder={t`Search systems`}
+			emptyText={<Trans>No systems found.</Trans>}
+		/>
+	)
+}
+
+/** Pick monitors by target, e.g. other targets on the same system to compare against. */
+export function MonitorMultiSelect({
+	id,
+	monitors,
+	selectedMonitorIds,
+	onChange,
+	disabled,
+	className,
+	placeholder,
+	canSelectMore,
+}: {
+	id: string
+	monitors: NetworkMonitorRecord[]
+	selectedMonitorIds: Set<string>
+	onChange: (ids: Set<string>) => void
+	disabled?: boolean
+	className?: string
+	placeholder?: string
+	canSelectMore?: boolean
+}) {
+	const { t } = useLingui()
+	const options = monitors
+		.map((monitor) => ({ id: monitor.id, label: getMonitorTarget(monitor), server: monitor.server }))
+		.sort((a, b) => a.label.localeCompare(b.label))
+	return (
+		<MultiSelect
+			id={id}
+			options={options}
+			selectedIds={selectedMonitorIds}
+			onChange={onChange}
+			disabled={disabled}
+			className={cn("ps-9.5", className)}
+			icon={GlobeIcon}
+			canSelectMore={canSelectMore}
+			placeholder={placeholder ?? t`Select targets`}
+			searchPlaceholder={t`Search targets`}
+			emptyText={<Trans>No targets found.</Trans>}
+			renderOption={(option) => (
+				<>
+					<span className="truncate">{option.label}</span>
+					{option.server && <span className="ms-auto shrink-0 text-xs text-muted-foreground">{option.server}</span>}
+				</>
+			)}
+		/>
+	)
+}
+
+type MultiSelectOption = { id: string; label: string }
+
+function MultiSelect<T extends MultiSelectOption>({
+	id,
+	options,
+	selectedIds,
+	onChange,
+	disabled,
+	className,
+	icon: Icon,
+	placeholder,
+	searchPlaceholder,
+	emptyText,
+	renderOption = (option) => <span className="truncate">{option.label}</span>,
+	canSelectMore = true,
+}: {
+	id: string
+	options: T[]
+	selectedIds: Set<string>
+	onChange: (ids: Set<string>) => void
+	disabled?: boolean
+	className?: string
+	icon: LucideIcon
+	placeholder: string
+	searchPlaceholder: string
+	emptyText: ReactNode
+	renderOption?: (option: T) => ReactNode
+	/** False once the selection is full; only already selected options can then be toggled. */
+	canSelectMore?: boolean
+}) {
 	const { t } = useLingui()
 	const [search, setSearch] = useState("")
 	const searchRef = useRef<HTMLInputElement>(null)
@@ -225,19 +326,15 @@ export function SystemMultiSelect({
 	}, [])
 	const contentRef = useRef<HTMLDivElement>(null)
 	const query = search.trim().toLocaleLowerCase()
-	const filteredSystems = systems.filter(
-		(system) =>
-			(systemIds ? systemIds.includes(system.id) : supportsNetworkMonitors(system)) &&
-			system.name.toLocaleLowerCase().includes(query)
-	)
-	const allSelected = filteredSystems.every((system) => selectedSystemIds.has(system.id))
-	const anySelected = filteredSystems.some((system) => selectedSystemIds.has(system.id))
+	const filteredOptions = options.filter((option) => option.label.toLocaleLowerCase().includes(query))
+	const allSelected = filteredOptions.every((option) => selectedIds.has(option.id))
+	const anySelected = filteredOptions.some((option) => selectedIds.has(option.id))

 	const selectFiltered = (selected: boolean) => {
-		const next = new Set(selectedSystemIds)
-		for (const system of filteredSystems) {
-			if (selected) next.add(system.id)
-			else next.delete(system.id)
+		const next = new Set(selectedIds)
+		for (const option of filteredOptions) {
+			if (selected) next.add(option.id)
+			else next.delete(option.id)
 		}
 		onChange(next)
 	}
@@ -251,13 +348,13 @@ export function SystemMultiSelect({
 					variant="outline"
 					className={cn("relative w-full min-w-0 ps-10 pe-10 justify-start font-normal text-start", className)}
 				>
-					<ServerIcon className="size-3.5 absolute start-4 top-1/2 -translate-y-1/2 opacity-85" />
+					<Icon className="size-3.5 absolute start-4 top-1/2 -translate-y-1/2 opacity-85" />
 					<span className="truncate">
-						{selectedSystemIds.size === 0
-							? (placeholder ?? t`Select systems`)
-							: selectedSystemIds.size === 1
-								? systems.find((s) => selectedSystemIds.has(s.id))?.name
-								: t`${selectedSystemIds.size} selected`}
+						{selectedIds.size === 0
+							? placeholder
+							: selectedIds.size === 1
+								? options.find((option) => selectedIds.has(option.id))?.label
+								: t`${selectedIds.size} selected`}
 					</span>
 					<ChevronDownIcon className="size-4 absolute end-4 top-1/2 -translate-y-1/2 opacity-50" />
 				</Button>
@@ -280,8 +377,8 @@ export function SystemMultiSelect({
 							ref={focusSearchOnMount}
 							value={search}
 							onChange={(event) => setSearch(event.target.value)}
-							placeholder={t`Search systems`}
-							aria-label={t`Search systems`}
+							placeholder={searchPlaceholder}
+							aria-label={searchPlaceholder}
 							className="h-10 min-w-0 rounded-none border-0 bg-transparent px-0 shadow-none focus-visible:ring-0 focus-visible:ring-offset-0"
 							onKeyDown={(event) => {
 								if (event.key === "Escape") return
@@ -303,7 +400,7 @@ export function SystemMultiSelect({
 						<div className="flex items-center">
 							<DropdownMenuItem
 								className="px-1.5 py-1 text-xs text-muted-foreground"
-								disabled={!filteredSystems.length || allSelected}
+								disabled={!filteredOptions.length || allSelected || !canSelectMore}
 								onSelect={(event) => {
 									event.preventDefault()
 									selectFiltered(true)
@@ -325,32 +422,29 @@ export function SystemMultiSelect({
 								{query ? <Trans>Clear matches</Trans> : <Trans>Clear all</Trans>}
 							</DropdownMenuItem>
 						</div>
-						<span className="px-1.5 text-xs tabular-nums text-muted-foreground">
-							{t`${selectedSystemIds.size} selected`}
-						</span>
+						<span className="px-1.5 text-xs tabular-nums text-muted-foreground">{t`${selectedIds.size} selected`}</span>
 					</div>
 				</div>
 				<div className="min-h-0 overflow-y-auto">
-					{filteredSystems.length === 0 && (
-						<output className="block px-2.5 py-3 text-sm text-muted-foreground">
-							<Trans>No systems found.</Trans>
-						</output>
+					{filteredOptions.length === 0 && (
+						<output className="block px-2.5 py-3 text-sm text-muted-foreground">{emptyText}</output>
 					)}
-					{filteredSystems.map((sys) => (
+					{filteredOptions.map((option) => (
 						<DropdownMenuCheckboxItem
-							key={sys.id}
-							checked={selectedSystemIds.has(sys.id)}
+							key={option.id}
+							checked={selectedIds.has(option.id)}
+							disabled={!canSelectMore && !selectedIds.has(option.id)}
 							onSelect={(event) => event.preventDefault()}
 							onCheckedChange={(checked) => {
-								const next = new Set(selectedSystemIds)
-								if (checked) next.add(sys.id)
-								else next.delete(sys.id)
+								const next = new Set(selectedIds)
+								if (checked) next.add(option.id)
+								else next.delete(option.id)
 								onChange(next)
 							}}
 							className="group min-w-0 gap-2.5 py-2 ps-2.5"
 							indicatorClassName="static size-4 shrink-0 rounded border border-input group-data-[state=checked]:border-primary group-data-[state=checked]:bg-primary group-data-[state=checked]:text-primary-foreground [&_svg]:size-3"
 						>
-							<span className="truncate">{sys.name}</span>
+							{renderOption(option)}
 						</DropdownMenuCheckboxItem>
 					))}
 				</div>
--- a/internal/site/src/components/network-monitors-table/network-monitors-table.tsx
+++ b/internal/site/src/components/network-monitors-table/network-monitors-table.tsx
@@ -39,7 +39,7 @@ import { SystemStatus } from "@/lib/enums"
 import { $allSystemsById, $direction, $textMeasureVersion, $userSettings, getUserChartTime } from "@/lib/stores"
 import { cn, formatShortDate, isVisuallyLonger, matchesFilterGroups, parseFilterGroups, parseSemVer } from "@/lib/utils"
 import type { ChartOptions, MonitorCertInfo, NetworkMonitorRecord } from "@/types"
-import { AddMonitorDialog, EditMonitorDialog, SystemMultiSelect } from "./monitor-dialog"
+import { AddMonitorDialog, EditMonitorDialog, MonitorMultiSelect, SystemMultiSelect } from "./monitor-dialog"
 import {
 	ArrowDownIcon,
 	ArrowLeftRightIcon,
@@ -67,7 +67,8 @@ import {
 import { Sheet, SheetContent, SheetDescription, SheetHeader, SheetTitle } from "@/components/ui/sheet"
 import ChartTimeSelect from "@/components/charts/chart-time-select"
 import { LossChart, AvgMinMaxResponseChart, ResponseChart } from "@/components/routes/system/charts/monitors-charts"
-import { useMatchingMonitors, useNetworkMonitorStats } from "@/lib/use-network-monitors"
+import { getMonitorCompareState } from "@/lib/monitor-compare"
+import { useCompareMonitors, useNetworkMonitorStats } from "@/lib/use-network-monitors"
 import { useStore } from "@nanostores/react"
 import { atom } from "nanostores"
 import { Separator } from "../ui/separator"
@@ -470,6 +471,7 @@ export default function NetworkMonitorsTableNew({
 					visibleColumnsKey={visibleColumnsKey}
 					rowSelection={rowSelection}
 					isLoading={isLoading}
+					includesAllSystems={!systemId}
 				/>
 			</div>
 		</Card>
@@ -483,6 +485,7 @@ const NetworkMonitorsTable = memo(function NetworkMonitorTable({
 	visibleColumnsKey,
 	rowSelection,
 	isLoading,
+	includesAllSystems,
 }: {
 	table: TableType<NetworkMonitorRecord>
 	rows: Row<NetworkMonitorRecord>[]
@@ -490,6 +493,8 @@ const NetworkMonitorsTable = memo(function NetworkMonitorTable({
 	visibleColumnsKey: string
 	rowSelection: RowSelectionState
 	isLoading: boolean
+	/** The table lists every system's monitors, so the sheet can compare without fetching. */
+	includesAllSystems: boolean
 }) {
 	const scrollRef = useRef<HTMLDivElement>(null)
 	const [sheetOpen, setSheetOpen] = useState(false)
@@ -560,6 +565,8 @@ const NetworkMonitorsTable = memo(function NetworkMonitorTable({
 					setSheetOpen(nextOpen)
 				}}
 				monitor={activeMonitor}
+				monitors={table.options.data}
+				includesAllSystems={includesAllSystems}
 			/>
 		</div>
 	)
@@ -630,16 +637,29 @@ function NetworkMonitorSheet({
 	open,
 	onOpenChange,
 	monitor,
+	monitors,
+	includesAllSystems,
 }: {
 	open: boolean
 	onOpenChange: (open: boolean) => void
 	monitor?: NetworkMonitorRecord
+	monitors: NetworkMonitorRecord[]
+	includesAllSystems: boolean
 }) {
 	if (!monitor) {
 		return null
 	}

-	return <NetworkMonitorSheetContent key={monitor.system} open={open} onOpenChange={onOpenChange} monitor={monitor} />
+	return (
+		<NetworkMonitorSheetContent
+			key={monitor.system}
+			open={open}
+			onOpenChange={onOpenChange}
+			monitor={monitor}
+			monitors={monitors}
+			includesAllSystems={includesAllSystems}
+		/>
+	)
 }

 const certExpiryTextColors = { ok: "", warning: "text-yellow-600 dark:text-yellow-500", critical: "text-red-500" }
@@ -676,10 +696,16 @@ function NetworkMonitorSheetContent({
 	open,
 	onOpenChange,
 	monitor,
+	monitors,
+	includesAllSystems,
 }: {
 	open: boolean
 	onOpenChange: (open: boolean) => void
 	monitor: NetworkMonitorRecord
+	/** Table monitors; used to find other targets on the same system to compare against. */
+	monitors: NetworkMonitorRecord[]
+	/** Whether `monitors` covers every system, so other systems' monitors needn't be fetched. */
+	includesAllSystems: boolean
 }) {
 	// Keep monitor exploration independent of the system charts' time range.
 	const [chartTimeStore] = useState(() => {
@@ -691,8 +717,7 @@ function NetworkMonitorSheetContent({
 	const systems = useStore($allSystemsById)
 	const system = systems[monitor.system]

-	// Same target probed from other systems, for side-by-side comparison (#2385).
-	const matchingMonitors = useMatchingMonitors(monitor, open)
+	const [compareTargetIds, setCompareTargetIds] = useState<Set<string>>(() => new Set())
 	const [compareSystemIds, setCompareSystemIds] = useState<Set<string>>(() => new Set())
 	// Scoped to this sheet so a filter doesn't carry over to other monitors' sheets.
 	const [compareFilterStore, setCompareFilterStore] = useState(() => atom(""))
@@ -701,16 +726,25 @@ function NetworkMonitorSheetContent({
 	if (compareMonitorId !== monitor.id) {
 		setCompareMonitorId(monitor.id)
 		setCompareSystemIds(new Set())
+		setCompareTargetIds(new Set())
 		setCompareFilterStore(atom(""))
 	}
-	const matchingSystemIds = useMemo(() => matchingMonitors.map((m) => m.system), [matchingMonitors])
-	// The opened system is always charted; the picker only adds other systems to compare against.
-	const compareMonitors = useMemo(
-		() => [monitor, ...matchingMonitors.filter((m) => compareSystemIds.has(m.system))],
-		[monitor, matchingMonitors, compareSystemIds]
+	// Other systems' monitors come from the table when it lists every system, otherwise from one fetch.
+	const fetchedMonitors = useCompareMonitors(monitor.system, monitor.protocol, open && !includesAllSystems)
+	const compare = useMemo(
+		() =>
+			getMonitorCompareState({
+				monitor,
+				localMonitors: monitors,
+				otherMonitors: includesAllSystems ? monitors : fetchedMonitors,
+				selectedSystemIds: compareSystemIds,
+				selectedTargetIds: compareTargetIds,
+				getSystemName: (id) => systems[id]?.name ?? id,
+			}),
+		[monitor, monitors, includesAllSystems, fetchedMonitors, compareSystemIds, compareTargetIds, systems]
 	)
+	const { compareMonitors } = compare
 	const comparing = compareMonitors.length > 1
-	const getSystemName = useCallback((m: NetworkMonitorRecord) => systems[m.system]?.name ?? m.system, [systems])

 	const monitorStats = useNetworkMonitorStats({
 		systemId: monitor.system,
@@ -766,21 +800,31 @@ function NetworkMonitorSheetContent({
 				<div className="grid gap-4">
 					<div className="flex flex-wrap items-center gap-2">
 						<ChartTimeSelect
-							className="bg-card flex-1 basis-48"
+							className="bg-card flex-1 min-w-0 basis-full sm:basis-0"
 							agentVersion={chartData.agentVersion}
 							chartTimeStore={chartTimeStore}
 							allowRealtime={false}
 						/>
-						{matchingMonitors.length > 0 && (
-							<SystemMultiSelect
-								id="monitor-compare-systems"
-								className="w-full sm:w-1/3 shrink-0 bg-card"
-								systemIds={matchingSystemIds}
-								selectedSystemIds={compareSystemIds}
-								onChange={setCompareSystemIds}
-								placeholder={t`Compare with other systems`}
-							/>
-						)}
+						<MonitorMultiSelect
+							id="monitor-compare-targets"
+							className="flex-1 min-w-0 basis-full sm:basis-0 bg-card"
+							monitors={compare.targetOptions}
+							selectedMonitorIds={compare.selectedTargetIds}
+							onChange={setCompareTargetIds}
+							disabled={compare.targetOptions.length === 0}
+							canSelectMore={compare.canAddTarget}
+							placeholder={t`Compare with other targets`}
+						/>
+						<SystemMultiSelect
+							id="monitor-compare-systems"
+							className="flex-1 min-w-0 basis-full sm:basis-0 bg-card"
+							systemIds={compare.systemOptions}
+							selectedSystemIds={compare.selectedSystemIds}
+							onChange={setCompareSystemIds}
+							disabled={compare.systemOptions.length === 0}
+							canSelectMore={compare.canAddSystem}
+							placeholder={t`Compare with other systems`}
+						/>
 					</div>
 					{comparing ? (
 						<>
@@ -790,7 +834,7 @@ function NetworkMonitorSheetContent({
 								monitors={compareMonitors}
 								chartData={chartData}
 								empty={!hasMonitorStats}
-								getLabel={getSystemName}
+								getLabel={compare.getLabel}
 								filterStore={compareFilterStore}
 							/>
 							<LossChart
@@ -799,7 +843,7 @@ function NetworkMonitorSheetContent({
 								monitors={compareMonitors}
 								chartData={chartData}
 								empty={!hasMonitorStats}
-								getLabel={getSystemName}
+								getLabel={compare.getLabel}
 								filterStore={compareFilterStore}
 							/>
 						</>