mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
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.
This commit is contained in:
1 parent
4bb03600b3
commit
4db7fc5113
3 files changed
+139
-2
No files matched your search
@@ -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<boolean> {
|
||||
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) => {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
|
||||
|
||||
Reference in new issue
Block a user