From 61314d10516b1059feab221f4dd7001420ddcd09 Mon Sep 17 00:00:00 2001 From: Srinivas Annam Date: Thu, 27 Aug 2026 12:31:18 -0700 Subject: [PATCH] feat(feedback): replace inline panel with milestone-triggered FeedbackDialog (#900) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the inline `FeedbackPanel` (a quiet star strip below the score card that got even quieter after 2 views) with a `FeedbackDialog` multi-step interstitial on `@design-system`'s `Dialog` primitive. - Automatic trigger: opens once after the user's 1st completed PDF or Markdown export (not the audit report), deferred until the `ExportDialog` itself closes so it never stacks on top of the export-findings advisory. - Ambient triggers: a `[★ Feedback]` button in `ParsedHeader` (visible in the score card header row when parsed) and a `Feedback` link in the `PageShell` footer (always accessible, including from the home page). - Flow: Step 1 is a 1-5 star rating with an explicit Close (the dialog opens itself, so it needs its own dismissal path) and a Back control on both Step 2 bodies, since the star group is a native radio and selects on the same keystroke that moves focus. 4-5★ routes to a GitHub-star CTA + optional praise; 1-3★ routes to category pills + an issue textarea. Both paths end in an in-place, `aria-live` confirmation step rather than the dialog just vanishing. - Capped via a new `ocv_feedback_dialog_seen` / `ocv_feedback_submitted` localStorage pair; feedback collection remains accessible at all times, with events dispatched via `trackFeedback`. - `useGitHubStars` is fetched from the positive step only, so a page load that never reaches the 4-5★ branch spends no GitHub API request. - The email opt-in (checkbox + revealed field) is a shared `EmailOptIn` component used by both step-2 bodies, with a visible label rather than a placeholder-only field. Closes #900 --- README.md | 9 +- src/App.tsx | 32 +- src/components/Result.tsx | 15 +- src/components/features/ExportDialog.test.tsx | 13 + src/components/features/ExportDialog.tsx | 21 +- .../features/FeedbackConstructiveStep.tsx | 105 +++++ .../features/FeedbackDialog.test.tsx | 256 ++++++++++++ src/components/features/FeedbackDialog.tsx | 153 +++++++ .../features/FeedbackPanel.test.tsx | 178 -------- src/components/features/FeedbackPanel.tsx | 379 ------------------ .../features/FeedbackPositiveStep.tsx | 97 +++++ src/components/features/PageShell.test.tsx | 23 ++ src/components/features/PageShell.tsx | 12 +- src/components/features/ParsedHeader.test.tsx | 23 +- src/components/features/ParsedHeader.tsx | 11 + .../features/ReportGapSection.test.tsx | 1 - src/design-system/index.ts | 1 + src/design-system/shared/EmailOptIn.tsx | 62 +++ src/hooks/useFeedbackDialog.test.ts | 138 +++++++ src/hooks/useFeedbackDialog.ts | 112 ++++++ src/lib/analytics.ts | 5 +- 21 files changed, 1068 insertions(+), 578 deletions(-) create mode 100644 src/components/features/FeedbackConstructiveStep.tsx create mode 100644 src/components/features/FeedbackDialog.test.tsx create mode 100644 src/components/features/FeedbackDialog.tsx delete mode 100644 src/components/features/FeedbackPanel.test.tsx delete mode 100644 src/components/features/FeedbackPanel.tsx create mode 100644 src/components/features/FeedbackPositiveStep.tsx create mode 100644 src/design-system/shared/EmailOptIn.tsx create mode 100644 src/hooks/useFeedbackDialog.test.ts create mode 100644 src/hooks/useFeedbackDialog.ts diff --git a/README.md b/README.md index 45868533..8abcea57 100644 --- a/README.md +++ b/README.md @@ -139,12 +139,12 @@ production`) so maintainer/staging/teammate traffic can be excluded from user-facing metrics; this app supplies the properties, the filter itself is configured in the PostHog project rather than here. -The one exception is the optional feedback panel (`feedback_submitted`): it +The one exception is the optional feedback dialog (`feedback_submitted`): it carries a 1–5 `rating` plus, **only when the user chooses to fill them**, a `category`, free-text `feedback_text`, and an `email`. That email is the single piece of user-supplied PII any event can carry — it is opt-in (the field is blank by default and is never sent as an empty string; see `buildFeedbackProps`), -attached only as a property on that single event, and the whole panel is +attached only as a property on that single event, and the whole dialog is hidden in builds where `VITE_POSTHOG_KEY` is unset. It is never promoted to a PostHog person profile: this app never calls `identify()` or `setPersonProperties` anywhere. (`register()` is used, but only for the @@ -167,9 +167,8 @@ scoped to your browser: | Key | Purpose | |---|---| -| `ocv_feedback_seen` | counts how many times the feedback ask has rendered; after 2 the panel switches from the full card to a quiet compact star strip | -| `ocv_feedback_submitted` | set after a successful feedback submit so the panel never re-asks in that browser | -| `ocv_star_cta_seen` | one-time flag so the post-feedback GitHub-star prompt shows only once per browser | +| `ocv_feedback_dialog_seen` | counts how many times the feedback dialog has been opened; once it is non-zero the dialog no longer opens itself after an export — you can still open it from the `★ Feedback` button | +| `ocv_feedback_submitted` | set after a successful feedback submit so the dialog never opens itself again in that browser | | `ocv_gh_stars_cache` | caches the fetched star count (~1h TTL) to avoid re-hitting the GitHub API on every parse | | `ocv_internal` | marks this browser as internal so team traffic can be filtered out of analytics; set via `?ocv_internal=1`, cleared via `?ocv_internal=0` — only written in builds where `VITE_POSTHOG_KEY` is set | diff --git a/src/App.tsx b/src/App.tsx index 401f2a81..df83ece6 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -20,12 +20,14 @@ import { ResumeLibrary } from "./components/features/ResumeLibrary.tsx"; import { ShareWithExtensionBar } from "./components/features/ShareWithExtensionBar.tsx"; import { ExportDialog } from "./components/features/ExportDialog.tsx"; import { ResumeChooserDialog } from "./components/features/ResumeChooserDialog.tsx"; +import { FeedbackDialog } from "./components/features/FeedbackDialog.tsx"; import { useAnalyzedResume } from "./hooks/useAnalyzedResume.ts"; import { useResumeLibrary } from "./hooks/useResumeLibrary.ts"; import { useReplaceResumeOnDrop } from "./hooks/useReplaceResumeOnDrop.ts"; import { useAutoRestoreResume } from "./hooks/useAutoRestoreResume.ts"; import { useAutosaveResume } from "./hooks/useAutosaveResume.ts"; import { useLlmRecovery } from "./hooks/useLlmRecovery.ts"; +import { useFeedbackDialog } from "./hooks/useFeedbackDialog.ts"; import { departToJobs, departToJobsAndNavigate, @@ -122,6 +124,13 @@ export default function App() { // committed frame in between for that to be visible in. if (recovery === null && exportOpen) setExportOpen(false); + // The multi-step feedback interstitial (#900) — one controller for both + // ways in: the ambient `[★ Feedback]` button (threaded through `Result` to + // `ParsedHeader`) and the automatic trigger, earned below from the export + // dialog's résumé-only callback and flushed when that dialog closes. Owned + // here, next to `exportOpen`, since both dialogs are page-level siblings. + const feedback = useFeedbackDialog(); + // Local-first resume library (#322) — save/reload parsed resumes without // re-uploading. Loading hydrates the "done" state from the cached parse. const library = useResumeLibrary(); @@ -455,6 +464,7 @@ export default function App() { badge="alpha" onSavedJobsNavigate={goToSavedJobs} journey={{ state: journeyState, onSelect: onJourneySelect }} + onOpenFeedback={feedback.openDialog} > {(state.phase === "idle" || state.phase === "parsing" || @@ -650,6 +660,8 @@ export default function App() { // it IS the Tailor stage, done. `ResultDetail` owns the pairing; // the key it is recorded under is only knowable here. onTailorApplied={() => progress.mark("tailor")} + // #900 — ambient feedback button trigger. + onOpenFeedback={feedback.openDialog} /> {/* Hand the parse to the capture extension (#620) — self-hides when no extension answers a probe, so it costs nothing on the visit @@ -721,7 +733,14 @@ export default function App() { // artifact matches what the page shows. setExportOpen(false)} + // #900 — closing is also when a pending feedback milestone flushes. + // The export dialog stays open after a download on purpose (#421), + // so opening the interstitial any earlier would stack a second + // native modal over the findings the download just produced. + onClose={() => { + setExportOpen(false); + feedback.notifyExportClosed(); + }} result={recovery.activeResult} score={recovery.activeScore} contactOverrides={edit.contactOverrides} @@ -729,9 +748,20 @@ export default function App() { // Download stage, the audit report included: the ledger records that // the user went through here, and the report is downloaded from it. onExported={() => progress.mark("download")} + // #900 — the résumé-only subset of the same success point feeds the + // feedback dialog's automatic first-export trigger. + onResumeExported={feedback.notifyResumeExported} /> )} + {/* #900 — the multi-step feedback interstitial. Page level, beside the + export dialog whose résumé-only callback can open it automatically. */} + + {/* #826 — which saved résumé did you mean? Opened only by a rail click that arrived with nothing on the page and two or more saved, and it FINISHES that click rather than merely loading (see the file). Page diff --git a/src/components/Result.tsx b/src/components/Result.tsx index 9569f202..9afbfcde 100644 --- a/src/components/Result.tsx +++ b/src/components/Result.tsx @@ -4,7 +4,6 @@ import type { CascadeResult } from "../lib/heuristics/types.ts"; import type { EditableParse } from "../hooks/useEditableParse.ts"; import { Card, StatusBadge, Button, ErrorState } from "@design-system"; -import { FeedbackPanel } from "./features/FeedbackPanel.tsx"; import { AtsScoreReadout } from "./features/AtsScoreReadout.tsx"; import { isScoreRevealed } from "../lib/contact.ts"; import { useResumeAnalysisLlm } from "../hooks/useResumeAnalysisLlm.ts"; @@ -57,6 +56,10 @@ interface ResultProps { /** A JD-steered whole-résumé rewrite was applied (#826) — see `ResultDetail`, * which pairs the rewrite event with the steering it owns. */ onTailorApplied?: () => void; + /** Opens `FeedbackDialog` (#900) — threaded down to `ParsedHeader`'s + * ambient `[★ Feedback]` trigger. `App` owns the dialog itself, since the + * automatic export-milestone trigger fires from page level too. */ + onOpenFeedback?: () => void; } export function Result({ @@ -70,6 +73,7 @@ export function Result({ autosave, onJdContextChange, onTailorApplied, + onOpenFeedback, }: ResultProps) { const isFontsUnmappable = result.triggers.includes("fonts_unmappable"); if (isFontsUnmappable) { @@ -90,6 +94,7 @@ export function Result({ autosave={autosave} onJdContextChange={onJdContextChange} onTailorApplied={onTailorApplied} + onOpenFeedback={onOpenFeedback} /> ); } @@ -107,6 +112,7 @@ function ParsedCard({ autosave, onJdContextChange, onTailorApplied, + onOpenFeedback, }: { result: CascadeResult; bytes?: ArrayBuffer; @@ -118,6 +124,7 @@ function ParsedCard({ autosave: AutosaveResume; onJdContextChange?: (jdContext: string | null) => void; onTailorApplied?: () => void; + onOpenFeedback?: () => void; }) { const triggerCount = result.triggers.length; const { activeResult, activeScore, parseIdentity, isLlmRecovered } = recovery; @@ -182,6 +189,7 @@ function ParsedCard({ onReset={onReset} saveState={autosave.state} onSave={autosave.save} + onOpenFeedback={onOpenFeedback} /> {isTwoColumn && ( @@ -199,11 +207,6 @@ function ParsedCard({ role are filled in below.

)} - {/* Star-rating feedback (#51). The "Report a parsing gap" affordance - lives in the "What an ATS misses" bottom section of the "Local AI - feedback" disclosure (#273), next to the disagreements it - characterizes. */} - {}, onExported?: () => void, + onResumeExported?: () => void, ): HTMLElement { container = document.createElement("div"); document.body.appendChild(container); @@ -139,6 +140,7 @@ function render( score: SCORE, contactOverrides: overrides, onExported, + onResumeExported, }), ); }); @@ -432,6 +434,17 @@ describe("ExportDialog", () => { expect(onExported).toHaveBeenCalledTimes(3); }); + it("fires the résumé-only feedback milestone (#900) for PDF and Markdown, never the report", () => { + const onResumeExported = vi.fn(); + render(exportable(), {}, () => {}, undefined, onResumeExported); + captured.report?.(); + expect(onResumeExported).not.toHaveBeenCalled(); + captured.pdf?.(); + expect(onResumeExported).toHaveBeenCalledTimes(1); + captured.markdown?.(); + expect(onResumeExported).toHaveBeenCalledTimes(2); + }); + // #621 — the export reports what it could not render cleanly, on the row that // produced the file. Advisory: the user already has the PDF. describe("export findings", () => { diff --git a/src/components/features/ExportDialog.tsx b/src/components/features/ExportDialog.tsx index 180d4df5..02b842c0 100644 --- a/src/components/features/ExportDialog.tsx +++ b/src/components/features/ExportDialog.tsx @@ -98,6 +98,13 @@ interface ExportDialogProps { * every row can fail into an inline error without the dialog closing. */ onExported?: () => void; + /** + * Fired only when the résumé itself — PDF or Markdown — downloads, never + * the audit report (#900). Feeds `FeedbackDialog`'s automatic milestone + * trigger, which cares about "you got your résumé" rather than every + * artifact this dialog can produce. + */ + onResumeExported?: () => void; } export function ExportDialog({ @@ -107,6 +114,7 @@ export function ExportDialog({ score, contactOverrides, onExported, + onResumeExported, }: ExportDialogProps) { const [body, setBody] = useState<"formats" | "gate">("formats"); // Focus for the half of the swap `ExportGateBody` cannot own. It focuses @@ -125,8 +133,17 @@ export function ExportDialog({ const [includeIdentity, setIncludeIdentity] = useState(false); const formatName = useId(); - const pdf = useDownloadPdf(result, score, onExported); - const markdown = useDownloadMarkdown(result, score, onExported); + // pdf/markdown fire BOTH callbacks — the shared Download-stage mark (#826) + // and the résumé-only feedback milestone (#900). `report` fires only the + // former: an audit report is not "you got your résumé". + const pdf = useDownloadPdf(result, score, () => { + onExported?.(); + onResumeExported?.(); + }); + const markdown = useDownloadMarkdown(result, score, () => { + onExported?.(); + onResumeExported?.(); + }); const report = useDownloadReport(result, score, onExported); // Re-derived every render, so the checklist reflects the edit the user just diff --git a/src/components/features/FeedbackConstructiveStep.tsx b/src/components/features/FeedbackConstructiveStep.tsx new file mode 100644 index 00000000..659cd2e0 --- /dev/null +++ b/src/components/features/FeedbackConstructiveStep.tsx @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 The offlinecv Authors + +/** + * FeedbackConstructiveStep — Step 2B of `FeedbackDialog` (#900), reached + * after a 1-3★ rating. Category pills route the report to a rough area + * without forcing a taxonomy pick; the rest — description, opt-in follow-up + * contact — mirrors the retired inline panel's own fields. + * + * Split out of `FeedbackDialog` so that file stays a thin step router — this + * one owns its own form state, and `FeedbackPositiveStep` (Step 2A) mirrors + * the same split for the opposite sentiment. + */ + +import { useState } from "react"; +import { Button, EmailOptIn, TextAreaField } from "@design-system"; +import type { FeedbackArgs } from "../../lib/analytics.ts"; + +const CATEGORIES = ["Parsing", "Scoring", "UI / Editor", "Export", "Other"] as const; +type Category = (typeof CATEGORIES)[number]; + +interface FeedbackConstructiveStepProps { + onSubmit: (fields: Omit) => void; + /** Returns to Step 1 without losing the star rating — the only way a + * keyboard user who overshot into 1-3★ (native radio: arrow keys select on + * the same keystroke that moves focus) can reach 4-5★ instead. */ + onBack: () => void; + onClose: () => void; +} + +export function FeedbackConstructiveStep({ + onSubmit, + onBack, + onClose, +}: FeedbackConstructiveStepProps) { + const [category, setCategory] = useState(""); + const [feedbackText, setFeedbackText] = useState(""); + const [wantsContact, setWantsContact] = useState(false); + const [email, setEmail] = useState(""); + + function handleSubmit() { + onSubmit({ + category: category || undefined, + feedbackText: feedbackText || undefined, + wantsContact, + // Email is PII — only forwarded when the user opted into follow-up. + email: wantsContact ? email : undefined, + }); + } + + return ( +
+
+ + What area needs improvement? (optional) + +
+ {CATEGORIES.map((c) => { + const selected = category === c; + return ( + + ); + })} +
+
+ + + + { + setWantsContact(next); + setEmail(nextEmail); + }} + /> + +
+ + + +
+
+ ); +} diff --git a/src/components/features/FeedbackDialog.test.tsx b/src/components/features/FeedbackDialog.test.tsx new file mode 100644 index 00000000..6cd38e44 --- /dev/null +++ b/src/components/features/FeedbackDialog.test.tsx @@ -0,0 +1,256 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 The offlinecv Authors + +// @vitest-environment jsdom + +/** + * FeedbackDialog (#900) — the multi-step feedback interstitial that replaced + * the inline `FeedbackPanel`. Covers: Step 1's sentiment routing (4-5★ → + * positive, 1-3★ → constructive), that submitting from either step-2 body ships + * sanitized props via `trackFeedback` and reports success via `onSubmitted`, + * and that it lands on the focused `aria-live` confirmation rather than closing + * — a dialog that vanishes confirms nothing to a screen reader. + * + * Uses raw `createRoot` (no RTL), matching `ExportDialog.test.tsx`. + */ + +import { afterEach, describe, expect, it, vi } from "vitest"; +import { act, createElement } from "react"; +import { createRoot, type Root } from "react-dom/client"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = + true; + +let container: HTMLDivElement | undefined; +let root: Root | undefined; + +async function mountDialog(opts: { + trackFeedback?: (...args: unknown[]) => void; + onClose?: () => void; + onSubmitted?: () => void; +} = {}): Promise { + // jsdom does not implement modal dialogs in every version, and the `Dialog` + // primitive calls `showModal()` from an effect. Stubbed to a plain open so + // the tests exercise the dialog's CONTENT rather than the UA's modality — + // same stub `ExportDialog.test.tsx` uses. + HTMLDialogElement.prototype.showModal = function showModal(this: HTMLDialogElement) { + this.open = true; + }; + HTMLDialogElement.prototype.close = function close(this: HTMLDialogElement) { + this.open = false; + }; + // FeedbackPositiveStep calls useGitHubStars on mount, which fetches on a + // cache miss and swallows failure — stubbed so this suite never touches the + // network (same reason PageShell.test.tsx stubs fetch). + vi.stubGlobal("fetch", vi.fn(() => Promise.reject(new Error("no network in tests")))); + vi.resetModules(); + vi.doMock("../../lib/analytics.ts", () => ({ + trackFeedback: opts.trackFeedback ?? (() => {}), + })); + const { FeedbackDialog } = await import("./FeedbackDialog.tsx"); + + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + await act(async () => { + root!.render( + createElement(FeedbackDialog, { + open: true, + onClose: opts.onClose ?? (() => {}), + onSubmitted: opts.onSubmitted ?? (() => {}), + }), + ); + }); + return container; +} + +afterEach(() => { + if (root) act(() => root!.unmount()); + container?.remove(); + root = undefined; + container = undefined; + vi.resetModules(); + vi.clearAllMocks(); + vi.unstubAllGlobals(); +}); + +/** The button whose visible text is exactly `label`, or null. */ +function button(el: HTMLElement, label: string): HTMLButtonElement | null { + return ( + [...el.querySelectorAll("button")].find( + (b) => b.textContent?.trim() === label, + ) ?? null + ); +} + +function rate(el: HTMLElement, value: number): void { + const star = el.querySelector( + `input[type="radio"][value="${value}"]`, + ) as HTMLInputElement; + star.click(); +} + +/** Type into a controlled `