diff --git a/apps/desktop/src/preview/GuestProtocol.ts b/apps/desktop/src/preview/GuestProtocol.ts index 1a73bb30f29e..d8f8ac2c88cd 100644 --- a/apps/desktop/src/preview/GuestProtocol.ts +++ b/apps/desktop/src/preview/GuestProtocol.ts @@ -2,6 +2,7 @@ export const START_PICK_CHANNEL = "preview:start-pick"; export const CANCEL_PICK_CHANNEL = "preview:cancel-pick"; export const ELEMENT_PICKED_CHANNEL = "preview:element-picked"; export const ANNOTATION_CAPTURED_CHANNEL = "preview:annotation-captured"; +export const ANNOTATION_DRAFT_CHANNEL = "preview:annotation-draft"; export const ANNOTATION_THEME_CHANNEL = "preview:annotation-theme"; export const HUMAN_INPUT_CHANNEL = "preview:human-input"; export const MOUSE_NAVIGATE_CHANNEL = "preview:mouse-navigate"; diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 0a949d39b5a4..2f442f5abbb3 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -3857,6 +3857,98 @@ describe("PreviewManager", () => { ), ); + effectIt.effect("keeps the markup when the page reloads during element picking", () => + withManager((manager) => + Effect.gen(function* () { + const listeners = new Map void>(); + let onDraft: ((event: unknown, value: unknown) => void) | undefined; + let currentUrl = "https://example.com/app#section"; + fromId.mockReturnValue({ + id: 42, + isDestroyed: () => false, + getType: () => "webview", + getURL: () => currentUrl, + getTitle: () => "Example", + isLoading: () => false, + isFocused: () => true, + getZoomFactor: () => 1, + setZoomFactor: vi.fn(), + setAudioMuted: vi.fn(), + isCurrentlyAudible: () => false, + on: vi.fn((event: string, listener: (...args: unknown[]) => void) => { + listeners.set(event, listener); + }), + once: vi.fn(), + off: vi.fn(), + ipc: { + on: vi.fn((channel: string, listener: typeof onDraft) => { + if (channel === "preview:annotation-draft") onDraft = listener; + }), + off: vi.fn(), + removeListener: vi.fn(), + }, + send: webviewSend, + navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), + setWindowOpenHandler: vi.fn(), + debugger: { + isAttached: () => false, + attach: vi.fn(), + sendCommand: vi.fn(async () => undefined), + on: vi.fn(), + off: vi.fn(), + }, + } as never); + const draft = { + comment: "Tighten this spacing", + tool: "marquee", + elements: [ + { + id: "element_1", + selector: ":root > body:nth-of-type(1) > main:nth-of-type(1)", + rect: { x: 1, y: 2, width: 30, height: 40 }, + }, + ], + regions: [{ id: "region_2", rect: { x: 5, y: 6, width: 20, height: 30 } }], + strokes: [], + styleChanges: [], + }; + const reload = (url: string) => { + listeners.get("did-start-navigation")?.({ + url, + isSameDocument: false, + isMainFrame: true, + frame: null, + }); + }; + + yield* manager.createTab("tab_1"); + yield* manager.registerWebview("tab_1", 42); + const pick = yield* manager.pickElement("tab_1").pipe(Effect.forkChild); + yield* Effect.yieldNow; + onDraft?.({}, draft); + webviewSend.mockClear(); + + // A dev-server refresh reloads the page it is already on. + reload("https://example.com/app"); + yield* Effect.yieldNow; + expect(pick.pollUnsafe()).toBeUndefined(); + + listeners.get("dom-ready")?.(); + yield* Effect.yieldNow; + expect(pick.pollUnsafe()).toBeUndefined(); + expect(webviewSend).toHaveBeenCalledWith("preview:start-pick", expect.anything(), draft); + + // A reload that redirects away ends the pick, as other navigations do. + reload("https://example.com/app"); + currentUrl = "https://example.com/login"; + listeners.get("dom-ready")?.(); + yield* Effect.yieldNow; + expect(yield* Fiber.join(pick)).toBeNull(); + }), + ), + ); + effectIt.effect("settles the pick when the annotation screenshot never arrives", () => withManager((manager) => Effect.gen(function* () { diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index 3593ae5f5df6..934edca246d3 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -72,6 +72,7 @@ import { PREVIEW_PICTURE_IN_PICTURE_FRAME_CHANNEL } from "../ipc/channels.ts"; import * as BrowserSession from "./BrowserSession.ts"; import { ANNOTATION_CAPTURED_CHANNEL, + ANNOTATION_DRAFT_CHANNEL, ANNOTATION_THEME_CHANNEL, CANCEL_PICK_CHANNEL, ELEMENT_PICKED_CHANNEL, @@ -84,7 +85,11 @@ import { RECORDING_CONTROLLER_CHANNEL, START_PICK_CHANNEL, } from "./GuestProtocol.ts"; -import { isPreviewAnnotationPayload } from "./PickedElementPayload.ts"; +import { + isPreviewAnnotationDraft, + isPreviewAnnotationPayload, + type PreviewAnnotationDraft, +} from "./PickedElementPayload.ts"; import { playwrightInjectedRuntimeInstallExpression } from "./PlaywrightInjectedRuntime.ts"; import { makePreviewAutomationKeySequence, @@ -345,6 +350,10 @@ const normalizeCaptureRect = (value: unknown): PreviewAnnotationRect | null => { }; }; +/** Whether two URLs load the same page, ignoring the fragment. */ +const isSamePageUrl = (left: string, right: string): boolean => + left.split("#", 1)[0] === right.split("#", 1)[0]; + /** `capturePage` never settles when the guest's compositor is wedged. */ const ANNOTATION_SCREENSHOT_TIMEOUT = "5 seconds"; @@ -2679,8 +2688,10 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const cleanup = Effect.fn("PreviewManager.cleanupPickElement")(function* () { yield* attempt({ operation: "pickElement.cleanup", tabId, webContentsId: wc.id }, () => { wc.ipc.removeListener(ELEMENT_PICKED_CHANNEL, onMessage); + wc.ipc.removeListener(ANNOTATION_DRAFT_CHANNEL, onDraft); wc.off("destroyed", onDestroyed); wc.off("did-start-navigation", onNavigated); + wc.off("dom-ready", onDomReady); }).pipe(Effect.ignore); // Only drop the slot while it is still ours. A newer session may // already have swapped itself in before cancelling this one. @@ -2737,12 +2748,19 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function } resume(Effect.succeed(null)); }); + // A reload of the page keeps the markup alive: the preload keeps the + // draft current here, and the fresh document gets it back on dom-ready. + const pageUrl = wc.getURL(); + let draft: PreviewAnnotationDraft | null = null; + let reloading = false; + let submitted = false; const onMessage = (_event: Electron.IpcMainEvent, ...args: unknown[]): void => { const payload = args[0]; if (!isPreviewAnnotationPayload(payload)) { settle(null); return; } + submitted = true; const cropRect = normalizeCaptureRect(args[1]); const submission = args[2] === "send" ? "send" : "attach"; runFork( @@ -2776,11 +2794,40 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function ), ); }; + const onDraft = (_event: Electron.IpcMainEvent, value: unknown): void => { + if (!submitted && isPreviewAnnotationDraft(value)) draft = value; + }; const onDestroyed = () => settle(null); const onNavigated = ( event: Electron.Event, ) => { - if (event.isMainFrame) settle(null); + if (!event.isMainFrame) return; + if (!event.isSameDocument && isSamePageUrl(event.url, pageUrl)) { + reloading = true; + return; + } + settle(null); + }; + const onDomReady = () => { + if (!reloading) return; + reloading = false; + // A submitted annotation is already capturing and settles on its own. + if (submitted) return; + // The reload redirected somewhere else, so the markup no longer applies. + if (!isSamePageUrl(wc.getURL(), pageUrl)) { + settle(null); + return; + } + runFork( + Ref.get(annotationThemeRef).pipe( + Effect.flatMap((theme) => + attempt({ operation: "pickElement.restore", tabId, webContentsId: wc.id }, () => + wc.send(START_PICK_CHANNEL, theme, draft), + ), + ), + Effect.ignore, + ), + ); }; const registerPickElement = Effect.fn("PreviewManager.registerPickElement")(function* () { // Two picks on one tab can overlap. Swap this session in and cancel @@ -2800,8 +2847,10 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function if (settled) return; yield* attempt({ operation: "pickElement.register", tabId, webContentsId: wc.id }, () => { wc.ipc.on(ELEMENT_PICKED_CHANNEL, onMessage); + wc.ipc.on(ANNOTATION_DRAFT_CHANNEL, onDraft); wc.once("destroyed", onDestroyed); wc.on("did-start-navigation", onNavigated); + wc.on("dom-ready", onDomReady); if (!wc.isFocused()) wc.focus(); wc.send(START_PICK_CHANNEL, annotationTheme); }); diff --git a/apps/desktop/src/preview/PickPreload.ts b/apps/desktop/src/preview/PickPreload.ts index 78351e9d79c3..58dc8bde02e7 100644 --- a/apps/desktop/src/preview/PickPreload.ts +++ b/apps/desktop/src/preview/PickPreload.ts @@ -16,10 +16,16 @@ import type { import { resolveAnnotationSubmission } from "./AnnotationKeyboard.ts"; import { previewAnnotationStyles } from "./AnnotationStyles.generated.ts"; +import { + isPreviewAnnotationDraft, + type PreviewAnnotationDraft, + type PreviewAnnotationTool, +} from "./PickedElementPayload.ts"; import { installRecordingCursor } from "./RecordingCursor.ts"; import { DEFAULT_RECORDING_INPUT_OPTIONS } from "./RecordingInput.ts"; import { ANNOTATION_CAPTURED_CHANNEL, + ANNOTATION_DRAFT_CHANNEL, ANNOTATION_THEME_CHANNEL, CANCEL_PICK_CHANNEL, ELEMENT_PICKED_CHANNEL, @@ -41,6 +47,9 @@ const MAX_MARQUEE_ELEMENTS = 20; const ELEMENT_CONTEXT_TIMEOUT_MS = 5_000; const CONTENT_LAYER_Z_INDEX = 1; const CHROME_LAYER_Z_INDEX = 10; +/** How long a restored draft keeps looking for elements the reloaded page renders late. */ +const DRAFT_RESTORE_TIMEOUT_MS = 5_000; +const DRAFT_RESTORE_INTERVAL_MS = 200; let recordingCursor: ReturnType | null = null; ipcRenderer.on( @@ -116,8 +125,6 @@ ipcRenderer.on(RECORDING_POINTER_CHANNEL, (_event, point: unknown) => { ); }); -type AnnotationTool = "select" | "marquee" | "draw" | "erase"; - interface SelectedElement { id: string; element: Element; @@ -129,6 +136,7 @@ interface SelectedElement { interface AnnotationSession { teardown: (notifyMain: boolean) => void; applyTheme: (theme: DesktopPreviewAnnotationTheme) => void; + syncDraft: () => void; } let activeSession: AnnotationSession | null = null; @@ -225,6 +233,14 @@ const nextId = (prefix: string): string => { return `${prefix}_${idSequence.toString(36)}`; }; +/** Keeps fresh ids from colliding with ids a restored draft brought along. */ +const reserveIds = (ids: ReadonlyArray): void => { + for (const id of ids) { + const sequence = Number.parseInt(id.slice(id.lastIndexOf("_") + 1), 36); + if (Number.isFinite(sequence)) idSequence = Math.max(idSequence, sequence); + } +}; + const rectFromDomRect = (rect: DOMRect): PreviewAnnotationRect => ({ x: rect.left, y: rect.top, @@ -281,6 +297,36 @@ function pickFromPoint(clientX: number, clientY: number): Element | null { return null; } +/** A CSS path that finds `element` again in a reloaded copy of the page. */ +function cssPathFor(element: Element): string { + const parts: string[] = []; + let current: Element | null = element; + while (current && current !== document.documentElement) { + if (current.id) { + const idSelector = `#${CSS.escape(current.id)}`; + if (document.querySelectorAll(idSelector).length === 1) { + parts.unshift(idSelector); + return parts.join(" > "); + } + } + const node: Element = current; + const siblings = node.parentElement ? Array.from(node.parentElement.children) : [node]; + const index = siblings.filter((sibling) => sibling.localName === node.localName).indexOf(node); + parts.unshift(`${CSS.escape(node.localName)}:nth-of-type(${index + 1})`); + current = node.parentElement; + } + return [":root", ...parts].join(" > "); +} + +function findBySelector(selector: string): Element | null { + try { + const element = document.querySelector(selector); + return element && !isAnnotationNode(element) ? element : null; + } catch { + return null; + } +} + function describeRawElement(element: Element): string { const tag = element.tagName.toLowerCase(); const id = element.id ? `#${element.id}` : ""; @@ -512,7 +558,7 @@ function strokeBounds( return { x: left, y: top, width: right - left, height: bottom - top }; } -function startAnnotation(): void { +function startAnnotation(draft: PreviewAnnotationDraft | null): void { activeSession?.teardown(false); let finished = false; const host = document.createElement("div"); @@ -603,8 +649,8 @@ function startAnnotation(): void { const regions: PreviewAnnotationRegionTarget[] = []; const strokes: PreviewAnnotationStrokeTarget[] = []; const styleChanges = new Map(); - const toolButtons = new Map(); - let tool: AnnotationTool = "select"; + const toolButtons = new Map(); + let tool: PreviewAnnotationTool = "select"; let dragStart: PreviewAnnotationPoint | null = null; let activeStroke: { target: PreviewAnnotationStrokeTarget; path: SVGPathElement } | null = null; let pendingCapture = false; @@ -613,6 +659,40 @@ function startAnnotation(): void { let editorPosition: { left: number; top: number } | null = null; let editorDrag: { pointerId: number; offsetX: number; offsetY: number } | null = null; let editorLayoutFrame: number | null = null; + let restoreTimer: number | null = null; + // Restored elements the reloaded page has not rendered yet. Each one holds + // its old spot as a placeholder region until it turns up. + const unresolved: Array< + PreviewAnnotationDraft["elements"][number] & { + placeholderId: string; + styleChanges: ReadonlyArray; + } + > = []; + + // Main keeps the latest copy so a reload of the page can hand it back. + const syncDraft = (): void => { + if (pendingCapture) return; + const placeholderIds = new Set(unresolved.map((element) => element.placeholderId)); + const snapshot: PreviewAnnotationDraft = { + comment: comment.value, + tool, + elements: [ + ...Array.from(selected.values(), (target) => ({ + id: target.id, + selector: cssPathFor(target.element), + rect: rectFromDomRect(target.element.getBoundingClientRect()), + })), + ...unresolved.map(({ id, selector, rect }) => ({ id, selector, rect })), + ], + regions: regions.filter((region) => !placeholderIds.has(region.id)), + strokes, + styleChanges: [ + ...styleChanges.values(), + ...unresolved.flatMap((element) => element.styleChanges), + ], + }; + ipcRenderer.send(ANNOTATION_DRAFT_CHANNEL, snapshot); + }; const resizeComment = (): void => { const maxHeight = 96; @@ -622,7 +702,10 @@ function startAnnotation(): void { comment.style.overflowY = comment.scrollHeight > maxHeight ? "auto" : "hidden"; queueEditorLayout(); }; - comment.addEventListener("input", resizeComment); + comment.addEventListener("input", () => { + resizeComment(); + syncDraft(); + }); const updateStatus = (): void => { const hasTargets = selected.size > 0 || regions.length > 0 || strokes.length > 0; @@ -636,6 +719,7 @@ function startAnnotation(): void { editorWasShown = true; window.setTimeout(() => comment.focus({ preventScroll: true }), 0); } + syncDraft(); }; const refreshToolButtons = (): void => { @@ -648,6 +732,7 @@ function startAnnotation(): void { if (tool !== "select") hoverOutline.style.display = "none"; if (tool !== "marquee") marqueeBox.style.display = "none"; document.documentElement.setAttribute("data-t3code-annotation-tool", tool); + syncDraft(); }; const removeSelected = (target: SelectedElement): void => { @@ -666,10 +751,10 @@ function startAnnotation(): void { updateStatus(); }; - const addSelected = (element: Element): void => { - if (selected.has(element)) return; + const addSelected = (element: Element, id = nextId("element")): SelectedElement | null => { + if (selected.has(element)) return null; const target: SelectedElement = { - id: nextId("element"), + id, element, outline: createBox(PRIMARY, PRIMARY_FILL), label: createLabel(), @@ -683,6 +768,7 @@ function startAnnotation(): void { stylePanel.style.display = "grid"; syncStyleControls(); } + return target; }; const toggleSelected = (element: Element, additive: boolean): void => { @@ -697,27 +783,38 @@ function startAnnotation(): void { addSelected(element); }; + const applyStyleChange = ( + target: SelectedElement, + change: PreviewAnnotationStyleChange, + ): void => { + if (!(target.element instanceof HTMLElement || target.element instanceof SVGElement)) return; + if (!target.baselineStyles.has(change.property)) { + target.baselineStyles.set( + change.property, + target.element.style.getPropertyValue(change.property), + ); + } + target.element.style.setProperty(change.property, change.value, "important"); + styleChanges.set(`${target.id}:${change.property}`, change); + updateSelectedVisual(target); + }; + const setStyleForSelected = (property: string, value: string): void => { for (const target of selected.values()) { if (!(target.element instanceof HTMLElement || target.element instanceof SVGElement)) continue; - if (!target.baselineStyles.has(property)) { - target.baselineStyles.set(property, target.element.style.getPropertyValue(property)); - } - const key = `${target.id}:${property}`; const previousValue = - styleChanges.get(key)?.previousValue ?? + styleChanges.get(`${target.id}:${property}`)?.previousValue ?? getComputedStyle(target.element).getPropertyValue(property).trim(); - target.element.style.setProperty(property, value, "important"); - styleChanges.set(key, { + applyStyleChange(target, { targetId: target.id, selector: null, property, previousValue, value, }); - updateSelectedVisual(target); } + syncDraft(); }; const textSection = createStyleSection(); @@ -943,7 +1040,7 @@ function startAnnotation(): void { gap.value = computed.gap === "normal" ? "0px" : computed.gap; }; - const tools: ReadonlyArray<[AnnotationTool, string, string]> = [ + const tools: ReadonlyArray<[PreviewAnnotationTool, string, string]> = [ ["select", "Select", "Select elements (V)"], ["marquee", "Region", "Draw a region or marquee-select elements (R)"], ["draw", "Draw", "Draw freehand (D)"], @@ -1117,8 +1214,7 @@ function startAnnotation(): void { y <= region.rect.y + region.rect.height, ); if (regionIndex >= 0) { - const [removed] = regions.splice(regionIndex, 1); - root.querySelector(`[data-region-id="${removed?.id}"]`)?.remove(); + removeRegion(regions[regionIndex]!.id); updateStatus(); return true; } @@ -1173,6 +1269,36 @@ function startAnnotation(): void { return candidates.length; }; + const addRegion = (region: PreviewAnnotationRegionTarget): void => { + regions.push(region); + const regionBox = createBox(PRIMARY, "color-mix(in srgb, var(--t3-primary) 6%, transparent)"); + regionBox.setAttribute("data-region-id", region.id); + positionBox(regionBox, region.rect); + root.appendChild(regionBox); + }; + + const removeRegion = (regionId: string): void => { + const index = regions.findIndex((region) => region.id === regionId); + if (index >= 0) regions.splice(index, 1); + root.querySelector(`[data-region-id="${regionId}"]`)?.remove(); + const unresolvedIndex = unresolved.findIndex((element) => element.placeholderId === regionId); + if (unresolvedIndex >= 0) unresolved.splice(unresolvedIndex, 1); + }; + + const createStrokePath = (stroke: PreviewAnnotationStrokeTarget): SVGPathElement => { + const path = document.createElementNS("http://www.w3.org/2000/svg", "path"); + path.setAttribute(OVERLAY_ATTRIBUTE, ""); + path.setAttribute("data-stroke-id", stroke.id); + path.setAttribute("fill", "none"); + path.setAttribute("stroke", stroke.color); + path.setAttribute("stroke-width", String(stroke.width)); + path.setAttribute("stroke-linecap", "round"); + path.setAttribute("stroke-linejoin", "round"); + path.setAttribute("d", pathFromPoints(stroke.points)); + svg.appendChild(path); + return path; + }; + const clearHoverOutline = (): void => { hoverOutline.style.display = "none"; }; @@ -1231,16 +1357,7 @@ function startAnnotation(): void { points: [dragStart], bounds: { x: dragStart.x, y: dragStart.y, width: 1, height: 1 }, }; - const path = document.createElementNS("http://www.w3.org/2000/svg", "path"); - path.setAttribute(OVERLAY_ATTRIBUTE, ""); - path.setAttribute("data-stroke-id", stroke.id); - path.setAttribute("fill", "none"); - path.setAttribute("stroke", stroke.color); - path.setAttribute("stroke-width", String(stroke.width)); - path.setAttribute("stroke-linecap", "round"); - path.setAttribute("stroke-linejoin", "round"); - svg.appendChild(path); - activeStroke = { target: stroke, path }; + activeStroke = { target: stroke, path: createStrokePath(stroke) }; } }; @@ -1253,17 +1370,7 @@ function startAnnotation(): void { marqueeBox.style.display = "none"; if (isUsableRect(rect)) { const found = selectElementsInRect(rect); - if (found === 0) { - const region: PreviewAnnotationRegionTarget = { id: nextId("region"), rect }; - regions.push(region); - const regionBox = createBox( - PRIMARY, - "color-mix(in srgb, var(--t3-primary) 6%, transparent)", - ); - regionBox.setAttribute("data-region-id", region.id); - positionBox(regionBox, rect); - root.appendChild(regionBox); - } + if (found === 0) addRegion({ id: nextId("region"), rect }); } } else if (tool === "draw" && activeStroke) { if (activeStroke.target.points.length > 1) strokes.push(activeStroke.target); @@ -1317,6 +1424,7 @@ function startAnnotation(): void { dragHandle.removeEventListener("pointerup", onEditorPointerUp); dragHandle.removeEventListener("pointercancel", onEditorPointerUp); if (editorLayoutFrame !== null) window.cancelAnimationFrame(editorLayoutFrame); + if (restoreTimer !== null) window.clearTimeout(restoreTimer); ipcRenderer.off(CANCEL_PICK_CHANNEL, onCancel); ipcRenderer.off(ANNOTATION_CAPTURED_CHANNEL, onCaptured); document.documentElement.removeAttribute("data-t3code-annotation-tool"); @@ -1347,6 +1455,7 @@ function startAnnotation(): void { const submitAnnotation = (submission: PreviewAnnotationSubmission): void => { if (pendingCapture || (selected.size === 0 && regions.length === 0 && strokes.length === 0)) return; + syncDraft(); pendingCapture = true; submit.disabled = true; submit.textContent = "Capturing…"; @@ -1428,18 +1537,79 @@ function startAnnotation(): void { ipcRenderer.on(CANCEL_PICK_CHANNEL, onCancel); ipcRenderer.on(ANNOTATION_CAPTURED_CHANNEL, onCaptured); document.documentElement.appendChild(host); + if (draft) restoreDraft(draft); refreshToolButtons(); updateStatus(); activeSession = { teardown, applyTheme: (theme) => applyAnnotationTheme(host, theme), + syncDraft, }; + + /** + * Rebuilds the markup a reload interrupted. Apps often render after + * DOMContentLoaded, so an element that is not there yet holds its old spot + * as a region and is swapped back in if it shows up before the timeout. + */ + function restoreDraft(restored: PreviewAnnotationDraft): void { + reserveIds([ + ...restored.elements.map((element) => element.id), + ...restored.regions.map((region) => region.id), + ...restored.strokes.map((stroke) => stroke.id), + ]); + comment.value = restored.comment; + resizeComment(); + tool = restored.tool; + for (const region of restored.regions) addRegion(region); + for (const stroke of restored.strokes) { + strokes.push(stroke); + createStrokePath(stroke); + } + for (const element of restored.elements) { + const placeholderId = nextId("region"); + unresolved.push({ + ...element, + placeholderId, + styleChanges: restored.styleChanges.filter((change) => change.targetId === element.id), + }); + addRegion({ id: placeholderId, rect: element.rect }); + } + const deadline = Date.now() + DRAFT_RESTORE_TIMEOUT_MS; + const resolveElements = (): void => { + restoreTimer = null; + if (finished || pendingCapture) return; + // removeRegion drops each match from `unresolved`, so collect first. + const matches = unresolved.flatMap((element) => { + const found = findBySelector(element.selector); + return found ? [{ element, found }] : []; + }); + for (const { element, found } of matches) { + removeRegion(element.placeholderId); + // The user may have selected it again by hand while it was missing. + const target = selected.get(found) ?? addSelected(found, element.id); + if (!target) continue; + for (const change of element.styleChanges) { + applyStyleChange(target, { ...change, targetId: target.id }); + } + } + updateStatus(); + if (unresolved.length > 0 && Date.now() < deadline) { + restoreTimer = window.setTimeout(resolveElements, DRAFT_RESTORE_INTERVAL_MS); + } + }; + resolveElements(); + } } -ipcRenderer.on(START_PICK_CHANNEL, (_event, theme: DesktopPreviewAnnotationTheme | undefined) => { - if (theme) annotationTheme = theme; - startAnnotation(); -}); +ipcRenderer.on( + START_PICK_CHANNEL, + (_event, theme: DesktopPreviewAnnotationTheme | undefined, draft: unknown) => { + if (theme) annotationTheme = theme; + startAnnotation(isPreviewAnnotationDraft(draft) ? draft : null); + }, +); +// Element rects drift as the page scrolls, so hand main a fresh copy on the way out. +window.addEventListener("pagehide", () => activeSession?.syncDraft()); ipcRenderer.on(ANNOTATION_THEME_CHANNEL, (_event, theme: DesktopPreviewAnnotationTheme) => { annotationTheme = theme; recordingCursor?.setTheme(theme); diff --git a/apps/desktop/src/preview/PickedElementPayload.test.ts b/apps/desktop/src/preview/PickedElementPayload.test.ts index d7a967324771..1f52bdc2eb3a 100644 --- a/apps/desktop/src/preview/PickedElementPayload.test.ts +++ b/apps/desktop/src/preview/PickedElementPayload.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vite-plus/test"; -import { isPickedElementPayload, isPreviewAnnotationPayload } from "./PickedElementPayload.ts"; +import { + isPickedElementPayload, + isPreviewAnnotationDraft, + isPreviewAnnotationPayload, +} from "./PickedElementPayload.ts"; function validPayload(overrides?: Record): Record { return { @@ -199,3 +203,37 @@ describe("isPreviewAnnotationPayload", () => { ).toBe(false); }); }); + +describe("isPreviewAnnotationDraft", () => { + const draft = (overrides?: Record): Record => { + const { elements: _elements, ...annotation } = validAnnotation(); + return { + regions: annotation["regions"], + strokes: annotation["strokes"], + styleChanges: annotation["styleChanges"], + comment: "Make this clearer", + tool: "select", + elements: [ + { + id: "element_1", + selector: ":root > body:nth-of-type(1) > button:nth-of-type(2)", + rect: { x: 10, y: 20, width: 100, height: 40 }, + }, + ], + ...overrides, + }; + }; + + it("accepts the markup a reload carries over", () => { + expect(isPreviewAnnotationDraft(draft())).toBe(true); + }); + + it("rejects drafts the preload could not restore", () => { + expect(isPreviewAnnotationDraft(draft({ tool: "lasso" }))).toBe(false); + expect(isPreviewAnnotationDraft(draft({ comment: null }))).toBe(false); + expect( + isPreviewAnnotationDraft(draft({ elements: [{ id: "element_1", rect: { x: 0 } }] })), + ).toBe(false); + expect(isPreviewAnnotationDraft(draft({ strokes: [{ id: "stroke_1" }] }))).toBe(false); + }); +}); diff --git a/apps/desktop/src/preview/PickedElementPayload.ts b/apps/desktop/src/preview/PickedElementPayload.ts index e2d596120dba..738084ba0913 100644 --- a/apps/desktop/src/preview/PickedElementPayload.ts +++ b/apps/desktop/src/preview/PickedElementPayload.ts @@ -10,7 +10,14 @@ * channel via prototype pollution) would otherwise throw deep in the * renderer and the chip silently never appears. */ -import type { PickedElementPayload, PreviewAnnotationPayload } from "@t3tools/contracts"; +import type { + PickedElementPayload, + PreviewAnnotationPayload, + PreviewAnnotationRect, + PreviewAnnotationRegionTarget, + PreviewAnnotationStrokeTarget, + PreviewAnnotationStyleChange, +} from "@t3tools/contracts"; function isStringOrNull(value: unknown): value is string | null { return value === null || typeof value === "string"; @@ -67,6 +74,41 @@ function isPoint(value: unknown): boolean { ); } +function isArrayOf(value: unknown, isEntry: (entry: unknown) => boolean): boolean { + return Array.isArray(value) && value.every(isEntry); +} + +function isRegionTarget(value: unknown): boolean { + if (typeof value !== "object" || value === null) return false; + const target = value as Record; + return typeof target["id"] === "string" && isRect(target["rect"]); +} + +function isStrokeTarget(value: unknown): boolean { + if (typeof value !== "object" || value === null) return false; + const target = value as Record; + return ( + typeof target["id"] === "string" && + typeof target["color"] === "string" && + typeof target["width"] === "number" && + Number.isFinite(target["width"]) && + isArrayOf(target["points"], isPoint) && + isRect(target["bounds"]) + ); +} + +function isStyleChange(value: unknown): boolean { + if (typeof value !== "object" || value === null) return false; + const change = value as Record; + return ( + typeof change["targetId"] === "string" && + isStringOrNull(change["selector"]) && + typeof change["property"] === "string" && + typeof change["previousValue"] === "string" && + typeof change["value"] === "string" + ); +} + export function isPreviewAnnotationPayload(value: unknown): value is PreviewAnnotationPayload { if (typeof value !== "object" || value === null) return false; const annotation = value as Record; @@ -77,10 +119,8 @@ export function isPreviewAnnotationPayload(value: unknown): value is PreviewAnno if (typeof annotation["createdAt"] !== "string") return false; if (annotation["screenshot"] !== null) return false; - const elements = annotation["elements"]; - if (!Array.isArray(elements)) return false; - if ( - !elements.every((entry) => { + return ( + isArrayOf(annotation["elements"], (entry) => { if (typeof entry !== "object" || entry === null) return false; const target = entry as Record; return ( @@ -88,59 +128,54 @@ export function isPreviewAnnotationPayload(value: unknown): value is PreviewAnno isPickedElementPayload(target["element"]) && isRect(target["rect"]) ); - }) - ) { - return false; - } - - const regions = annotation["regions"]; - if (!Array.isArray(regions)) return false; - if ( - !regions.every((entry) => { - if (typeof entry !== "object" || entry === null) return false; - const target = entry as Record; - return typeof target["id"] === "string" && isRect(target["rect"]); - }) - ) { - return false; - } - - const strokes = annotation["strokes"]; - if (!Array.isArray(strokes)) return false; - if ( - !strokes.every((entry) => { + }) && + isArrayOf(annotation["regions"], isRegionTarget) && + isArrayOf(annotation["strokes"], isStrokeTarget) && + isArrayOf(annotation["styleChanges"], isStyleChange) + ); +} + +export type PreviewAnnotationTool = "select" | "marquee" | "draw" | "erase"; + +/** + * An in-progress markup that outlives a reload of the inspected page. The + * preload keeps main's copy current, and main hands it back to the fresh + * document so a dev-server refresh does not throw the user's comment away. + * Selected elements travel as a CSS path plus their last rect, because the + * DOM nodes themselves do not survive the reload. + */ +export interface PreviewAnnotationDraft { + readonly comment: string; + readonly tool: PreviewAnnotationTool; + readonly elements: ReadonlyArray<{ + readonly id: string; + readonly selector: string; + readonly rect: PreviewAnnotationRect; + }>; + readonly regions: ReadonlyArray; + readonly strokes: ReadonlyArray; + readonly styleChanges: ReadonlyArray; +} + +const ANNOTATION_TOOLS: ReadonlySet = new Set(["select", "marquee", "draw", "erase"]); + +export function isPreviewAnnotationDraft(value: unknown): value is PreviewAnnotationDraft { + if (typeof value !== "object" || value === null) return false; + const draft = value as Record; + return ( + typeof draft["comment"] === "string" && + ANNOTATION_TOOLS.has(draft["tool"]) && + isArrayOf(draft["elements"], (entry) => { if (typeof entry !== "object" || entry === null) return false; const target = entry as Record; return ( typeof target["id"] === "string" && - typeof target["color"] === "string" && - typeof target["width"] === "number" && - Number.isFinite(target["width"]) && - Array.isArray(target["points"]) && - target["points"].every(isPoint) && - isRect(target["bounds"]) - ); - }) - ) { - return false; - } - - const styleChanges = annotation["styleChanges"]; - if (!Array.isArray(styleChanges)) return false; - if ( - !styleChanges.every((entry) => { - if (typeof entry !== "object" || entry === null) return false; - const change = entry as Record; - return ( - typeof change["targetId"] === "string" && - isStringOrNull(change["selector"]) && - typeof change["property"] === "string" && - typeof change["previousValue"] === "string" && - typeof change["value"] === "string" + typeof target["selector"] === "string" && + isRect(target["rect"]) ); - }) - ) { - return false; - } - return true; + }) && + isArrayOf(draft["regions"], isRegionTarget) && + isArrayOf(draft["strokes"], isStrokeTarget) && + isArrayOf(draft["styleChanges"], isStyleChange) + ); } diff --git a/apps/web/src/components/preview/PreviewView.tsx b/apps/web/src/components/preview/PreviewView.tsx index 810e6d502804..bb2ce3508676 100644 --- a/apps/web/src/components/preview/PreviewView.tsx +++ b/apps/web/src/components/preview/PreviewView.tsx @@ -733,7 +733,9 @@ export function PreviewView({ // Disable when there's no tab (nothing to pick on) OR the page // failed to load (a React overlay covers the webview, so the // user wouldn't be able to actually click anything underneath). - pickDisabled={!tabId || isUnreachable} + // An active pick stays cancellable: a failed reload keeps its markup + // waiting for the next successful refresh. + pickDisabled={!tabId || (isUnreachable && !pickActive)} pickDisabledReason={ isUnreachable ? "Page didn't load — pick unavailable until the page renders" : undefined }