From 819e8de172f23f7bd0ddfde7710485c8f1a43054 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Sat, 25 Apr 2026 08:21:13 -0500 Subject: [PATCH] Revert modal changes (#23001) * revert modal changes from #22963 * add test and lint --- web/e2e/specs/export.spec.ts | 110 ++++++++++++++++++ web/e2e/specs/face-library.spec.ts | 4 +- web/src/components/card/ExportCard.tsx | 2 +- web/src/components/card/ReviewCard.tsx | 2 +- web/src/components/menu/LiveContextMenu.tsx | 2 +- .../components/menu/SearchResultActions.tsx | 4 +- .../components/overlay/ActionsDropdown.tsx | 2 +- web/src/pages/FaceLibrary.tsx | 2 +- .../classification/ModelSelectionView.tsx | 2 +- .../classification/ModelTrainingView.tsx | 2 +- 10 files changed, 122 insertions(+), 10 deletions(-) diff --git a/web/e2e/specs/export.spec.ts b/web/e2e/specs/export.spec.ts index 4db98d5e9..5b7e9f0b3 100644 --- a/web/e2e/specs/export.spec.ts +++ b/web/e2e/specs/export.spec.ts @@ -1,4 +1,114 @@ import { test, expect } from "../fixtures/frigate-test"; +import { + expectBodyInteractive, + waitForBodyInteractive, +} from "../helpers/overlay-interaction"; + +test.describe("Export Page - Delete race @high", () => { + // Empirical guard for radix-ui/primitives#3445: when a modal DropdownMenu + // opens an AlertDialog and the AlertDialog's confirm action causes the + // parent's optimistic cache update to unmount the card, we want to know + // whether the deduped react-dismissable-layer (1.1.11) handles the + // pointer-events stack cleanup or whether `modal={false}` is still + // required on the DropdownMenu. The classic "canonical" pattern, distinct + // from the FaceSelectionDialog auto-unmount race already covered by + // face-library.spec.ts. + test("deleting an export via dropdown→alert→confirm leaves body interactive", async ({ + frigateApp, + }) => { + if (frigateApp.isMobile) { + test.skip(); + return; + } + + const initialExports = [ + { + id: "export-race-001", + camera: "front_door", + name: "Race - Test Export", + date: 1775490731.3863528, + video_path: "/exports/export-race-001.mp4", + thumb_path: "/exports/export-race-001-thumb.jpg", + in_progress: false, + export_case_id: null, + }, + ]; + let deleted = false; + + await frigateApp.installDefaults({ + exports: initialExports, + }); + + // Flip /api/export to empty after the delete POST is observed so the + // page's SWR mutate sees the export gone. + await frigateApp.page.route("**/api/export**", async (route) => { + const payload = deleted ? [] : initialExports; + await route.fulfill({ json: payload }); + }); + await frigateApp.page.route("**/api/exports/delete", async (route) => { + deleted = true; + const delayMs = Number( + (globalThis as { process?: { env?: Record } }).process + ?.env?.DELETE_DELAY_MS ?? "100", + ); + if (delayMs > 0) { + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + await route.fulfill({ json: { success: true } }); + }); + + await frigateApp.goto("/export"); + await expect(frigateApp.page.getByText("Race - Test Export")).toBeVisible({ + timeout: 5_000, + }); + + // Open the kebab menu on the export card. The kebab uses the + // (misleading) aria-label "Edit name" from ExportCard's source — it + // wraps the FiMoreVertical icon. There is exactly one such button on + // the page once we have a single export rendered. + const kebab = frigateApp.page + .getByRole("button", { name: /edit name/i }) + .first(); + await expect(kebab).toBeVisible({ timeout: 5_000 }); + await kebab.click(); + + const menu = frigateApp.page + .locator('[role="menu"], [data-radix-menu-content]') + .first(); + await expect(menu).toBeVisible({ timeout: 3_000 }); + + // Delete Export + await menu + .getByRole("menuitem", { name: /delete export/i }) + .first() + .click(); + + // AlertDialog at page level. The confirm button's accessible name is + // "Delete Export" (its aria-label), the visible text is just "Delete". + const confirm = frigateApp.page.getByRole("alertdialog"); + await expect(confirm).toBeVisible({ timeout: 3_000 }); + await confirm + .getByRole("button", { name: /^delete export$/i }) + .first() + .click(); + + // The card optimistically disappears, the dialog closes, and body + // pointer-events must come unstuck. + await expect( + frigateApp.page.getByText("Race - Test Export"), + ).not.toBeVisible({ timeout: 5_000 }); + await waitForBodyInteractive(frigateApp.page, 5_000); + await expectBodyInteractive(frigateApp.page); + + // Sanity: another page-level button still responds. + const newCase = frigateApp.page.getByRole("button", { name: /new case/i }); + await expect(newCase).toBeVisible({ timeout: 3_000 }); + await newCase.click(); + await expect( + frigateApp.page.getByRole("dialog").filter({ hasText: /create case/i }), + ).toBeVisible({ timeout: 3_000 }); + }); +}); test.describe("Export Page - Overview @high", () => { test("renders uncategorized exports and case cards from mock data", async ({ diff --git a/web/e2e/specs/face-library.spec.ts b/web/e2e/specs/face-library.spec.ts index 74f12fce2..e499178b7 100644 --- a/web/e2e/specs/face-library.spec.ts +++ b/web/e2e/specs/face-library.spec.ts @@ -472,7 +472,9 @@ test.describe("FaceSelectionDialog @high", () => { await groupedImage.locator("xpath=..").click(); const dialog = frigateApp.page .getByRole("dialog") - .filter({ has: frigateApp.page.locator('img[src*="clips/faces/train/"]') }) + .filter({ + has: frigateApp.page.locator('img[src*="clips/faces/train/"]'), + }) .first(); await expect(dialog).toBeVisible({ timeout: 5_000 }); diff --git a/web/src/components/card/ExportCard.tsx b/web/src/components/card/ExportCard.tsx index 966aab4dc..893f251f8 100644 --- a/web/src/components/card/ExportCard.tsx +++ b/web/src/components/card/ExportCard.tsx @@ -266,7 +266,7 @@ export function ExportCard({ )} {!exportedRecording.in_progress && !selectionMode && (
- + - + {content} diff --git a/web/src/components/menu/LiveContextMenu.tsx b/web/src/components/menu/LiveContextMenu.tsx index 982895200..8ed78e348 100644 --- a/web/src/components/menu/LiveContextMenu.tsx +++ b/web/src/components/menu/LiveContextMenu.tsx @@ -272,7 +272,7 @@ export default function LiveContextMenu({ return (
- + {children}
diff --git a/web/src/components/menu/SearchResultActions.tsx b/web/src/components/menu/SearchResultActions.tsx index 2a8cca5a8..aa2562b42 100644 --- a/web/src/components/menu/SearchResultActions.tsx +++ b/web/src/components/menu/SearchResultActions.tsx @@ -258,13 +258,13 @@ export default function SearchResultActions({ {isContextMenu ? ( - + {children} {menuItems} ) : ( <> - + diff --git a/web/src/components/overlay/ActionsDropdown.tsx b/web/src/components/overlay/ActionsDropdown.tsx index 7f841be4f..9f9596d0a 100644 --- a/web/src/components/overlay/ActionsDropdown.tsx +++ b/web/src/components/overlay/ActionsDropdown.tsx @@ -22,7 +22,7 @@ export default function ActionsDropdown({ const { t } = useTranslation(["components/dialog", "views/replay", "common"]); return ( - +
- + e.stopPropagation()}> diff --git a/web/src/views/classification/ModelTrainingView.tsx b/web/src/views/classification/ModelTrainingView.tsx index 23fd6f374..dd4f3c9c0 100644 --- a/web/src/views/classification/ModelTrainingView.tsx +++ b/web/src/views/classification/ModelTrainingView.tsx @@ -698,7 +698,7 @@ function LibrarySelector({ regexErrorMessage={t("description.invalidName")} /> - +