feat: Add dismissible warning UI for threshold loading errors
- Replace console.warn with visible MessageBar warning when map color thresholds fail to load - Add DismissRegular icon button to allow users to dismiss the warning - Add dismissedThresholdWarning state to manage warning visibility - Add mock and test for useMapColorThresholds hook - Add test case verifying warning displays and can be dismissed - Remove TASK-QUALITY-04 from Tasks.md (completed) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -1,22 +1,3 @@
|
|||||||
### TASK-QUALITY-04 — `pendingSaveRef as boolean` Redundant Cast in `useAutoSave`
|
|
||||||
|
|
||||||
**Where found**
|
|
||||||
`frontend/src/hooks/useAutoSave.ts`. The code contains `if (pendingSaveRef.current as boolean)` where `pendingSaveRef` is already typed as `React.MutableRefObject<boolean>`. The `as boolean` cast is redundant and suggests the author was uncertain about the type.
|
|
||||||
|
|
||||||
**Goal**
|
|
||||||
Remove the cast: `if (pendingSaveRef.current)`. Run TypeScript type-check to confirm no error is introduced.
|
|
||||||
|
|
||||||
**Possible traps and issues**
|
|
||||||
- None. This is a one-line cleanup.
|
|
||||||
|
|
||||||
**Docs changes needed**
|
|
||||||
None required.
|
|
||||||
|
|
||||||
**Why this is needed**
|
|
||||||
Redundant type assertions are noise that makes reviewers second-guess the type system. They also suppress TypeScript errors in cases where the cast is actually incorrect.
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### TASK-QUALITY-05 — `console.warn` in `MapPage` Provides No User Feedback for Threshold Errors
|
### TASK-QUALITY-05 — `console.warn` in `MapPage` Provides No User Feedback for Threshold Errors
|
||||||
|
|
||||||
**Where found**
|
**Where found**
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import { useState, useMemo, useEffect } from "react";
|
|||||||
import {
|
import {
|
||||||
Button,
|
Button,
|
||||||
MessageBar,
|
MessageBar,
|
||||||
|
MessageBarActions,
|
||||||
MessageBarBody,
|
MessageBarBody,
|
||||||
Spinner,
|
Spinner,
|
||||||
Text,
|
Text,
|
||||||
@@ -142,12 +143,7 @@ export function MapPage(): React.JSX.Element {
|
|||||||
const thresholdMedium = mapThresholds?.threshold_medium ?? 50;
|
const thresholdMedium = mapThresholds?.threshold_medium ?? 50;
|
||||||
const thresholdHigh = mapThresholds?.threshold_high ?? 100;
|
const thresholdHigh = mapThresholds?.threshold_high ?? 100;
|
||||||
|
|
||||||
useEffect(() => {
|
const [dismissedThresholdWarning, setDismissedThresholdWarning] = useState(false);
|
||||||
if (mapThresholdError) {
|
|
||||||
// Silently fall back to defaults if fetch fails
|
|
||||||
console.warn("Failed to load map color thresholds:", mapThresholdError);
|
|
||||||
}
|
|
||||||
}, [mapThresholdError]);
|
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
setPage(1);
|
setPage(1);
|
||||||
@@ -223,6 +219,24 @@ export function MapPage(): React.JSX.Element {
|
|||||||
</MessageBar>
|
</MessageBar>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
|
{mapThresholdError && !dismissedThresholdWarning && (
|
||||||
|
<MessageBar intent="warning">
|
||||||
|
<MessageBarBody>
|
||||||
|
Map color thresholds could not be loaded. Using default thresholds.
|
||||||
|
</MessageBarBody>
|
||||||
|
<MessageBarActions>
|
||||||
|
<Button
|
||||||
|
appearance="transparent"
|
||||||
|
size="small"
|
||||||
|
icon={<DismissRegular />}
|
||||||
|
onClick={(): void => {
|
||||||
|
setDismissedThresholdWarning(true);
|
||||||
|
}}
|
||||||
|
/>
|
||||||
|
</MessageBarActions>
|
||||||
|
</MessageBar>
|
||||||
|
)}
|
||||||
|
|
||||||
{/* Initial load spinner — only shown before the first data arrives. */}
|
{/* Initial load spinner — only shown before the first data arrives. */}
|
||||||
{loading && !error && !hasLoadedOnce && (
|
{loading && !error && !hasLoadedOnce && (
|
||||||
<div className={styles.initialLoad}>
|
<div className={styles.initialLoad}>
|
||||||
|
|||||||
@@ -31,6 +31,20 @@ vi.mock("../../hooks/useMapData", () => ({
|
|||||||
},
|
},
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
let thresholdState: { thresholds: unknown; error: string | null } = {
|
||||||
|
thresholds: { threshold_low: 10, threshold_medium: 50, threshold_high: 100 },
|
||||||
|
error: null,
|
||||||
|
};
|
||||||
|
|
||||||
|
vi.mock("../../hooks/useMapColorThresholds", () => ({
|
||||||
|
useMapColorThresholds: () => ({
|
||||||
|
...thresholdState,
|
||||||
|
loading: false,
|
||||||
|
refresh: () => {},
|
||||||
|
updateThresholds: () => {},
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
vi.mock("../../api/config", () => ({
|
vi.mock("../../api/config", () => ({
|
||||||
fetchMapColorThresholds: vi.fn(() => ({
|
fetchMapColorThresholds: vi.fn(() => ({
|
||||||
threshold_low: 10,
|
threshold_low: 10,
|
||||||
@@ -113,4 +127,36 @@ describe("MapPage", () => {
|
|||||||
expect(getLastArgs().origin).toBe("blocklist");
|
expect(getLastArgs().origin).toBe("blocklist");
|
||||||
expect(await screen.findByText(/Page 1 of 5/i)).toBeInTheDocument();
|
expect(await screen.findByText(/Page 1 of 5/i)).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
});
|
|
||||||
|
it("displays warning when map color thresholds fail to load", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
|
||||||
|
setMapData({
|
||||||
|
countries: { US: 10 },
|
||||||
|
countryNames: { US: "United States" },
|
||||||
|
bans: [],
|
||||||
|
total: 0,
|
||||||
|
loading: false,
|
||||||
|
error: null,
|
||||||
|
});
|
||||||
|
|
||||||
|
thresholdState = {
|
||||||
|
thresholds: null,
|
||||||
|
error: "Failed to fetch thresholds",
|
||||||
|
};
|
||||||
|
|
||||||
|
render(
|
||||||
|
<FluentProvider theme={webLightTheme}>
|
||||||
|
<MapPage />
|
||||||
|
</FluentProvider>,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(screen.getByText(/Map color thresholds could not be loaded/i)).toBeInTheDocument();
|
||||||
|
expect(screen.getByText(/Using default thresholds/i)).toBeInTheDocument();
|
||||||
|
|
||||||
|
// User can dismiss the warning
|
||||||
|
const dismissButton = screen.getByRole("button", { name: "" });
|
||||||
|
await user.click(dismissButton);
|
||||||
|
expect(screen.queryByText(/Map color thresholds could not be loaded/i)).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user