From e3fd7898f69c1b6ca2ea81e5c3fc0580f16541e8 Mon Sep 17 00:00:00 2001 From: Josh Theisen Date: Mon, 26 Oct 2026 01:51:41 -0500 Subject: [PATCH] =?UTF-8?q?fix(user-notifications):=20simplify=20row=20key?= =?UTF-8?q?=20serialization=20in=20TableField=20and=20clean=20up=20UserNo?= =?UTF-8?q?=E2=80=A6=20(#1583)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - simplify row key serialization in TableField - clean up userNotificationOverridesSection Previously, after typing a single character, the browser would lose focus due to the row keys changes on every new value. The fix makes the row key independent of the URL’s changing text. Additionally, fixed the per-user notification routes so that unchecking "All" clears all event selections. --- .../components/settings/fields/TableField.tsx | 61 +++++++++---------- .../UserNotificationOverridesSection.tsx | 4 -- src/frontend/src/tests/tableField.test.ts | 28 +++++++++ 3 files changed, 57 insertions(+), 36 deletions(-) create mode 100644 src/frontend/src/tests/tableField.test.ts diff --git a/src/frontend/src/components/settings/fields/TableField.tsx b/src/frontend/src/components/settings/fields/TableField.tsx index 4f47191e..d5d33801 100644 --- a/src/frontend/src/components/settings/fields/TableField.tsx +++ b/src/frontend/src/components/settings/fields/TableField.tsx @@ -1,6 +1,7 @@ import type { CSSProperties } from 'react'; -import { useMemo } from 'react'; +import { useMemo, useRef, useState } from 'react'; +import { useDependencyEffect } from '../../../hooks/useMountEffect'; import type { MultiSelectFieldConfig, TableFieldConfig, @@ -61,6 +62,12 @@ function normalizeMultiValue(value: unknown): string[] { return []; } +export function buildRowEntries(rows: Record[], rowIds: string[]) { + return rows.map((row, index) => { + return { row, key: rowIds[index] ?? `untracked-${index}` }; + }); +} + function normalizeRows( rows: Record[], columns: TableFieldColumn[], @@ -147,46 +154,33 @@ function getFilteredSelectOptions( return options.filter((opt) => !opt.childOf || opt.childOf === filterValue); } -function serializeRowKeyValue(value: unknown): string { - if (value === null || value === undefined) { - return ''; - } - if (typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean') { - return String(value); - } - if (Array.isArray(value)) { - return value.map((entry) => serializeRowKeyValue(entry)).join('|'); - } - - try { - return JSON.stringify(value) ?? ''; - } catch { - return '[unserializable]'; - } -} - export const TableField = ({ field, value, onChange, disabled }: TableFieldProps) => { const isDisabled = disabled ?? false; + const [rowIds, setRowIds] = useState(() => (value ?? []).map((_, index) => `row-${index}`)); + const nextRowIdRef = useRef(rowIds.length); + const valueLength = (value ?? []).length; const columns = useMemo(() => field.columns ?? [], [field.columns]); const rows = useMemo(() => normalizeTableRows(value ?? [], columns), [value, columns]); - const rowEntries = useMemo(() => { - const rowOccurrences = new Map(); + const rowEntries = useMemo(() => buildRowEntries(rows, rowIds), [rows, rowIds]); - return rows.map((row) => { - const baseKey = columns - .map((col) => `${col.key}:${serializeRowKeyValue(row[col.key])}`) - .join('||'); - const occurrence = rowOccurrences.get(baseKey) ?? 0; + useDependencyEffect(() => { + setRowIds((currentRowIds) => { + if (valueLength > currentRowIds.length) { + const newIds = Array.from( + { length: valueLength - currentRowIds.length }, + () => `row-${nextRowIdRef.current++}`, + ); + return [...currentRowIds, ...newIds]; + } - rowOccurrences.set(baseKey, occurrence + 1); + if (valueLength < currentRowIds.length) { + return currentRowIds.slice(0, valueLength); + } - return { - row, - key: `${baseKey}::${occurrence}`, - }; + return currentRowIds; }); - }, [rows, columns]); + }, [valueLength]); // Use minmax(0, ...) so the grid can shrink inside the settings modal. // Use fixed width for delete button column to ensure header/data alignment. @@ -216,11 +210,14 @@ export const TableField = ({ field, value, onChange, disabled }: TableFieldProps columns.forEach((col) => { newRow[col.key] = defaultCellValue(col); }); + const newRowId = `row-${nextRowIdRef.current++}`; + setRowIds((current) => [...current, newRowId]); commitRows([...rows, newRow]); }; const removeRow = (rowIndex: number) => { const next = rows.filter((_, idx) => idx !== rowIndex); + setRowIds((current) => current.filter((_, idx) => idx !== rowIndex)); commitRows(next); }; diff --git a/src/frontend/src/components/settings/users/UserNotificationOverridesSection.tsx b/src/frontend/src/components/settings/users/UserNotificationOverridesSection.tsx index 8bddce13..28cf47a7 100644 --- a/src/frontend/src/components/settings/users/UserNotificationOverridesSection.tsx +++ b/src/frontend/src/components/settings/users/UserNotificationOverridesSection.tsx @@ -127,10 +127,6 @@ function normalizeRoutesValue(value: unknown): Array> { } const events = normalizeRouteEvents(row.event); - if (events.length === 0) { - return; - } - const url = toTrimmedTextValue(row.url); const key = `${events.join('|')}::${url}`; if (seen.has(key)) { diff --git a/src/frontend/src/tests/tableField.test.ts b/src/frontend/src/tests/tableField.test.ts new file mode 100644 index 00000000..03ee3a57 --- /dev/null +++ b/src/frontend/src/tests/tableField.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest'; + +import { buildRowEntries } from '../components/settings/fields/TableField'; + +describe('TableField row entries', () => { + it('keeps keys stable when controlled row values are replaced', () => { + const rowIds = ['row-0']; + + const [firstEntry] = buildRowEntries([{ url: 'https://example.com' }], rowIds); + const [secondEntry] = buildRowEntries([{ url: 'https://example.com/changed' }], rowIds); + + expect(secondEntry.key).toBe(firstEntry.key); + }); + + it('keeps remaining row keys when a row is removed', () => { + const rowIds = ['row-0', 'row-1']; + + const initialEntries = buildRowEntries( + [{ url: 'https://one.test' }, { url: 'https://two.test' }], + rowIds, + ); + const remainingIds = rowIds.filter((_, index) => index !== 0); + const [remainingEntry] = buildRowEntries([{ url: 'https://two.test' }], remainingIds); + + expect(initialEntries[0].key).not.toBe(initialEntries[1].key); + expect(remainingEntry.key).toBe(initialEntries[1].key); + }); +});