From 160f5fc53e0154865846bc578158e5c1e4503628 Mon Sep 17 00:00:00 2001 From: callumalpass Date: Fri, 2 Oct 2026 22:26:53 +1000 Subject: [PATCH] Prepare shared private feedback integration for Reader --- apps/reader/package.json | 1 + apps/reader/scripts/feedback-browser-test.mjs | 69 +++++++++++++++++++ apps/reader/src/ConnectReader.tsx | 50 +++++++++++--- apps/reader/src/ConnectedDocument.tsx | 7 +- apps/reader/src/DocumentRendererStage.tsx | 10 ++- apps/reader/src/FeedbackRoot.test.tsx | 32 +++++++++ apps/reader/src/FeedbackRoot.tsx | 43 ++++++++++++ apps/reader/src/LibraryRowMenu.tsx | 3 + apps/reader/src/ReaderHeader.tsx | 3 + apps/reader/src/ReaderWorkspaceView.tsx | 2 + apps/reader/src/env.d.ts | 2 + apps/reader/src/feedback-shell.css | 15 ++++ apps/reader/src/main.tsx | 29 ++++---- apps/reader/vite.config.ts | 2 + docs/shared-feedback.md | 43 ++++++++++++ 15 files changed, 287 insertions(+), 24 deletions(-) create mode 100644 apps/reader/scripts/feedback-browser-test.mjs create mode 100644 apps/reader/src/FeedbackRoot.test.tsx create mode 100644 apps/reader/src/FeedbackRoot.tsx create mode 100644 apps/reader/src/feedback-shell.css create mode 100644 docs/shared-feedback.md diff --git a/apps/reader/package.json b/apps/reader/package.json index 2e257b8..81730d9 100644 --- a/apps/reader/package.json +++ b/apps/reader/package.json @@ -15,6 +15,7 @@ "preview": "vite preview --host 127.0.0.1", "test": "pnpm manifest && pnpm manifest:verify && node --test scripts/*.test.mjs && vitest run src capture --maxWorkers=2 --testTimeout=15000", "test:browser": "node scripts/audit-reader.mjs", + "test:feedback": "node scripts/feedback-browser-test.mjs", "test:pdf-touch": "node scripts/run-pdf-touch-audits.mjs", "typecheck": "tsc -p tsconfig.json", "test:a11y": "node scripts/audit-accessibility.mjs" diff --git a/apps/reader/scripts/feedback-browser-test.mjs b/apps/reader/scripts/feedback-browser-test.mjs new file mode 100644 index 0000000..53b272e --- /dev/null +++ b/apps/reader/scripts/feedback-browser-test.mjs @@ -0,0 +1,69 @@ +// Sample-data acceptance only. Intercepts every feedback POST; never sends mail. +/* global window */ +import assert from "node:assert/strict"; +import { chromium, expect } from "@playwright/test"; + +const origin = process.env.READER_FEEDBACK_TEST_ORIGIN ?? "http://127.0.0.1:8891"; +if (!/^http:\/\/(127\.0\.0\.1|localhost):\d+$/u.test(origin)) { + throw new Error("Feedback acceptance requires a loopback preview."); +} +const browser = await chromium.launch(); +try { + for (const width of [1440, 390]) { + const context = await browser.newContext({ viewport: { width, height: 900 } }); + const page = await context.newPage(); + const posts = []; + await context.route("**/v1/feedback", async (route) => { + posts.push(route.request().postDataJSON()); + await route.fulfill({ status: 200, contentType: "application/json", body: "{}" }); + }); + await page.addInitScript(() => { + window.feedbackCaptures = 0; + Object.defineProperty(navigator, "mediaDevices", { + configurable: true, + value: { + getDisplayMedia: () => { + window.feedbackCaptures++; + return Promise.reject(new DOMException("Cancelled", "NotAllowedError")); + }, + }, + }); + }); + await page.goto(`${origin}/?preview`); + const trigger = page + .locator(".reader-header") + .getByRole("button", { name: "Send feedback", exact: true }); + await trigger.click(); + const dialog = page.getByRole("dialog", { name: "Send feedback", exact: true }); + const message = dialog.getByLabel("What happened?"); + await expect(message).toBeFocused(); + await expect(dialog.locator("details")).not.toHaveAttribute("open"); + assert.equal(await page.evaluate(() => window.feedbackCaptures), 0); + await message.fill("A private sample draft"); + await page.screenshot({ path: `/tmp/shared-feedback-reader-form-${width}.png` }); + await message.press("Control+k"); + assert.equal(await page.locator("dialog[open]").count(), 1); + await message.press("Escape"); + await expect(dialog).not.toBeVisible(); + await expect(trigger).toBeFocused(); + await trigger.click(); + await expect(message).toHaveValue("A private sample draft"); + await dialog.getByRole("button", { name: "Attach screenshot", exact: true }).click(); + await expect(dialog.getByRole("status")).toContainText("No screenshot taken"); + assert.equal(await page.evaluate(() => window.feedbackCaptures), 1); + await dialog.getByRole("button", { name: "Send feedback", exact: true }).click(); + await expect(dialog.getByRole("heading", { name: "Thanks for the report." })).toBeFocused(); + assert.equal(posts.length, 1); + const payload = posts[0]; + assert.equal(payload.schema_version, 2); + assert.equal(payload.application.product, "mdbase reader"); + assert.ok(["library", "document"].includes(payload.application.source_view)); + for (const key of ["context", "diagnostics", "screenshot", "reply_email"]) + assert.equal(payload[key], undefined); + await page.screenshot({ path: `/tmp/shared-feedback-reader-${width}.png` }); + await context.close(); + } + console.log("Reader feedback desktop/mobile acceptance passed; all delivery intercepted."); +} finally { + await browser.close(); +} diff --git a/apps/reader/src/ConnectReader.tsx b/apps/reader/src/ConnectReader.tsx index 593da12..da85c5a 100644 --- a/apps/reader/src/ConnectReader.tsx +++ b/apps/reader/src/ConnectReader.tsx @@ -1,3 +1,4 @@ +import { FeedbackButton, useFeedback } from "@mdbase-dev/ui/feedback"; import { ConnectLayout, OpeningScreen } from "@mdbase-dev/ui/screens"; import { connectProblemMessage, type ReaderConnectSnapshot } from "@mdbase-reader/connect"; import { createReaderRuntimeServices, createWebPlatform } from "@mdbase-reader/platform"; @@ -54,14 +55,23 @@ async function startSession(): Promise(null); const start = async (): Promise => { setError(null); try { - setError(connectProblemMessage(await startSession())); + const outcome = await startSession(); + setError(connectProblemMessage(outcome)); + if (!outcome.ok && !/cancel|abort|supersed/u.test(outcome.problem.code)) { + reportError({ code: "unknown_error" }); + } } catch (reason) { + if (reason instanceof DOMException && reason.name === "AbortError") { + return; + } + reportError({ code: "source_open_failed" }); setError(readerErrorMessage(reason, "Reader could not open this collection.")); } }; @@ -72,17 +82,21 @@ export function ConnectReader(): JSX.Element { .then((outcome) => { if (active) { setError(connectProblemMessage(outcome)); + if (!outcome.ok && !/cancel|abort|supersed/u.test(outcome.problem.code)) { + reportError({ code: "unknown_error" }); + } } }) .catch((reason: unknown) => { - if (active) { + if (active && !(reason instanceof DOMException && reason.name === "AbortError")) { + reportError({ code: "source_open_failed" }); setError(readerErrorMessage(reason, "Reader could not open this collection.")); } }); return () => { active = false; }; - }, []); + }, [reportError]); useEffect(() => { // Overlap workspace download with live collection checks, not the first query. @@ -197,17 +211,20 @@ function OpenedReader({ collectionId }: { readonly collectionId: string }): JSX. ); } +interface ConnectionScreenProps { + readonly session: Exclude; + readonly error: string | null; + readonly onError: (message: string | null) => void; + readonly onRetry: () => void; +} + function ConnectionScreen({ session, error, onError, onRetry, -}: { - readonly session: Exclude; - readonly error: string | null; - readonly onError: (message: string | null) => void; - readonly onRetry: () => void; -}): JSX.Element { +}: ConnectionScreenProps): JSX.Element { + const { reportError } = useFeedback(); const [working, setWorking] = useState(false); const selectedCollectionId = "collectionId" in session ? session.collectionId : null; // Some failures arrive only as the session's status, not as a step's error. @@ -219,7 +236,14 @@ function ConnectionScreen({ try { const outcome = await readerSession.authorize(target); onError(connectProblemMessage(outcome)); + if (!outcome.ok && !/cancel|abort|supersed/u.test(outcome.problem.code)) { + reportError({ code: "unknown_error" }); + } } catch (reason) { + if (reason instanceof DOMException && reason.name === "AbortError") { + return; + } + reportError({ code: "unknown_error" }); onError(readerErrorMessage(reason, "Reader could not review application access.")); } finally { setWorking(false); @@ -231,7 +255,14 @@ function ConnectionScreen({ try { const outcome = await readerSession.applyCollectionSetup(); onError(connectProblemMessage(outcome)); + if (!outcome.ok && !/cancel|abort|supersed/u.test(outcome.problem.code)) { + reportError({ code: "unknown_error" }); + } } catch (reason) { + if (reason instanceof DOMException && reason.name === "AbortError") { + return; + } + reportError({ code: "unknown_error" }); onError(readerErrorMessage(reason, "Reader could not apply the reviewed setup.")); } finally { setWorking(false); @@ -304,6 +335,7 @@ function ConnectionScreen({ The managed service requires an HTTPS Reader origin.

) : null} + ); } diff --git a/apps/reader/src/ConnectedDocument.tsx b/apps/reader/src/ConnectedDocument.tsx index bf84d66..27e551c 100644 --- a/apps/reader/src/ConnectedDocument.tsx +++ b/apps/reader/src/ConnectedDocument.tsx @@ -1,3 +1,4 @@ +import { useFeedback } from "@mdbase-dev/ui/feedback"; import { lazy, Suspense, useCallback, useEffect, useMemo, useState, type JSX } from "react"; import { isEpub, isHtml, isPdf } from "./document-media.js"; @@ -63,6 +64,7 @@ function OpenConnectedDocument({ source, onSurfaceChange, }: ConnectedDocumentProps & { readonly descriptor: DocumentDescriptor }): JSX.Element { + const { reportError } = useFeedback(); const [state, setState] = useState({ status: "opening" }); const [attempt, setAttempt] = useState(0); const stableDescriptor = useMemo( @@ -99,7 +101,8 @@ function OpenConnectedDocument({ } }) .catch((reason: unknown) => { - if (active) { + if (active && !controller.signal.aborted) { + reportError({ code: "source_open_failed" }); setState({ status: "error", message: message(reason) }); } }); @@ -110,7 +113,7 @@ function OpenConnectedDocument({ void opened.close(); } }; - }, [attempt, repository, source.collectionId, stableDescriptor]); + }, [attempt, repository, source.collectionId, stableDescriptor, reportError]); useEffect(() => () => onSurfaceChange(null), [onSurfaceChange]); const handle = state.status === "open" ? state.handle : null; diff --git a/apps/reader/src/DocumentRendererStage.tsx b/apps/reader/src/DocumentRendererStage.tsx index 09cb4ea..8c5a670 100644 --- a/apps/reader/src/DocumentRendererStage.tsx +++ b/apps/reader/src/DocumentRendererStage.tsx @@ -1,4 +1,5 @@ -import type { JSX, ReactNode } from "react"; +import { FeedbackButton, useFeedback } from "@mdbase-dev/ui/feedback"; +import { useEffect, type JSX, type ReactNode } from "react"; export type RendererState = { readonly status: "opening" | "ready" } | { readonly status: "error"; readonly message: string }; @@ -14,6 +15,12 @@ export function RendererStage({ readonly errorName: string; readonly children: ReactNode; }): JSX.Element { + const { reportError } = useFeedback(); + useEffect(() => { + if (state.status === "error") { + reportError({ code: "preview_failed" }); + } + }, [state.status, reportError]); return (
{children} @@ -49,6 +56,7 @@ export function DocumentMessage({ role={tone === "error" ? "alert" : "status"} > {label} + {tone === "error" ? : null} {action ? ( ) : null} {display} +
{display} + {bar.actions}
diff --git a/apps/reader/src/ReaderWorkspaceView.tsx b/apps/reader/src/ReaderWorkspaceView.tsx index e5e4a77..432a378 100644 --- a/apps/reader/src/ReaderWorkspaceView.tsx +++ b/apps/reader/src/ReaderWorkspaceView.tsx @@ -5,6 +5,7 @@ import { confirmCollectionSwitch } from "./collection-switching.js"; import { DeploymentUpdateNotice } from "./DeploymentUpdateNotice.js"; import { inspectorPanelId, navigatorPanelId } from "./dockview-workspace-state.js"; import { DockviewWorkspace } from "./DockviewWorkspace.js"; +import { useReaderFeedbackContext } from "./FeedbackRoot.js"; import { importHref } from "./import-navigation.js"; import { inspectorSourceForTab } from "./inspector-source.js"; import { InspectorPane, type InspectorTab } from "./InspectorPane.js"; @@ -95,6 +96,7 @@ export function ReaderWorkspaceView({ readonly model: ReaderWorkspaceViewModel; }): JSX.Element { const { library, source, workspace, sourceWorkspace, composer, sourceAddition } = model; + useReaderFeedbackContext(Boolean(sourceWorkspace.activeSourceId), library.collectionName); const shell = useWorkspaceShellPreferences( library.sources[0]?.collectionId ?? library.collectionName, ); diff --git a/apps/reader/src/env.d.ts b/apps/reader/src/env.d.ts index 0ee4d61..a8633e1 100644 --- a/apps/reader/src/env.d.ts +++ b/apps/reader/src/env.d.ts @@ -2,6 +2,8 @@ interface ImportMetaEnv { readonly VITE_MDBASE_ENV?: string; + readonly VITE_MDBASE_FEEDBACK_URL?: string; + readonly VITE_MDBASE_FEEDBACK_TURNSTILE_SITE_KEY?: string; readonly VITE_MDBASE_CONNECT_URL?: string; readonly VITE_MDBASE_CONNECT_LOOPBACK_URL?: string; readonly VITE_MDBASE_READER_BUILD_ID?: string; diff --git a/apps/reader/src/feedback-shell.css b/apps/reader/src/feedback-shell.css new file mode 100644 index 0000000..d847de8 --- /dev/null +++ b/apps/reader/src/feedback-shell.css @@ -0,0 +1,15 @@ +@media (max-width: 900px) { + .reader-header .mdbase-feedback-trigger { + min-width: 44px; + min-height: 44px; + padding: 8px; + } + + .reader-header .mdbase-feedback-trigger > span { + position: absolute; + width: 1px; + height: 1px; + overflow: hidden; + clip: rect(0, 0, 0, 0); + } +} diff --git a/apps/reader/src/main.tsx b/apps/reader/src/main.tsx index 7529472..109c8ad 100644 --- a/apps/reader/src/main.tsx +++ b/apps/reader/src/main.tsx @@ -6,9 +6,12 @@ import "./reader.css"; import "./reader-improvements.css"; import "./annotation-polish.css"; import "./reader-shell.css"; +import "@mdbase-dev/ui/feedback.css"; +import "./feedback-shell.css"; import { ConnectReader } from "./ConnectReader.js"; import { EnvironmentBadge } from "./EnvironmentBadge.js"; +import { FeedbackRoot } from "./FeedbackRoot.js"; import { forgetOfflineCopies } from "./forget-offline-copies.js"; import { importService } from "./import-navigation.js"; import { keepFocusedFieldInView } from "./keep-focused-field-in-view.js"; @@ -37,17 +40,19 @@ keepFocusedFieldInView(window); const migrationService = importService(location.pathname); createRoot(root).render( - - {migrationService ? ( - - - - ) : new URL(location.href).searchParams.has("preview") ? ( - - - - ) : ( - - )} + + + {migrationService ? ( + + + + ) : new URL(location.href).searchParams.has("preview") ? ( + + + + ) : ( + + )} + , ); diff --git a/apps/reader/vite.config.ts b/apps/reader/vite.config.ts index 7cbe278..9b3cd79 100644 --- a/apps/reader/vite.config.ts +++ b/apps/reader/vite.config.ts @@ -12,6 +12,8 @@ export default defineConfig({ plugins: [react(), deploymentRevision(buildId)], resolve: { dedupe: [ + "react", + "react-dom", "@codemirror/autocomplete", "@codemirror/commands", "@codemirror/language", diff --git a/docs/shared-feedback.md b/docs/shared-feedback.md new file mode 100644 index 0000000..b4b13ec --- /dev/null +++ b/docs/shared-feedback.md @@ -0,0 +1,43 @@ +# Private feedback integration (release-blocked) + +This branch uses the shared `@mdbase-dev/ui/feedback` provider, form, screenshot +capture/markup and verification. It adds no email adapter, telemetry or storage. +The provider stays above connection/workspace lifetimes so cancelling or changing +views preserves the draft. Header entries remain available on desktop and mobile; +connection and document failures have a report entry. + +Application metadata uses fixed connection/library/document identifiers, a bounded +build ID and deployment environment. Collection names and diagnostics require +explicit consent; source titles, IDs, paths, URLs, credentials and raw exceptions +are never passed to the feedback API. Only active source-open, renderer and +explicit connection/setup failures nudge the bug. Aborted/superseded operations +and background refreshes do not. Global capture-phase shortcuts ignore dialogs. + +## Release dependency + +Do not merge or deploy this branch until the UI package containing these exports +has been published through mdbase-connect's coordinated release process. The +currently pinned beta.123 does **not** contain them. Update all existing UI pins +and regenerate the lockfile from that actual published version, then run clean +install, full CI and the browser acceptance again. Local worktree links are used +only for isolated verification and are not committed as dependencies. + +Deploy the v1/v2-compatible feedback Worker first via guarded cloud-ops. Approve +exact Reader origins in CORS and the separate Turnstile widgets before enabling +these build variables: + +- `VITE_MDBASE_FEEDBACK_URL`: approved environment's `/v1/feedback` endpoint. +- `VITE_MDBASE_FEEDBACK_TURNSTILE_SITE_KEY`: that environment's public widget key. + +Unset/invalid endpoints hide feedback. Existing deployment tooling supplies +`VITE_MDBASE_ENV` and `VITE_MDBASE_READER_BUILD_ID`; no implicit production endpoint +is selected. No production widget, secrets or deployment is changed here. + +## Sample-data acceptance + +Build with a loopback feedback URL and serve the output on loopback port 8891. +Run `node apps/reader/scripts/feedback-browser-test.mjs`; override the loopback +origin with `READER_FEEDBACK_TEST_ORIGIN`. It uses `?preview`, intercepts every +feedback POST, and checks desktop/mobile focus, draft restoration, explicit +capture cancellation, shortcut isolation, metadata and default consent. No +native chooser or real delivery is exercised; those remain separate acceptance.