From 4db7fc511354d316919509f44bf2de632f164d34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 5 Oct 2026 08:21:52 +0200 Subject: [PATCH] fix(coach): open the deep-linked sheet only once the viewport is known /coach?settings=data opened the conversations sheet from the first render. The server and hydration renders do not know the viewport and take the narrow layout, so on a desktop the sheet was open until the docked panel replaced it; only the batching of the hydration re-render kept it off the screen. The deep link now opens the sheet only once hydrated on a viewport that does not dock the panel; on a docked one the panel is revealed as before. The desktop deep-link spec records every mount of the sheet from the first byte and expects none; a phone deep-link spec covers the sheet path and serves as the watcher's positive control. --- e2e/coach-layout.spec.ts | 62 +++++++++++++++++++ .../conversations-panel-sheet.test.ts | 37 +++++++++++ .../coach-panel/conversations-panel.tsx | 42 ++++++++++++- 3 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 src/components/insights/coach-panel/__tests__/conversations-panel-sheet.test.ts diff --git a/e2e/coach-layout.spec.ts b/e2e/coach-layout.spec.ts index a4a051e4a..b933afd30 100644 --- a/e2e/coach-layout.spec.ts +++ b/e2e/coach-layout.spec.ts @@ -238,6 +238,39 @@ async function shot(page: Page, testInfo: TestInfo, name: string) { if (dir) await page.screenshot({ path: `${dir}/${name}.png` }); } +/** + * Record every mount of the conversations sheet from the first byte on. + * Reads the added nodes rather than the live DOM, so a sheet that mounts and + * unmounts again before the observer's callback runs is still counted. + */ +async function watchSheetMounts(page: Page) { + await page.addInitScript(() => { + const w = window as unknown as { __coachSheetSeen?: boolean }; + w.__coachSheetSeen = false; + const sheet = '[role="dialog"][data-slot="coach-conversations-panel"]'; + new MutationObserver((records) => { + for (const record of records) { + for (const node of record.addedNodes) { + if ( + node instanceof Element && + (node.matches(sheet) || node.querySelector(sheet)) + ) { + w.__coachSheetSeen = true; + } + } + } + }).observe(document, { childList: true, subtree: true }); + }); +} + +async function sheetMounted(page: Page): Promise { + return page.evaluate( + () => + (window as unknown as { __coachSheetSeen?: boolean }).__coachSheetSeen === + true, + ); +} + async function openCoach(page: Page, path = "/coach?c=frame-bp") { await page.goto(path, { waitUntil: "domcontentloaded" }); await expect( @@ -460,11 +493,18 @@ test.describe("Coach page frame", () => { await page.addInitScript(() => { window.localStorage.setItem("healthlog.coach.panelOpen", "false"); }); + // The hydration render does not know the viewport yet; the phone sheet + // must not mount on a desktop even for a moment. + await watchSheetMounts(page); await page.goto("/coach?settings=data", { waitUntil: "domcontentloaded" }); const popover = page.locator('[data-slot="coach-settings-popover"]'); await expect(popover).toBeVisible({ timeout: 15_000 }); await expect(panel(page)).toHaveAttribute("data-state", "open"); + expect( + await sheetMounted(page), + "the phone sheet flashed on a desktop deep link", + ).toBe(false); await expect( popover.locator('[data-slot="coach-settings-data"]'), ).toBeFocused(); @@ -523,6 +563,28 @@ test.describe("Coach page frame", () => { }); } + test("390: /coach?settings=data opens the sheet and the settings on what the Coach sees", async ({ + page, + }) => { + await page.setViewportSize({ width: 390, height: 844 }); + await mockCoach(page); + await watchSheetMounts(page); + await page.goto("/coach?settings=data", { waitUntil: "domcontentloaded" }); + + // The settings bottom sheet opens over the conversations sheet and + // hides it from the accessibility tree, so the sheet is found by slot. + await expect(page.locator('[data-slot="coach-settings-data"]')).toBeVisible( + { timeout: 15_000 }, + ); + await expect( + page.locator('[role="dialog"][data-slot="coach-conversations-panel"]'), + ).toBeVisible(); + // The positive control for the desktop case: the same watcher sees the + // sheet mount where it belongs. + expect(await sheetMounted(page)).toBe(true); + await expect(page).toHaveURL(/\/coach$/); + }); + test("390: the panel is a sheet, the gear a bottom sheet, and nothing scrolls sideways", async ({ page, }, testInfo) => { diff --git a/src/components/insights/coach-panel/__tests__/conversations-panel-sheet.test.ts b/src/components/insights/coach-panel/__tests__/conversations-panel-sheet.test.ts new file mode 100644 index 000000000..969157191 --- /dev/null +++ b/src/components/insights/coach-panel/__tests__/conversations-panel-sheet.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; + +import { sheetShown } from "../conversations-panel"; + +/** + * `/coach?settings=data` opens the phone sheet, and only the phone sheet. + * The server and hydration renders take the narrow layout (the viewport is + * not known yet), so a deep link that opened the sheet from the first + * render flashed it on a desktop before the docked panel replaced it. + */ +describe("sheetShown", () => { + const deepLink = { chosen: false, deepLinkPending: true }; + + it("keeps the sheet shut while the viewport is not known", () => { + expect(sheetShown({ ...deepLink, hydrated: false, docked: false })).toBe( + false, + ); + }); + + it("never opens the sheet for a deep link on a docked viewport", () => { + expect(sheetShown({ ...deepLink, hydrated: true, docked: true })).toBe( + false, + ); + }); + + it("opens the sheet for a deep link on a phone once hydrated", () => { + expect(sheetShown({ ...deepLink, hydrated: true, docked: false })).toBe( + true, + ); + }); + + it("follows the person's own choice otherwise", () => { + const base = { deepLinkPending: false, hydrated: true, docked: false }; + expect(sheetShown({ ...base, chosen: true })).toBe(true); + expect(sheetShown({ ...base, chosen: false })).toBe(false); + }); +}); diff --git a/src/components/insights/coach-panel/conversations-panel.tsx b/src/components/insights/coach-panel/conversations-panel.tsx index b9ec5df7e..c3082381b 100644 --- a/src/components/insights/coach-panel/conversations-panel.tsx +++ b/src/components/insights/coach-panel/conversations-panel.tsx @@ -1,6 +1,6 @@ "use client"; -import { useRef, useState } from "react"; +import { useRef, useState, useSyncExternalStore } from "react"; import Link from "next/link"; import { PanelRightClose, @@ -48,6 +48,25 @@ import { useDeleteCoachConversationWithUndo } from "./use-coach"; */ export const COACH_PANEL_ID = "coach-conversations-panel"; +const noSubscription = () => () => {}; + +/** + * Whether the phone sheet is open. A settings deep link opens it only once + * the viewport is known: the server and hydration renders take the narrow + * (sheet) layout, so opening it from the first render flashed the sheet on + * a desktop before the docked panel replaced it. + */ +export function sheetShown(state: { + chosen: boolean; + deepLinkPending: boolean; + hydrated: boolean; + docked: boolean; +}): boolean { + return ( + state.chosen || (state.deepLinkPending && state.hydrated && !state.docked) + ); +} + export interface ConversationsPanelProps { activeId: string | null; onSelect: (id: string) => void; @@ -75,7 +94,26 @@ export function ConversationsPanel({ // A settings deep-link shows the panel without rewriting the remembered // choice; the next toggle settles it. const [revealed, setRevealed] = useState(openSettingsOnData); - const [sheetOpen, setSheetOpen] = useState(openSettingsOnData); + const hydrated = useSyncExternalStore( + noSubscription, + () => true, + () => false, + ); + const [deepLinkPending, setDeepLinkPending] = useState(openSettingsOnData); + const [sheetChosen, setSheetChosen] = useState(false); + const sheetOpen = sheetShown({ + chosen: sheetChosen, + deepLinkPending, + hydrated, + docked, + }); + // On a docked viewport the deep link is served by `revealed`; it must not + // open the sheet later if the window narrows. + if (deepLinkPending && hydrated && docked) setDeepLinkPending(false); + const setSheetOpen = (next: boolean) => { + setDeepLinkPending(false); + setSheetChosen(next); + }; const [settingsOpen, setSettingsOpen] = useState(openSettingsOnData); const [settingsOnData, setSettingsOnData] = useState(openSettingsOnData);