Replace index keys with stable keys in editable list components
This commit is contained in:
@@ -375,7 +375,11 @@ const source = timeRange === "24h" ? "fail2ban" : "archive";
|
||||
|
||||
---
|
||||
|
||||
### TASK-019 — Replace index keys with stable keys in editable lists
|
||||
### TASK-019 — Replace index keys with stable keys in editable lists (done)
|
||||
|
||||
**Where fixed:** `frontend/src/components/config/StringListEditor.tsx`, `frontend/src/components/config/RegexList.tsx`, `frontend/src/components/config/JailsTab.tsx`, `frontend/src/components/config/stableListEntries.ts`
|
||||
|
||||
**Summary:** Added stable internal IDs for editable string lists, replaced index-based React keys with stable entry IDs, and updated the jail log path list to use the path value as its key.
|
||||
|
||||
**Where found:** 19 instances identified, including:
|
||||
- `frontend/src/components/config/StringListEditor.tsx` line 34 — `key={index}` on editable `Input` rows.
|
||||
|
||||
@@ -381,7 +381,7 @@ function JailConfigDetail({
|
||||
</Text>
|
||||
) : (
|
||||
logPaths.map((p, i) => (
|
||||
<div key={i} className={styles.regexItem}>
|
||||
<div key={p} className={styles.regexItem}>
|
||||
<Input
|
||||
className={styles.codeFont}
|
||||
style={{ flexGrow: 1 }}
|
||||
|
||||
@@ -5,10 +5,15 @@
|
||||
* Used in jail config panels and the filter form.
|
||||
*/
|
||||
|
||||
import { useCallback, useState } from "react";
|
||||
import { useCallback, useMemo, useRef, useState } from "react";
|
||||
import { Button, Input, Text } from "@fluentui/react-components";
|
||||
import { Dismiss24Regular } from "@fluentui/react-icons";
|
||||
import { useConfigStyles } from "./configStyles";
|
||||
import {
|
||||
createStableStringEntry,
|
||||
reconcileStableStringEntries,
|
||||
StableStringEntry,
|
||||
} from "./stableListEntries";
|
||||
|
||||
export interface RegexListProps {
|
||||
/** Section label displayed above the list. */
|
||||
@@ -34,6 +39,23 @@ export function RegexList({
|
||||
readOnly = false,
|
||||
}: RegexListProps): React.JSX.Element {
|
||||
const styles = useConfigStyles();
|
||||
const entriesRef = useRef<StableStringEntry[]>(
|
||||
patterns.map(createStableStringEntry),
|
||||
);
|
||||
|
||||
const entries = useMemo(() => {
|
||||
if (
|
||||
entriesRef.current.length === patterns.length &&
|
||||
entriesRef.current.every((entry, index) => entry.value === patterns[index])
|
||||
) {
|
||||
return entriesRef.current;
|
||||
}
|
||||
|
||||
const reconciled = reconcileStableStringEntries(entriesRef.current, patterns);
|
||||
entriesRef.current = reconciled;
|
||||
return reconciled;
|
||||
}, [patterns]);
|
||||
|
||||
const [newPattern, setNewPattern] = useState("");
|
||||
|
||||
const handleAdd = useCallback(() => {
|
||||
@@ -62,11 +84,11 @@ export function RegexList({
|
||||
(none)
|
||||
</Text>
|
||||
)}
|
||||
{patterns.map((p, i) => (
|
||||
<div key={i} className={styles.regexItem}>
|
||||
{entries.map(({ id, value }, i) => (
|
||||
<div key={id} className={styles.regexItem}>
|
||||
<Input
|
||||
className={styles.regexInput}
|
||||
value={p}
|
||||
value={value}
|
||||
readOnly={readOnly}
|
||||
aria-label={`${label} pattern ${String(i + 1)}`}
|
||||
onChange={(_e, d) => {
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
import { Button, Input } from "@fluentui/react-components";
|
||||
import { Add24Regular, Delete24Regular } from "@fluentui/react-icons";
|
||||
import { useMemo, useRef } from "react";
|
||||
import {
|
||||
createStableStringEntry,
|
||||
reconcileStableStringEntries,
|
||||
StableStringEntry,
|
||||
} from "./stableListEntries";
|
||||
|
||||
interface StringListEditorProps {
|
||||
items: string[];
|
||||
@@ -14,6 +20,23 @@ export function StringListEditor({
|
||||
placeholder,
|
||||
addLabel = "Add entry",
|
||||
}: StringListEditorProps): React.JSX.Element {
|
||||
const entriesRef = useRef<StableStringEntry[]>(
|
||||
items.map(createStableStringEntry),
|
||||
);
|
||||
|
||||
const entries = useMemo(() => {
|
||||
if (
|
||||
entriesRef.current.length === items.length &&
|
||||
entriesRef.current.every((entry, index) => entry.value === items[index])
|
||||
) {
|
||||
return entriesRef.current;
|
||||
}
|
||||
|
||||
const reconciled = reconcileStableStringEntries(entriesRef.current, items);
|
||||
entriesRef.current = reconciled;
|
||||
return reconciled;
|
||||
}, [items]);
|
||||
|
||||
const handleChange = (index: number, value: string): void => {
|
||||
const next = [...items];
|
||||
next[index] = value;
|
||||
@@ -30,14 +53,14 @@ export function StringListEditor({
|
||||
|
||||
return (
|
||||
<div>
|
||||
{items.map((item, index) => (
|
||||
<div key={index} style={{ display: "flex", gap: 4, marginBottom: 4, alignItems: "center" }}>
|
||||
{entries.map(({ id, value }, index) => (
|
||||
<div key={id} style={{ display: "flex", gap: 4, marginBottom: 4, alignItems: "center" }}>
|
||||
<Input
|
||||
value={item}
|
||||
value={value}
|
||||
size="small"
|
||||
style={{ flex: 1, fontFamily: "monospace" }}
|
||||
placeholder={placeholder}
|
||||
aria-label={`Entry ${String(index + 1)}${item ? `: ${item}` : ""}`}
|
||||
aria-label={`Entry ${String(index + 1)}${value ? `: ${value}` : ""}`}
|
||||
onChange={(_e, d) => { handleChange(index, d.value); }}
|
||||
/>
|
||||
<Button
|
||||
|
||||
34
frontend/src/components/config/__tests__/RegexList.test.tsx
Normal file
34
frontend/src/components/config/__tests__/RegexList.test.tsx
Normal file
@@ -0,0 +1,34 @@
|
||||
import { fireEvent, render, screen } from "@testing-library/react";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { RegexList } from "../RegexList";
|
||||
|
||||
describe("RegexList", () => {
|
||||
it("handles duplicate patterns and deletes the correct pattern", () => {
|
||||
const handleChange = vi.fn();
|
||||
|
||||
render(
|
||||
<RegexList
|
||||
label="Test patterns"
|
||||
patterns={["foo", "foo"]}
|
||||
onChange={handleChange}
|
||||
/>,
|
||||
);
|
||||
|
||||
const inputs = screen.getAllByRole("textbox", {
|
||||
name: /Test patterns pattern \d+$/i,
|
||||
});
|
||||
expect(inputs).toHaveLength(2);
|
||||
const [firstInput, secondInput] = inputs;
|
||||
expect(firstInput).toHaveValue("foo");
|
||||
expect(secondInput).toHaveValue("foo");
|
||||
|
||||
const deleteButtons = screen.getAllByRole("button", {
|
||||
name: /Remove Test patterns pattern \d+$/i,
|
||||
});
|
||||
expect(deleteButtons).toHaveLength(2);
|
||||
const [firstDeleteButton] = deleteButtons;
|
||||
|
||||
fireEvent.click(firstDeleteButton);
|
||||
expect(handleChange).toHaveBeenLastCalledWith(["foo"]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,44 @@
|
||||
import { fireEvent, render, screen } from "@testing-library/react";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { StringListEditor } from "../StringListEditor";
|
||||
|
||||
describe("StringListEditor", () => {
|
||||
it("renders duplicate items and updates the correct entry", () => {
|
||||
const handleChange = vi.fn();
|
||||
|
||||
render(
|
||||
<StringListEditor
|
||||
items={["foo", "foo"]}
|
||||
onChange={handleChange}
|
||||
placeholder="Value"
|
||||
/>,
|
||||
);
|
||||
|
||||
const inputs = screen.getAllByRole("textbox");
|
||||
expect(inputs).toHaveLength(2);
|
||||
const [firstInput, secondInput] = inputs;
|
||||
expect(firstInput).toHaveValue("foo");
|
||||
expect(secondInput).toHaveValue("foo");
|
||||
|
||||
fireEvent.change(firstInput, { target: { value: "bar" } });
|
||||
expect(handleChange).toHaveBeenLastCalledWith(["bar", "foo"]);
|
||||
});
|
||||
|
||||
it("removes the correct item when delete is clicked", () => {
|
||||
const handleChange = vi.fn();
|
||||
|
||||
render(
|
||||
<StringListEditor
|
||||
items={["one", "two", "three"]}
|
||||
onChange={handleChange}
|
||||
/>,
|
||||
);
|
||||
|
||||
const deleteButtons = screen.getAllByRole("button", { name: /Remove entry/i });
|
||||
expect(deleteButtons).toHaveLength(3);
|
||||
const [, secondDeleteButton] = deleteButtons;
|
||||
|
||||
fireEvent.click(secondDeleteButton);
|
||||
expect(handleChange).toHaveBeenLastCalledWith(["one", "three"]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,38 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { reconcileStableStringEntries } from "../stableListEntries";
|
||||
|
||||
describe("reconcileStableStringEntries", () => {
|
||||
it("preserves stable ids for unchanged values and generates new ids for inserted values", () => {
|
||||
const previous = [
|
||||
{ id: "a", value: "foo" },
|
||||
{ id: "b", value: "bar" },
|
||||
{ id: "c", value: "foo" },
|
||||
];
|
||||
|
||||
const next = reconcileStableStringEntries(previous, ["foo", "baz", "foo"]);
|
||||
|
||||
expect(next).toHaveLength(3);
|
||||
if (next.length !== 3) throw new Error("expected three entries");
|
||||
|
||||
expect(next[0].id).toBe("a");
|
||||
expect(next[0].value).toBe("foo");
|
||||
expect(next[1].value).toBe("baz");
|
||||
expect(next[1].id).not.toBe("b");
|
||||
expect(next[2].id).toBe("c");
|
||||
});
|
||||
|
||||
it("reuses ids when string values move positions", () => {
|
||||
const previous = [
|
||||
{ id: "a", value: "foo" },
|
||||
{ id: "b", value: "bar" },
|
||||
{ id: "c", value: "baz" },
|
||||
];
|
||||
|
||||
const next = reconcileStableStringEntries(previous, ["bar", "foo", "baz"]);
|
||||
|
||||
if (next.length !== 3) throw new Error("expected three entries");
|
||||
expect(next[0].id).toBe("b");
|
||||
expect(next[1].id).toBe("a");
|
||||
expect(next[2].id).toBe("c");
|
||||
});
|
||||
});
|
||||
34
frontend/src/components/config/stableListEntries.ts
Normal file
34
frontend/src/components/config/stableListEntries.ts
Normal file
@@ -0,0 +1,34 @@
|
||||
export interface StableStringEntry {
|
||||
id: string;
|
||||
value: string;
|
||||
}
|
||||
|
||||
const generateStableStringEntryId = (): string => {
|
||||
if (typeof crypto !== "undefined" && typeof crypto.randomUUID === "function") {
|
||||
return crypto.randomUUID();
|
||||
}
|
||||
|
||||
return `entry-${Math.random().toString(36).slice(2)}`;
|
||||
};
|
||||
|
||||
export const createStableStringEntry = (value: string): StableStringEntry => ({
|
||||
id: generateStableStringEntryId(),
|
||||
value,
|
||||
});
|
||||
|
||||
export const reconcileStableStringEntries = (
|
||||
previousEntries: StableStringEntry[],
|
||||
nextValues: string[],
|
||||
): StableStringEntry[] => {
|
||||
const available = [...previousEntries];
|
||||
|
||||
return nextValues.map((value) => {
|
||||
const matchIndex = available.findIndex((entry) => entry.value === value);
|
||||
if (matchIndex >= 0) {
|
||||
const [existing] = available.splice(matchIndex, 1) as [StableStringEntry];
|
||||
return existing;
|
||||
}
|
||||
|
||||
return createStableStringEntry(value);
|
||||
});
|
||||
};
|
||||
Reference in New Issue
Block a user