diff --git a/frontend/editor/src/core/components/shared/AppConfigModal.tsx b/frontend/editor/src/core/components/shared/AppConfigModal.tsx index 8c6bf423ed..c5adef038d 100644 --- a/frontend/editor/src/core/components/shared/AppConfigModal.tsx +++ b/frontend/editor/src/core/components/shared/AppConfigModal.tsx @@ -33,7 +33,6 @@ import { useUnsavedChanges, } from "@app/contexts/UnsavedChangesContext"; import { stripBasePath, withBasePath } from "@app/constants/app"; -import { EDITOR_BASENAME } from "@app/routes/editorBasename"; interface AppConfigModalProps { opened: boolean; @@ -232,27 +231,9 @@ const AppConfigModalInner: React.FC = ({ const handleClose = useCallback(async () => { const canProceed = await confirmIfDirty(); if (!canProceed) return false; - - // Only unwind history if settings was opened via the URL; opened via state - // there's no /settings entry to pop and navigate(-1) would jump to /files. - if (urlSync && location.pathname.startsWith("/settings")) { - // "default" key = first entry (deep link/refresh); nothing to pop to. - if (location.key === "default") { - navigate(EDITOR_BASENAME, { replace: true }); - } else { - navigate(-1); - } - } onClose(); return true; - }, [ - confirmIfDirty, - location.key, - location.pathname, - navigate, - onClose, - urlSync, - ]); + }, [confirmIfDirty, onClose]); // Synchronous wrapper for contexts (e.g. tour buttons) that need () => void const handleCloseSync = useCallback(() => { diff --git a/frontend/editor/src/core/components/shared/superSearch/SuperSearch.tsx b/frontend/editor/src/core/components/shared/superSearch/SuperSearch.tsx index 9b5cf22190..054e99cfde 100644 --- a/frontend/editor/src/core/components/shared/superSearch/SuperSearch.tsx +++ b/frontend/editor/src/core/components/shared/superSearch/SuperSearch.tsx @@ -289,16 +289,25 @@ export default function SuperSearch({ inputRef.current?.select(); }; // Focus handover from a closing dialog. Only the on-screen instance - // responds (offsetParent is null while display:none / unmounted hosts), - // and focus waits two frames so the dialog's own return-focus runs first. + // responds (offsetParent is null while a host is display:none / unmounted). const onFocusRequest = () => { const input = inputRef.current; if (!input || input.offsetParent === null) return; setOpen(true); + const grab = () => { + input.focus(); + input.select(); + }; requestAnimationFrame(() => requestAnimationFrame(() => { - input.focus(); - input.select(); + grab(); + // The dialog's return-focus fires shortly after it closes and steals + // focus back once; re-grab it if that happens. + input.addEventListener("focusout", grab, { once: true }); + window.setTimeout( + () => input.removeEventListener("focusout", grab), + 250, + ); }), ); }; diff --git a/frontend/editor/src/core/pages/HomePage.tsx b/frontend/editor/src/core/pages/HomePage.tsx index d695de8ac3..e0c5303447 100644 --- a/frontend/editor/src/core/pages/HomePage.tsx +++ b/frontend/editor/src/core/pages/HomePage.tsx @@ -29,6 +29,7 @@ import LocalIcon from "@app/components/shared/LocalIcon"; import AppConfigModal from "@app/components/shared/AppConfigModalLazy"; import { getStartupNavigationAction } from "@app/utils/homePageNavigation"; import { EDITOR_BASENAME } from "@app/routes/editorBasename"; +import { stripBasePath } from "@app/constants/app"; import { HomePageExtensions } from "@app/components/home/HomePageExtensions"; import { FilesPageProvider, @@ -123,12 +124,30 @@ export default function HomePage() { return () => window.removeEventListener("appConfig:open", handler); }, []); - const handleCloseConfig = useCallback(() => { - setConfigModalOpen(false); - if (location.pathname.startsWith("/settings")) { - navigate(EDITOR_BASENAME, { replace: true }); + // Where the user was before settings opened, so close can restore it. Null + // when opened directly on a /settings URL (deep link) - close falls back to + // the editor root. + const settingsOriginRef = useRef(null); + const wasConfigOpenRef = useRef(false); + useEffect(() => { + if (configModalOpen && !wasConfigOpenRef.current) { + settingsOriginRef.current = location.pathname.startsWith("/settings") + ? null + : location.pathname; } - }, [location.pathname, navigate]); + wasConfigOpenRef.current = configModalOpen; + }, [configModalOpen, location.pathname]); + + const handleCloseConfig = useCallback(() => { + // Restore the URL before clearing the flag, or a late /settings commit + // re-opens the modal. Read window.location, not useLocation: a tab switch + // updates the URL synchronously while the router's commit lags. Replace to + // the origin rather than navigate(-1), which webkit can drop. + if (stripBasePath(window.location.pathname).startsWith("/settings")) { + navigate(settingsOriginRef.current ?? EDITOR_BASENAME, { replace: true }); + } + setConfigModalOpen(false); + }, [navigate]); const { activeFiles } = useFileContext(); const navigationState = useNavigationState(); diff --git a/frontend/editor/src/core/tests/helpers/ui-helpers.ts b/frontend/editor/src/core/tests/helpers/ui-helpers.ts index 353037c106..1ce62bd828 100644 --- a/frontend/editor/src/core/tests/helpers/ui-helpers.ts +++ b/frontend/editor/src/core/tests/helpers/ui-helpers.ts @@ -69,31 +69,73 @@ export async function waitForModalClose( /** * Upload one or more files through the FileSidebar's "Open from computer" - * action. The button is always rendered (collapsed or expanded sidebar) and - * fires the hidden `data-testid="file-input"`. Its native OS picker is mocked - * globally by `suppressNativeFilePicker` (installed by the test fixtures), so - * the click is safe on every browser; we then set the files directly on the - * input via `setInputFiles`. - * - * `setInputFiles` doesn't await the input's async onChange (which writes to - * IndexedDB via `addFiles`), so without a sync point a caller that follows - * with `page.goto()` can race the IDB flush. Wait for the workbench to - * pick up the upload (the FileSidebar renders the added file in its scroll - * list once `addFiles` resolves and IDB has been written). + * action. The native picker is mocked globally by `suppressNativeFilePicker`, + * so the click is safe on every browser; files are set on the hidden input via + * `setInputFiles`. Returns only once the files are durably in IndexedDB, so + * callers may navigate or reload without racing the write. */ export async function uploadFiles( page: Page, filePaths: string | string[], ): Promise { const paths = Array.isArray(filePaths) ? filePaths : [filePaths]; + const names = paths.map((p) => p.split(/[\\/]/).pop() ?? p); await page.getByTestId("files-button").click(); await page.locator('[data-testid="file-input"]').setInputFiles(paths); - // Sync point: wait until at least one file lands in the sidebar's file - // list. The list only renders once `addFiles` has resolved (which awaits - // the IDB write). Use first() so multi-file uploads pass too. + // The sidebar renders from in-memory state, before the IDB write commits. + // first() so multi-file uploads pass too. await expect(page.locator(".file-sidebar-file-item").first()).toBeVisible({ timeout: 10_000, }); + // A navigation before the write commits aborts the transaction and drops the + // file, so wait for it to land rather than assume the sidebar means it did. + await waitForStoredFiles(page, names); +} + +/** + * Resolve once every uploaded name is in the files store. Read-only: aborts + * rather than create or upgrade the DB, so it can't race the app's own + * versioned open and leave it without object stores. Reads only a DB the app + * already made; until then each poll returns false and retries. + */ +async function waitForStoredFiles(page: Page, names: string[]): Promise { + await page.waitForFunction( + (expected) => + new Promise((resolve) => { + const open = indexedDB.open("stirling-pdf-files"); + open.onupgradeneeded = () => { + open.transaction?.abort(); + resolve(false); + }; + open.onsuccess = () => { + const db = open.result; + if (!db.objectStoreNames.contains("files")) { + db.close(); + resolve(false); + return; + } + const request = db + .transaction("files", "readonly") + .objectStore("files") + .getAll(); + request.onsuccess = () => { + const stored = new Set( + (request.result as Array<{ name?: string }>).map((r) => r.name), + ); + db.close(); + resolve(expected.every((name) => stored.has(name))); + }; + request.onerror = () => { + db.close(); + resolve(false); + }; + }; + open.onerror = () => resolve(false); + open.onblocked = () => resolve(false); + }), + names, + { timeout: 10_000, polling: 100 }, + ); } /** @@ -152,11 +194,20 @@ export async function openSettings(page: Page): Promise { * dialog is fully dismissed before returning. */ export async function closeSettings(page: Page): Promise { - const closeBtn = page.locator('[aria-label="Close"]').first(); - await closeBtn.click(); - await expect(page.locator(".mantine-Modal-content").first()).not.toBeVisible({ - timeout: 5_000, - }); + const modal = page.locator(".mantine-Modal-content").first(); + // A click on the X can be swallowed by a re-render, leaving the modal open; + // retry until it's gone. A genuinely broken close still fails - every retry + // misses and the modal stays past the cap. + await expect(async () => { + if (await modal.isVisible().catch(() => false)) { + await page + .locator('[aria-label="Close"]') + .first() + .click({ timeout: 2_000 }) + .catch(() => {}); + } + await expect(modal).not.toBeVisible({ timeout: 2_000 }); + }).toPass({ timeout: 12_000 }); } /**