diff --git a/docs/process-hardening.md b/docs/process-hardening.md index abbf4578cd..a26a65fb01 100644 --- a/docs/process-hardening.md +++ b/docs/process-hardening.md @@ -103,10 +103,11 @@ For each: trace which module-scope helpers/icons/types it uses; move solely-cons - Making the dashboard and document viewer client-only via `dynamic(..., { ssr: false })` (PRs #144/#147) meant "page loaded" no longer implies "app mounted". Firefox/WebKit paint the client chunk later than Chromium, so three release-browser-matrix specs raced and failed on `main` while Chromium stayed green. All three were test-timing gaps, not product regressions — the app renders correctly in every browser. - `tests/ui-overlap.spec.ts`: `gotoHome` waited on `networkidle` (Playwright discourages it) and then measured the header, which had not mounted yet — every failure was `header#search not found` (count 0), never a real overlap. Now waits for `header#search` to be visible before measuring. - - `tests/ui-tools.spec.ts` (forms detail → shared search): the shell re-syncs its query from the URL on mount via `requestAnimationFrame`, which on Firefox/WebKit can land just after a programmatic `fill` and wipe the value (button stays disabled) or drop the submit before the router navigates. The fill-and-submit now runs as one `toPass` unit that retries until the search routes. Root-cause fix: the `requestAnimationFrame` effect in `GlobalMockupSearchShellClient` now skips the `setQuery("")` reset on detail pages (where the URL carries no `q`/`query` param), so the programmatic fill is never wiped. + - `tests/ui-tools.spec.ts` (forms detail → shared search): the shell re-synced its query from the URL on mount via `requestAnimationFrame`, which on Firefox/WebKit could land just after a programmatic `fill` and wipe the value (button stays disabled) or drop the submit before the router navigates. The fill-and-submit runs as one `toPass` unit that retries until the search routes. **Root-cause fix (see the 2026-07-03 follow-up below):** `GlobalMockupSearchShellClient` no longer uses a `requestAnimationFrame` sync at all — it seeds the composer mode/query from the URL and re-syncs only when the search string actually changes, so a mount-time frame can never wipe an in-progress fill. - `tests/ui-stress.spec.ts` (desktop evidence panel): the evidence `
` is opened by focusing its `` and pressing Enter; in CI WebKit the key event could fire before focus landed, so it never toggled. Now asserts `toBeFocused()` before pressing Enter. - Rule of thumb for these client-only surfaces: never gate an interaction on `networkidle` or a bare `goto`. Wait for the specific mounted element, and wrap fill→submit→navigate races in `toPass` (the same idiom `openAppModeMenu`/`openDailyActions` already use). The `verify` + `ui-smoke` PR gates run Chromium only, so Firefox/WebKit-specific races surface solely in the gated `release-browser-matrix` (main/release/dispatch/schedule) — keep that job green rather than letting these re-accumulate. -- **Post-merge outcome (PR #178):** `ui-overlap` and `ui-stress` fixes verified green in CI WebKit. `ui-tools.spec.ts:264` (forms-detail search) still fails on **CI WebKit only** — the composer input stays focused-but-empty and the submit disabled across the full retry, and it does **not** reproduce on local WebKit, so it can't be iterated locally. Ruled out: the inline `availableModeIds={["forms"]}` arrays in the `forms`/`services`/`favourites` layouts churning the effect (those layouts are Server Components, so the ref is stable). On WebKit the test now runs its **structural half** (the detail page renders inside the shell with the Forms composer present) and returns before the known-broken **submit-and-route half** (`if (browserName === "webkit") return;`); Chromium + Firefox still verify the full wiring. The root-cause fix (shell mount `requestAnimationFrame` query-sync) is deferred and needs CI-based iteration — removing that WebKit early-return is its exit criterion. +- **Post-merge outcome (PR #178):** `ui-overlap` and `ui-stress` fixes verified green in CI WebKit. `ui-tools.spec.ts:264` (forms-detail search) still failed on **CI WebKit only** — the composer input stayed focused-but-empty and the submit disabled across the full retry, and it did **not** reproduce on local WebKit, so it couldn't be iterated locally. PR #186 added an interim `isDetailPage` guard to the mount `requestAnimationFrame` but stayed `[WIP]`, keeping a `if (browserName === "webkit") return;` early-return that ran only the **structural half** on WebKit. +- **Follow-up resolution (2026-07-03):** removed the `requestAnimationFrame` mount sync in `GlobalMockupSearchShellClient` entirely. The composer mode/query are now seeded from the URL at initial state, and a `lastSyncedSearchParamsRef` gates the re-sync effect so it fires **only when the search string actually changes** (a real navigation). Because typing never changes the URL and the initial mount is a no-op, no deferred frame can wipe an in-progress `fill` on a no-query detail route — the CI-WebKit-only race. This also retired the `isDetailPage`/`previousUrlHadQueryRef` special-casing from PR #186. With the race fixed at the source, the WebKit early-return and its comment were deleted from `ui-tools.spec.ts:264`, so all three browsers now verify the full submit-and-route wiring. Ruled out earlier (and still true): the inline `availableModeIds={["forms"]}` arrays are stable because those layouts are Server Components. ## Suspense fallback must not re-render page children (2026-07-02) diff --git a/src/components/clinical-dashboard/global-mockup-search-shell.tsx b/src/components/clinical-dashboard/global-mockup-search-shell.tsx index 0ccb26c9cc..7907da8c04 100644 --- a/src/components/clinical-dashboard/global-mockup-search-shell.tsx +++ b/src/components/clinical-dashboard/global-mockup-search-shell.tsx @@ -91,9 +91,21 @@ function GlobalMockupSearchShellClient({ const requestedQuery = (searchParams.get("q") ?? searchParams.get("query") ?? "").trim(); const requestedMode = searchParams.get("mode"); const searchParamString = searchParams.toString(); + // Mode resolved from the URL (?mode=), falling back to this shell's default when + // the param is missing, unknown, or not offered here. Seeds the initial mode and + // re-syncs it after a navigation. + const resolvedSearchMode = + isAppModeId(requestedMode) && + isAppModeVisible(requestedMode) && + (!availableModeIds?.length || availableModeIds.includes(requestedMode)) + ? requestedMode + : initialSearchMode; const [query, setQuery] = useState(requestedQuery); - const previousUrlHadQueryRef = useRef(currentUrlHasQuery); - const [searchMode, setSearchMode] = useState(initialSearchMode); + // The search string we last synced into local state, so the effect below only + // reacts to genuine navigations. Seeded with the current string so the initial + // mount is a no-op — the state above is already derived from the URL. + const lastSyncedSearchParamsRef = useRef(searchParamString); + const [searchMode, setSearchMode] = useState(resolvedSearchMode); const [queryMode, setQueryMode] = useState("auto"); const [scopeFilters, setScopeFilters] = useState({}); const [mobileMenuOpen, setMobileMenuOpen] = useState(false); @@ -104,12 +116,6 @@ function GlobalMockupSearchShellClient({ const { theme, toggleTheme } = useTheme(); const auth = useAuthSession(); const sidebarIdentity = useMemo(() => deriveSidebarIdentity(auth.session?.user.email), [auth.session?.user.email]); - const dashboardSearchMode = - isAppModeId(requestedMode) && - isAppModeVisible(requestedMode) && - (!availableModeIds?.length || availableModeIds.includes(requestedMode)) - ? requestedMode - : initialSearchMode; const shouldRenderDashboardSearch = requestedRun && requestedQuery.length > 0; const isFormsOnlyShell = availableModeIds?.length === 1 && availableModeIds[0] === "forms"; const isStandaloneModeHome = @@ -119,41 +125,24 @@ function GlobalMockupSearchShellClient({ (searchMode === "favourites" && pathname === "/favourites") || (searchMode === "differentials" && pathname === "/differentials")); const isDifferentialPresentationWorkflow = pathname.startsWith("/differentials/presentations"); - // True when on a sub-route of a mode home (e.g. /forms/transport-crisis-form, - // /services/13yarn) rather than the mode home itself (/forms, /services). - const isDetailPage = - /^\/(forms|services|favourites)\/.+/.test(pathname) || /^\/differentials\/diagnoses\/.+/.test(pathname); useEffect(() => { - const frame = window.requestAnimationFrame(() => { - const params = new URLSearchParams(window.location.search); - const requestedMode = params.get("mode"); - const nextMode = - isAppModeId(requestedMode) && - isAppModeVisible(requestedMode) && - (!availableModeIds?.length || availableModeIds.includes(requestedMode)) - ? requestedMode - : initialSearchMode; - setSearchMode(nextMode); + // Re-derive the mode and query from the URL, but only when the search string + // actually changes (a real navigation). Reacting on every render — as the old + // requestAnimationFrame sync effectively did — let a deferred frame land after + // a programmatic/user fill and wipe the controlled input; on slow CI WebKit + // that raced the forms-detail composer to empty (input focused-but-empty, + // submit stuck disabled). Typing never changes the URL, so a URL-gated sync + // cannot clobber in-progress input, and the initial mount is skipped entirely + // because the state above is already seeded from the URL. + if (lastSyncedSearchParamsRef.current === searchParamString) return; + lastSyncedSearchParamsRef.current = searchParamString; - const urlHasQuery = params.has("q") || params.has("query"); - const hadQueryBeforeThisSync = previousUrlHadQueryRef.current; - previousUrlHadQueryRef.current = urlHasQuery; - if (urlHasQuery) { - // Sync the controlled query state from the URL query param. - const requestedQuery = (params.get("q") ?? params.get("query"))?.trim(); - setQuery(requestedQuery ?? ""); - } else if (!isDetailPage || hadQueryBeforeThisSync) { - // On no-query routes, clear any stale URL-derived query. Initial detail - // page mounts still skip the deferred clear so programmatic fills are - // not wiped by the WebKit requestAnimationFrame race. - setQuery(""); - } + setSearchMode(resolvedSearchMode); + setQuery(currentUrlHasQuery ? requestedQuery : ""); - if (params.get("focus") === "1") inputRef.current?.focus({ preventScroll: true }); - }); - return () => window.cancelAnimationFrame(frame); - }, [availableModeIds, initialSearchMode, isDetailPage, pathname, searchParamString]); + if (searchParams.get("focus") === "1") inputRef.current?.focus({ preventScroll: true }); + }, [currentUrlHasQuery, requestedQuery, resolvedSearchMode, searchParamString, searchParams]); useEffect(() => { let cancelled = false; @@ -231,14 +220,14 @@ function GlobalMockupSearchShellClient({ navigateToMode("answer", { query: recentQuery, focus: true }); } - if (shouldRenderDashboardSearch && dashboardSearchMode === "forms" && isFormsOnlyShell) { + if (shouldRenderDashboardSearch && resolvedSearchMode === "forms" && isFormsOnlyShell) { return ; } if (shouldRenderDashboardSearch) { return ( { await expectNoPageHorizontalOverflow(page); }); - test("form detail pages keep the shared forms search wired to form results", async ({ page, browserName }) => { + test("form detail pages keep the shared forms search wired to form results", async ({ page }) => { await page.setViewportSize({ width: 1280, height: 900 }); await gotoLauncher(page, "/forms/transport-crisis-form"); @@ -274,22 +274,12 @@ test.describe("Clinical KB applications launcher", () => { const formsSearchInput = page.locator('input[placeholder="Search forms..."]:visible').first(); await expect(formsSearchInput).toBeVisible(); - // Submit-and-route half is known-broken on CI Linux WebKit only: the shell's - // mount requestAnimationFrame query-sync wipes the composer value there (the - // input stays focused-but-empty and the submit disabled), so the search never - // routes. It does not reproduce on local WebKit and needs CI-based iteration - // on the shell to fix. Skip ONLY this half on WebKit (tracked as follow-up); - // Chromium and Firefox still verify the full wiring, and WebKit keeps the - // structural checks above. See docs/process-hardening.md "Cross-browser test - // robustness". - if (browserName === "webkit") return; - - // Under client-only (ssr:false) rendering the shell re-syncs its query from - // the URL on mount via requestAnimationFrame. On Firefox that frame can land - // right after a programmatic fill — wiping the value, disabling the submit, or - // dropping the submit before the router navigates. Drive the fill-and-submit - // as one retried unit until the search actually routes to the forms results - // URL; the assertions below still verify the result. + // The shell now seeds its composer state from the URL and only re-syncs on a + // real navigation, so a programmatic fill on this no-query detail route is no + // longer wiped by a mount-time frame — the race that used to break CI WebKit + // (and could flake Firefox). Drive the fill-and-submit as one retried unit + // regardless, so any residual cross-browser navigation-timing jitter cannot + // flake the route assertion; the assertions below still verify the result. const formsSearchButton = page.getByRole("button", { name: "Search forms" }); await expect(async () => { // A previous attempt's click may have navigated late — after the inner URL