diff --git a/packages/ui/src/features/canvas/components/WebsiteLayout.tsx b/packages/ui/src/features/canvas/components/WebsiteLayout.tsx index 9981a8e24c..a924ce011d 100644 --- a/packages/ui/src/features/canvas/components/WebsiteLayout.tsx +++ b/packages/ui/src/features/canvas/components/WebsiteLayout.tsx @@ -1,4 +1,5 @@ import { + ArrowClockwiseIcon, DotsThreeIcon, GitForkIcon, LinkIcon, @@ -18,6 +19,8 @@ import { ChannelBreadcrumb } from "@posthog/ui/features/canvas/components/Channe import { iconForTemplate } from "@posthog/ui/features/canvas/components/canvasTemplateIcon"; import { NewCanvasMenu } from "@posthog/ui/features/canvas/components/NewCanvasMenu"; import { CanvasFrameHost } from "@posthog/ui/features/canvas/freeform/CanvasFrameHost"; +import { useCanvasFrameStore } from "@posthog/ui/features/canvas/freeform/canvasFrameStore"; +import { CANVAS_QUERY_KEY } from "@posthog/ui/features/canvas/freeform/freeformDataBridge"; import { useChannels } from "@posthog/ui/features/canvas/hooks/useChannels"; import { useChannelTasks } from "@posthog/ui/features/canvas/hooks/useChannelTasks"; import { @@ -39,6 +42,7 @@ import { toast } from "@posthog/ui/primitives/toast"; import { track } from "@posthog/ui/shell/analytics"; import { useHeaderStore } from "@posthog/ui/shell/headerStore"; import { Box, Flex } from "@radix-ui/themes"; +import { useQueryClient } from "@tanstack/react-query"; import { Outlet, useNavigate, @@ -106,6 +110,23 @@ function FreeformEditControls({ const revert = useFreeformChatStore((s) => s.revert); const goToLatest = useFreeformChatStore((s) => s.goToLatest); + const queryClient = useQueryClient(); + const remountFrame = useCanvasFrameStore((s) => s.remount); + // Fully remount the mounted canvas iframe: drop the host-side read cache so + // queries re-run, then recreate the iframe element (not just reload its + // document) so a refresh also recovers from a wedged frame. + const onRefresh = () => { + track(ANALYTICS_EVENTS.DASHBOARD_ACTION, { + action_type: "refresh", + surface: "canvas", + channel_id: channelId, + dashboard_id: dashboardId, + kind: "freeform", + }); + void queryClient.invalidateQueries({ queryKey: [CANVAS_QUERY_KEY] }); + remountFrame(dashboardId); + }; + const hasCode = code.length > 0; // Viewing the head version (or there's no history yet) → autosave is live. // Otherwise the user has undone to an older version and is browsing. @@ -212,6 +233,10 @@ function FreeformEditControls({ } /> + + + Refresh + void copyCanvasLink(channelId, dashboardId, "canvas") diff --git a/packages/ui/src/features/canvas/freeform/CanvasFrameHost.tsx b/packages/ui/src/features/canvas/freeform/CanvasFrameHost.tsx index 56aaa920d9..d64bf9d90c 100644 --- a/packages/ui/src/features/canvas/freeform/CanvasFrameHost.tsx +++ b/packages/ui/src/features/canvas/freeform/CanvasFrameHost.tsx @@ -11,6 +11,7 @@ import { FreeformCanvas } from "./FreeformCanvas"; export function CanvasFrameHost() { const slots = useCanvasFrameStore((s) => s.slots); const activeDashboardId = useCanvasFrameStore((s) => s.activeDashboardId); + const frameKeys = useCanvasFrameStore((s) => s.frameKeys); return ( // Fixed, viewport-filling, click-through layer. Children are positioned in @@ -43,7 +44,9 @@ export function CanvasFrameHost() { // Keyed by the physical frame's identity (its slot index), NOT dashboardId: // reassigning a slot to a new canvas must reuse the same iframe (init // code-swap), not remount it — remounting re-parents the iframe = reload. - const frameKey = `slot-${slotId}`; + // The remount generation (bumped only by an explicit user Refresh) is + // folded in so that action — and only that action — recreates the iframe. + const frameKey = `slot-${slotId}-${frameKeys[slotId] ?? 0}`; return (
@@ -51,7 +54,6 @@ export function CanvasFrameHost() { code={slot.inputs.code} mode="edit" analytics={slot.inputs.analytics} - refreshKey={slot.inputs.refreshKey} onDataRequest={slot.inputs.onDataRequest} onError={slot.inputs.onError} onRendered={slot.inputs.onRendered} diff --git a/packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.test.tsx b/packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.test.tsx new file mode 100644 index 0000000000..0c50f86ee7 --- /dev/null +++ b/packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.test.tsx @@ -0,0 +1,62 @@ +import { cleanup, render } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { CanvasFramePlaceholder } from "./CanvasFramePlaceholder"; +import { useCanvasFrameStore } from "./canvasFrameStore"; + +// The warm-frame host only shows a canvas once its slot has a measured rect +// (`active = ... && !!slot.rect`). The placeholder must populate that rect from +// its own synchronous measure on mount, WITHOUT relying on a later +// ResizeObserver/scroll re-measure — on a settled layout no such re-measure +// fires, so a canvas opened while the app has been running a while (e.g. via a +// deep link) would otherwise stay hidden until a hard refresh. The jsdom +// ResizeObserver stub never fires, so this test reproduces that settled-layout +// condition: if the rect isn't captured synchronously, it never is. + +const RECT = { top: 10, left: 20, width: 800, height: 600 }; + +function resetStore() { + useCanvasFrameStore.setState({ + slots: [], + activeDashboardId: null, + maxWarmFrames: 2, + }); +} + +describe("CanvasFramePlaceholder", () => { + beforeEach(() => { + resetStore(); + vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockReturnValue({ + ...RECT, + right: RECT.left + RECT.width, + bottom: RECT.top + RECT.height, + x: RECT.left, + y: RECT.top, + toJSON: () => "", + } as DOMRect); + }); + + afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + resetStore(); + }); + + it("captures the slot rect on mount without a follow-up re-measure", () => { + render( + null} + />, + ); + + const slot = useCanvasFrameStore + .getState() + .slots.find((s) => s?.dashboardId === "dash-1"); + expect(slot).toBeTruthy(); + // Pre-fix this is null (the slot is registered in a passive effect that runs + // after the layout-effect measure, so the first measure is dropped). + expect(slot?.rect).toEqual(RECT); + expect(useCanvasFrameStore.getState().activeDashboardId).toBe("dash-1"); + }); +}); diff --git a/packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.tsx b/packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.tsx index fb56f90e44..b3c1105f82 100644 --- a/packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.tsx +++ b/packages/ui/src/features/canvas/freeform/CanvasFramePlaceholder.tsx @@ -2,8 +2,7 @@ import type { CanvasAnalyticsConfig, CanvasNavIntent, } from "@posthog/core/canvas/freeformSchemas"; -import { useEffect, useLayoutEffect, useMemo, useRef } from "react"; -import { useCanvasRefreshNonce } from "../stores/canvasRefreshStore"; +import { useLayoutEffect, useMemo, useRef } from "react"; import { useCanvasFrameStore } from "./canvasFrameStore"; // Stands in for the canvas inside the route tree. It renders nothing visible — @@ -29,7 +28,6 @@ export function CanvasFramePlaceholder({ onNavigate?: (intent: CanvasNavIntent) => void; }) { const ref = useRef(null); - const refreshKey = useCanvasRefreshNonce(`dashboard:${dashboardId}`); const register = useCanvasFrameStore((s) => s.register); const setRect = useCanvasFrameStore((s) => s.setRect); @@ -40,24 +38,21 @@ export function CanvasFramePlaceholder({ () => ({ code, analytics, - refreshKey, onDataRequest, onError, onRendered, onNavigate, }), - [ - code, - analytics, - refreshKey, - onDataRequest, - onError, - onRendered, - onNavigate, - ], + [code, analytics, onDataRequest, onError, onRendered, onNavigate], ); - useEffect(() => { + // Layout effect (not passive) and declared first, so the slot exists before the + // rect-measure effect below runs its initial synchronous measure. Otherwise the + // slot is created too late (setRect no-ops with no slot) and the frame only + // becomes visible once a later scroll/resize re-measures — which never happens + // on a settled layout, so a canvas opened while the app has been running a while + // (e.g. via a deep link) stays hidden until a hard refresh. + useLayoutEffect(() => { register(dashboardId, inputs); }, [dashboardId, inputs, register]); diff --git a/packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx b/packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx index 9a64218654..db0201d6f2 100644 --- a/packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx +++ b/packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx @@ -45,12 +45,6 @@ export interface FreeformCanvasProps { * never crosses into the iframe. */ analytics?: CanvasAnalyticsConfig; - /** - * Bump to force a full iframe reload (a fresh boot re-runs the app's data - * `useEffect`s = a real refresh). Folded into the srcDoc so changing it - * reloads the frame via the existing reload path. - */ - refreshKey?: number; } // Renders a freeform-React canvas inside a null-origin sandboxed iframe and @@ -64,7 +58,6 @@ export function FreeformCanvas({ onRendered, onNavigate, analytics, - refreshKey = 0, }: FreeformCanvasProps) { const iframeRef = useRef(null); // The canvas mirrors the host's light/dark theme. Passed via `init` (not the @@ -79,12 +72,10 @@ export function FreeformCanvas({ // for posthog-js), not on code: code is injected via `init`, so changing it // never reloads the iframe — it re-renders in place. const analyticsHost = analytics?.apiHost; - const srcDoc = useMemo(() => { - const doc = buildSandboxDocument(mode, analyticsHost); - // Append the refresh nonce as a comment so a bump changes the srcDoc string, - // which reloads the iframe (re-announces "ready" → re-init → fresh run). - return refreshKey > 0 ? `${doc}\n` : doc; - }, [mode, analyticsHost, refreshKey]); + const srcDoc = useMemo( + () => buildSandboxDocument(mode, analyticsHost), + [mode, analyticsHost], + ); // Latest props, read by the once-bound listener + the (stable) postInit. const latest = useRef({ diff --git a/packages/ui/src/features/canvas/freeform/canvasFrameStore.test.ts b/packages/ui/src/features/canvas/freeform/canvasFrameStore.test.ts index 2fba1c80da..5a0426380b 100644 --- a/packages/ui/src/features/canvas/freeform/canvasFrameStore.test.ts +++ b/packages/ui/src/features/canvas/freeform/canvasFrameStore.test.ts @@ -5,7 +5,7 @@ import { } from "./canvasFrameStore"; function inputs(code: string): CanvasFrameInputs { - return { code, refreshKey: 0, onDataRequest: vi.fn() }; + return { code, onDataRequest: vi.fn() }; } function reset() { @@ -13,6 +13,7 @@ function reset() { slots: [], activeDashboardId: null, maxWarmFrames: 2, + frameKeys: {}, }); } @@ -88,6 +89,38 @@ describe("canvasFrameStore", () => { expect(useCanvasFrameStore.getState().slots).toBe(after); }); + it("remount bumps only the frame generation of the canvas's slot", () => { + const { register, remount } = useCanvasFrameStore.getState(); + register("a", inputs("A")); + register("b", inputs("B")); + + remount("a"); + expect(useCanvasFrameStore.getState().frameKeys[0]).toBe(1); + expect(useCanvasFrameStore.getState().frameKeys[1] ?? 0).toBe(0); + }); + + it("remount is a no-op for a canvas with no slot", () => { + const { remount } = useCanvasFrameStore.getState(); + const before = useCanvasFrameStore.getState().frameKeys; + remount("missing"); + expect(useCanvasFrameStore.getState().frameKeys).toBe(before); + }); + + it("keeps a slot's remount generation when it is reassigned (warm reuse)", () => { + const { register, activate, remount } = useCanvasFrameStore.getState(); + register("a", inputs("A")); + activate("a"); + remount("a"); // slot 0 generation -> 1 + register("b", inputs("B")); + activate("b"); + register("c", inputs("C")); // evicts LRU "a", "c" takes slot 0 + + expect(slotIndexOf("c")).toBe(0); + // Reassigning slot 0 to a different canvas must NOT change its key, so the + // warm iframe is reused (code-swap) rather than remounted on navigation. + expect(useCanvasFrameStore.getState().frameKeys[0]).toBe(1); + }); + it("deactivate clears the active id only when it matches", () => { const { register, activate, deactivate } = useCanvasFrameStore.getState(); register("a", inputs("A")); diff --git a/packages/ui/src/features/canvas/freeform/canvasFrameStore.ts b/packages/ui/src/features/canvas/freeform/canvasFrameStore.ts index 779d0697b2..4133fd785c 100644 --- a/packages/ui/src/features/canvas/freeform/canvasFrameStore.ts +++ b/packages/ui/src/features/canvas/freeform/canvasFrameStore.ts @@ -30,7 +30,6 @@ export interface CanvasFrameRect { export interface CanvasFrameInputs { code: string; analytics?: CanvasAnalyticsConfig; - refreshKey: number; onDataRequest: (method: string, payload: unknown) => Promise; onError?: (message: string, stack?: string) => void; onRendered?: () => void; @@ -49,11 +48,18 @@ interface CanvasFrameStore { slots: (CanvasFrameSlot | null)[]; activeDashboardId: string | null; maxWarmFrames: number; + // Per-slot remount generation. Folded into the frame's React key by the host, + // so bumping it tears down and recreates that slot's iframe element (a true + // remount, unlike a code-swap). Keyed by SLOT INDEX, not canvas, and only ever + // bumped by an explicit user refresh — so reassigning a slot to another canvas + // (navigation) leaves it untouched and still reuses the warm iframe. + frameKeys: Record; register: (dashboardId: string, inputs: CanvasFrameInputs) => void; setRect: (dashboardId: string, rect: CanvasFrameRect) => void; activate: (dashboardId: string) => void; deactivate: (dashboardId: string) => void; + remount: (dashboardId: string) => void; setMaxWarmFrames: (n: number) => void; } @@ -118,6 +124,7 @@ export const useCanvasFrameStore = create()((set) => ({ slots: [], activeDashboardId: null, maxWarmFrames: DEFAULT_MAX_WARM_FRAMES, + frameKeys: {}, register: (dashboardId, inputs) => set((s) => ({ @@ -168,6 +175,18 @@ export const useCanvasFrameStore = create()((set) => ({ s.activeDashboardId === dashboardId ? { activeDashboardId: null } : s, ), + // Force a full remount of the canvas's mounted iframe (recreate the element + + // its sandboxed document), for a manual "Refresh" that must recover from a + // wedged frame, not just reload the document. No-op if the canvas has no slot. + remount: (dashboardId) => + set((s) => { + const idx = findSlot(s.slots, dashboardId); + if (idx < 0) return s; + return { + frameKeys: { ...s.frameKeys, [idx]: (s.frameKeys[idx] ?? 0) + 1 }, + }; + }), + setMaxWarmFrames: (n) => set((s) => ({ maxWarmFrames: Math.max(1, n), diff --git a/packages/ui/src/features/canvas/stores/canvasRefreshStore.ts b/packages/ui/src/features/canvas/stores/canvasRefreshStore.ts deleted file mode 100644 index 35f7c37840..0000000000 --- a/packages/ui/src/features/canvas/stores/canvasRefreshStore.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { create } from "zustand"; - -// View-state bridge between the toolbar Refresh button and a freeform canvas: -// the button and the iframe live in separate subtrees, connected only by the -// canvas thread id. Bumping a thread's nonce reloads its sandbox iframe, which -// re-mounts the React app and re-runs its `ph.query` calls (fresh data). -// -// NB: canvas reads are cached host-side (see freeformDataBridge), so a refresh -// trigger should also invalidate the relevant `CANVAS_QUERY_KEY` entries via the -// QueryClient — done at the call site (which has `useQueryClient`), not here: a -// store holds state only and must not reach into a client. -interface CanvasRefreshStore { - nonces: Record; - bump: (threadId: string) => void; -} - -export const useCanvasRefreshStore = create()((set) => ({ - nonces: {}, - bump: (threadId) => - set((s) => ({ - nonces: { ...s.nonces, [threadId]: (s.nonces[threadId] ?? 0) + 1 }, - })), -})); - -export function useCanvasRefreshNonce(threadId: string): number { - return useCanvasRefreshStore((s) => s.nonces[threadId] ?? 0); -}