feat(feedback): replace inline panel with milestone-triggered FeedbackDialog (#900)

[s-annam]

Summary

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 trigger: a [★ Feedback] button in ParsedHeader, available on demand.
• Flow: Step 1 is a 1–5 star rating; 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, cleanly suppressed when ANALYTICS_ENABLED === false.

Closes #900

Adversarial review

Two rounds, pre-push, on the full branch diff.

Round 1 — 4 blocking findings, all fixed:

1. Auto-trigger reused the retired panel's ocv_feedback_seen key with different increment semantics — every existing browser already had it non-zero, so the new trigger would never fire. Fixed with a dedicated ocv_feedback_dialog_seen key.
2. The dialog could open on top of the still-open ExportDialog, burying the export-findings advisory. Fixed: the open is deferred to ExportDialog's onClose.
3. README.md's storage-key table still described the retired panel and a dead key (ocv_star_cta_seen). Reworded/removed to match current code.
4. Submitting closed the dialog with no confirmation (a11y regression vs main, which had a focused thank-you region). Fixed: an aria-live="polite" confirmation step with explicit Close.

Round 2 — clean: true. Each fix re-verified independently (not from the fix report) by tracing every caller and running the tests live. 5 nits surfaced and are left for a human pass, not blocking: a force-close edge case can leave the auto-trigger armed until the next export dialog interaction; useGitHubStars() double-fetches on cold cache; the step bodies lost <form>/Enter-to-submit and a visible email <label> vs the retired panel; the positive step's title thanks the user before anything is actually submitted.

Validation

npm run typecheck, npm run lint, and the affected vitest suites (useFeedbackDialog, FeedbackDialog, ExportDialog, ParsedHeader, Result, App.*) all pass. npm run check:fixtures clean (no fixtures touched). Pre-push verify passed (fallow complexity/duplication findings are report-only per repo convention).

[cloudflare-workers-and-pages]

Deploying offlinecv with    Cloudflare Pages

Latest commit:	61314d1
Status:	✅  Deploy successful!
Preview URL:	https://9ea9a911.offlinecv.pages.dev
Branch Preview URL:	https://gh-900-feedback-visibility-d.offlinecv.pages.dev

View logs

[rohithgollapalli]

Reviewed against #900's acceptance criteria first, then the diff, then the description. All nine ACs are implemented, the branch is one commit, and typecheck / lint / the four touched suites are green locally. Two findings block: both are in Step 1's rating gesture, and both hit keyboard and touch users specifically.

Blocking

1. Step 1 has no dismiss control, and this is the only dialog in the repo that opens itself. FeedbackDialog.tsx:105-118 renders a <p> and a StarRating — zero buttons. Verified by mounting it: STEP1 BUTTONS: [], STEP1 FOCUSABLES: ["INPUT:radio" x5]. The Dialog primitive draws no close affordance of its own, sets no closedby, and native <dialog>+showModal() does not light-dismiss on a backdrop click, so the only exit is Esc. On touch there is no Esc key, and this dialog opens unprompted after the first export. Every sibling dialog gives an explicit out — ExportDialog.tsx:258 Close, ResumeChooserDialog.tsx:137 Cancel; steps 2A/2B and thanks here do too. Only the auto-opened step does not.

2. A keyboard-only user cannot reach a 4-5★ rating at all. StarRating is a native radio group — its own docblock says "arrow keys move between stars and select (native radio behaviour)". FeedbackDialog.tsx:88-91 routes on the first onChange, so Tab into the group + first Arrow press selects star 2 and immediately unmounts the group into FeedbackConstructiveStep. There is no way to move focus across radios without selecting, and no Back on step 2, so a keyboard or screen-reader user is confined to 1-2★ and cannot correct an overshoot. Mouse users hit the same wall on a mis-click. This is also AC 4 ("keyboard/touch navigation") not being met under a Closes #900.

Both fall out of one change: a ghost Close on the rating step, and a Back on steps 2A/2B that returns to "rating" (the routing-without-a-next-click stays intact for pointer users).

Secondary

3. useGitHubStars() runs at App mount for a dialog that is usually never opened — FeedbackDialog.tsx:65. App.tsx:760 mounts FeedbackDialog unconditionally, Dialog renders children even while closed, and the hook fetches from a []-dep effect. Verified: mounted with open: false, FETCH CALLS: ["https://api.github.com/repos/offlinecv/OfflineCV"]. PageShell.tsx:108 already calls the same hook on every page load, so on a cold cache both race and you spend two of the 60/hr unauthenticated budget for one number. The PR body lists this as a known nit; the clean fix is to move the hook into FeedbackPositiveStep, which only mounts on the 4-5★ branch — that also deletes the starCount prop threading.

4. FeedbackDialog.test.tsx makes a real network request on every run. It stubs showModal/close and mocks analytics.ts, but never fetch — and src/test-setup.ts's font shim delegates every non-font URL to the real fetch, so the same api.github.com call above fires in CI. PageShell.test.tsx:91-96 stubs it for exactly this reason. The suite's 2106 ms (vs 163-484 ms for its neighbours) is this.

5. The email opt-in field is a verbatim duplicate across the two new step files — FeedbackPositiveStep.tsx:68-77 and FeedbackConstructiveStep.tsx:86-95, identical eleven lines. This is the pressure that produced Checkbox in the first place (its docblock: "would otherwise have been the THIRD verbatim copy of the same six lines"). It also drops the visible <label>Your email</label> the retired panel carried — placeholder plus aria-label only, which is WCAG 3.3.2. One EmailOptIn shared by both steps fixes both halves.

6. GitHubStarCta's variant="card" branch now has no consumers. FeedbackPanel's local StarCtaOnce was its only caller and went with the file; grep -rn 'variant="card"' src/ returns nothing. Its docblock still describes it as "used inside FeedbackPanel's post-positive-feedback thank-you surface". Either delete the variant or point the docblock at the intended consumer. (fallow does not catch it — it is a prop value, not an export.)

Nits

7. Five docblocks still reference the deleted FeedbackPanel. Checkbox.tsx:16 is the one that is now actively wrong — "FeedbackPanel is still unmigrated ... it belongs in its own change" — this PR is that change, and both new steps use the primitive. Also ReportGapSection.tsx:15, GitHubStarCta.tsx:13, ResumeLibrary.test.tsx:11, ReportGapSection.test.tsx:7-8. None are in the diff, so no inline anchor.

8. Nothing tests the App-level wiring end to end. useFeedbackDialog and FeedbackDialog are each covered well, and ExportDialog.test.tsx:437 pins onResumeExported, but no test asserts that export → close ExportDialog → FeedbackDialog opens through App.

9. GitHubStarCta's policy list says "✗ No star modal on app load" and calls itself load-bearing. This PR still satisfies it (post-export, not app load), but it is the first time the CTA lives inside a self-opening modal — worth one line in that docblock so the next reader does not have to re-derive the distinction.

Gates

Gate	Result
/code-review correctness pass	folded in above
3a fixture PII	skipped — no fixtures touched
3b design-system / reuse	pass on the ESLint-enforced surface; finding 5 is the soft half
3c style tokens	pass — the only hex-shaped matches are #900/#421/#621 issue refs
3d fallow	pass as a gate; 1 complexity finding on App.tsx (37 cyclomatic, pre-existing, +10 lines here), 16 clone groups (warn). Report-only per CLAUDE.md
3e command-level	skipped — no skill/script/bash files in the diff
3f description accuracy	accurate; it discloses findings 3 and the label half of 5 itself. Omits findings 1, 2, 4 and 6
typecheck / lint	green
touched suites	47 tests green (FeedbackDialog, useFeedbackDialog, ExportDialog, ParsedHeader)

Verdict rule: ≥1 Blocking finding → REQUEST_CHANGES, and nothing was committed or pushed to the branch. It is already one commit, so the invariant is intact.

---

Reviewed by: Claude Opus 5 (high)

[s-annam]

Addressed all 6 review threads and collapsed to one commit (56493f0):

• Step 1 had no dismiss path — added a ghost Close beside the stars, matching ExportDialog/ResumeChooserDialog.
• Keyboard lockout on 4-5★/1-3★ routing — added a Back button to both step-2 bodies; the star group is a native radio, so a keyboard overshoot was previously unrecoverable.
• useGitHubStars fired on every page load — moved the fetch into FeedbackPositiveStep, which only mounts on the 4-5★ branch.
• Test suite hit the network — stubbed fetch in FeedbackDialog.test.tsx; suite now runs in ~190ms vs ~2.1s.
• Duplicate email opt-in, no visible label — extracted a shared EmailOptIn (src/design-system/shared/EmailOptIn.tsx) used by both step bodies, with a real <label> instead of placeholder-only.

typecheck/lint clean, full suite green (380 files / 6328 tests). Re-requesting review — ready for another look.

[s-annam]

Ready for the next review round.

Summary of Latest Changes

1. Unconditional Feedback Availability (Feedback control gets too quiet after 2 views — testers can't find/recognize it #900):
   • Removed ANALYTICS_ENABLED gating from FeedbackDialog, useFeedbackDialog, ParsedHeader, Result, and App. Feedback triggers are no longer suppressed when VITE_POSTHOG_KEY is absent (local dev / OSS builds), allowing users to submit feedback at all times.
2. Home Page & Pre-drop Feedback Trigger:
   • Added an ambient Feedback link in the shared PageShell footer so users can submit feedback from the home page or any surface.
3. Score Card Ambient Trigger:
   • The [★ Feedback] button in ParsedHeader is unconditionally passed down and visible in the score card header row when parsed.
4. Review Findings Addressed:
   • Step 1 explicit dismiss control (Close).
   • Step 2 Back navigation to allow correcting ratings via keyboard/mouse.
   • Lazy-loading GitHub star fetching inside FeedbackPositiveStep.
   • Shared EmailOptIn component with visible label.
   • Removed obsolete FeedbackPanel references and cleaned up test network mocks.
5. Verification:
   • npm run typecheck, npm run lint, and all 380 vitest test suites (6,328 tests) pass.
   • Pre-push verification hook (npm run verify, vite build, fallow audit) passed cleanly.
   • Branch collapsed to a single commit (61314d1).

[s-annam]

Merging this now rather than holding it — I'm going to be away from the computer and don't want it sitting open while I'm offline.

Functionality is verified through the UI.

@rohithgollapalli — I'm merging ahead of clearing your review comments from 8/26, so they're not lost, just not blocking. Happy to take anything still outstanding as a follow-up.

[offlinecv-approver]

🕹️✔️

Both Blocking findings were addressed in 61314d1 (force-pushed 2026-08-27): the rating step now carries a ghost Close, and steps 2A/2B carry Back returning to "rating" — with tests pinning that the dialog closes only on Close. Secondary findings 3-6 and the nits were addressed or answered; all six review threads are resolved. verify / fallow / Cloudflare Pages are green and offlinecv-approver has approved the current head. Dismissing so the merge queue is not held by a review written against the pre-revision diff.