Fix: Keep ConfigPage tabs mounted to preserve form state
Previously, the tab content wrapper used 'key={tab}' which caused React to
unmount and remount the entire subtree when switching tabs. This destroyed
all component state, including unsaved form data and pending auto-saves.
Changes:
- Removed 'key={tab}' from the wrapper div
- All tab panels now render at page initialization
- Inactive tabs use CSS 'display: none' to hide without unmounting
- Tabs remain mounted throughout the page lifetime
- Users can now switch tabs without losing form input
Updated ConfigPage.test.tsx to reflect that inactive tabs remain in the DOM
(just hidden with CSS) rather than being removed entirely.
Documentation: Added 'Tab Panels' section to Web-Development.md
explaining the rule and rationale.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -1,15 +1,3 @@
|
|||||||
|
|
||||||
### ✅ TASK-BUG-01 — Infinite Re-Fetch Loop in `useJailConfigs` — DONE
|
|
||||||
|
|
||||||
**Fix Summary**
|
|
||||||
Wrapped the `onSuccess` callback in `useCallback` with empty dependencies in `frontend/src/hooks/useJailConfigs.ts` (lines 33-35). The inline callback was creating a new reference on every render, which caused `useListData`'s internal `refresh` function to be recreated (since `onSuccess` is in its deps), which triggered the `useEffect` again, causing an infinite fetch loop.
|
|
||||||
|
|
||||||
Added comprehensive test coverage in `frontend/src/hooks/__tests__/useJailConfigs.test.ts` to verify the hook no longer triggers infinite refetches. Updated `Docs/Refactoring.md` with documentation explaining the `onSuccess` stability requirement for all `useListData` callers.
|
|
||||||
|
|
||||||
Commit: `de8af09a3da36dbf24b56fa28656673b232b5e91`
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### TASK-BUG-02 — `ConfigPage` Tab Switch Destroys All Form State
|
### TASK-BUG-02 — `ConfigPage` Tab Switch Destroys All Form State
|
||||||
|
|
||||||
**Where found**
|
**Where found**
|
||||||
@@ -213,7 +201,6 @@ None required.
|
|||||||
**Why this is needed**
|
**Why this is needed**
|
||||||
Typing `"500"` in the Lines field currently fires three HTTP requests (`"5"`, `"50"`, `"500"`). Each request fetches potentially hundreds of log lines and serializes them, adding unnecessary backend load.
|
Typing `"500"` in the Lines field currently fires three HTTP requests (`"5"`, `"50"`, `"500"`). Each request fetches potentially hundreds of log lines and serializes them, adding unnecessary backend load.
|
||||||
|
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### TASK-ABORT-01 — Missing `signal` Parameter on Multiple API Functions
|
### TASK-ABORT-01 — Missing `signal` Parameter on Multiple API Functions
|
||||||
|
|||||||
@@ -270,6 +270,12 @@ function BanCard({ isHighlighted }: BanCardProps): JSX.Element {
|
|||||||
- Supply a `key` prop whenever rendering lists — never use array indices as keys if the list can reorder.
|
- Supply a `key` prop whenever rendering lists — never use array indices as keys if the list can reorder.
|
||||||
- Prefer Fluent UI components (`Button`, `Table`, `Input`, …) over raw HTML elements for any interactive or styled element.
|
- Prefer Fluent UI components (`Button`, `Table`, `Input`, …) over raw HTML elements for any interactive or styled element.
|
||||||
|
|
||||||
|
### Tab Panels
|
||||||
|
|
||||||
|
- **Never** use `key` on a tab panel wrapper to switch between tabs. This causes the entire subtree to unmount and remount, destroying all state, pending saves, and form input.
|
||||||
|
- Instead, render all tab panels and use CSS `display: none` / `display: block` to hide inactive tabs, keeping components mounted across tab switches.
|
||||||
|
- All tab components remain mounted throughout the page lifetime. Hooks continue to run in hidden tabs — if a tab-specific effect must only run on activation, use an explicit activation flag rather than relying on mount/unmount.
|
||||||
|
|
||||||
```tsx
|
```tsx
|
||||||
import { Table, TableBody, TableRow, TableCell, Button } from "@fluentui/react-components";
|
import { Table, TableBody, TableRow, TableCell, Button } from "@fluentui/react-components";
|
||||||
import type { Ban } from "../types/ban";
|
import type { Ban } from "../types/ban";
|
||||||
|
|||||||
@@ -36,7 +36,11 @@ const useStyles = makeStyles({
|
|||||||
header: {
|
header: {
|
||||||
marginBottom: tokens.spacingVerticalL,
|
marginBottom: tokens.spacingVerticalL,
|
||||||
},
|
},
|
||||||
tabContent: {
|
tabPanel: {
|
||||||
|
display: "none",
|
||||||
|
},
|
||||||
|
tabPanelVisible: {
|
||||||
|
display: "block",
|
||||||
marginTop: tokens.spacingVerticalL,
|
marginTop: tokens.spacingVerticalL,
|
||||||
animationName: "fadeInUp",
|
animationName: "fadeInUp",
|
||||||
animationDuration: tokens.durationNormal,
|
animationDuration: tokens.durationNormal,
|
||||||
@@ -94,16 +98,26 @@ export function ConfigPage(): React.JSX.Element {
|
|||||||
<Tab value="regex">Regex Tester</Tab>
|
<Tab value="regex">Regex Tester</Tab>
|
||||||
</TabList>
|
</TabList>
|
||||||
|
|
||||||
<div className={styles.tabContent} key={tab}>
|
<div className={tab === "jails" ? styles.tabPanelVisible : styles.tabPanel}>
|
||||||
{tab === "jails" && (
|
<JailsTab
|
||||||
<JailsTab
|
initialJail={(location.state as { jail?: string } | null)?.jail}
|
||||||
initialJail={(location.state as { jail?: string } | null)?.jail}
|
/>
|
||||||
/>
|
</div>
|
||||||
)}
|
|
||||||
{tab === "filters" && <FiltersTab />}
|
<div className={tab === "filters" ? styles.tabPanelVisible : styles.tabPanel}>
|
||||||
{tab === "actions" && <ActionsTab />}
|
<FiltersTab />
|
||||||
{tab === "server" && <ServerTab />}
|
</div>
|
||||||
{tab === "regex" && <RegexTesterTab />}
|
|
||||||
|
<div className={tab === "actions" ? styles.tabPanelVisible : styles.tabPanel}>
|
||||||
|
<ActionsTab />
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<div className={tab === "server" ? styles.tabPanelVisible : styles.tabPanel}>
|
||||||
|
<ServerTab />
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<div className={tab === "regex" ? styles.tabPanelVisible : styles.tabPanel}>
|
||||||
|
<RegexTesterTab />
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -38,7 +38,8 @@ describe("ConfigPage", () => {
|
|||||||
renderPage();
|
renderPage();
|
||||||
fireEvent.click(screen.getByRole("tab", { name: /filters/i }));
|
fireEvent.click(screen.getByRole("tab", { name: /filters/i }));
|
||||||
expect(screen.getByTestId("filters-tab")).toBeInTheDocument();
|
expect(screen.getByTestId("filters-tab")).toBeInTheDocument();
|
||||||
expect(screen.queryByTestId("jails-tab")).not.toBeInTheDocument();
|
// Jails tab remains mounted (not removed from DOM), just hidden with CSS
|
||||||
|
expect(screen.getByTestId("jails-tab")).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("switches to Actions tab when Actions tab is clicked", () => {
|
it("switches to Actions tab when Actions tab is clicked", () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user