feat: make approval mode discoverable from the approval bar - #5942
Conversation
|
Pre-push review round (local GPT + Opus lane mirrors + a new-user usability reviewer) on pre-fix head 2b503fa57 — all blocking findings fixed in 918b20e (see PR body). Declined findings, each with rationale:
|
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS A real first-session friction point, solved with layered, self-retiring UI that adds no backend surface and is fully reversible (two components + one localStorage flag). The hint/nudge/spotlight trio could have been noise, but the mutual suppression (one pointer at a time), the single shared retirement flag, and the forever-vs-sitting dismissal split are all deliberate, documented in-code, and pinned by tests — the intricate part of this design is exactly the part with the strongest coverage. Capture harness and screenshots follow the repo's established convention. No contract, schema, or one-way-door changes. [DESIGN-REVIEWED] 715a4a8 |
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of I have everything I need. The capture harness and committed screenshots follow a documented repo convention (2,393 files under First-Principles-Verdict: CONCERNS The B2 threshold nudge is a second pointer at the same control the A1 hint already points at, on an unsupported requirement — no report, no issue, "design mockups… in-session". What this change shipsIntent: teach new users, at the moment of approval friction, that prompting is adjustable via the footer mode picker. ADDITION.
WatchItem 3's zero option: during all three counted approvals the A1 hint ("Tired of confirming every step?") is already on the approval bar — the description itself has A1 fire on the first approval and retire only on discovery, so B2 escalates a pointer the user has now seen three times. The only incremental harm is banner-blindness, and the description concedes there is no fundamental behind it: "no linked issue: product improvement chosen from design mockups by the maintainer in-session." B2 carries most of the PR's permanent surface — the Popover callout, dual dismiss/hide semantics, Subtractions
[FIRST-PRINCIPLES-REVIEWED] 715a4a8 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsI've verified the core logic paths myself: No findings. [OPUS-REVIEWED] 715a4a8 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
UX Review (Fable 5) — 🟡 CONCERNSUX-level review of UX-Verdict: CONCERNS Solid teaching flow — but outside-click only hides the nudge for the sitting, so the callout re-summons itself on every subsequent approval. Watch
Suggestions
[UX-REVIEWED] 715a4a8 |
918b20e to
384b208
Compare
|
First Principles CONCERNS on 918b20e, per finding:
|
|
UX CONCERNS on 918b20e, per finding:
|
384b208 to
5d9e5be
Compare
|
Opus advisory on 384b208:
|
|
UX CONCERNS on 384b208, per finding:
|
|
First Principles CONCERNS on 384b208, per finding:
|
|
Design CONCERNS on 384b208:
|
5d9e5be to
b917390
Compare
|
UX CONCERNS on 5d9e5be, per finding:
|
|
First Principles CONCERNS on 5d9e5be:
|
b917390 to
715a4a8
Compare
|
UX CONCERNS on b917390, per finding:
|
|
First Principles CONCERNS on b917390, per finding:
|
|
UX on 715a4a8 (Suggestion only, no Watch items — converged):
|
|
First Principles on 715a4a8, per finding:
|
|
Review-ready at head 715a4a8: 62/62 checks green (0 pending, 0 failing), MERGEABLE, all 5 AI review lanes settled — GPT / Opus / Design PASS; First Principles and UX advisory CONCERNS each answered with per-lane, per-finding dispositions (latest: this head's two comments above). Every raised concern across 5 review rounds has a recorded disposition: 15 findings fixed with pinning tests, the rest rebutted or deferred with rationale. Awaiting human review and merge. |
Problem / Motivation
New users who hit the chat approval bar only see Allow once / Trust / Reject. Nothing tells them that approval prompting itself is adjustable (the Normal / Reads / Trust / YOLO picker in the composer footer), so repeated prompts read as unavoidable friction — the picker is effectively undiscoverable at the exact moment its existence matters.
Why it matters
Approval fatigue is a first-session churn driver: a user who does not find the mode picker either rubber-stamps every prompt (annoyance) or gives up on agentic work. Users who DO know the picker exists get no cost; discoverability only has to fire once.
What changed (motivation → approach → change)
Goal: teach where the real control lives, without re-adding an on-card trust decision (the chat approval endpoint is one-shot; an on-card Trust tier was deliberately removed in #5433/#5434, so a fake affordance is off the table).
ChatInput.tsx): a muted "Tired of confirming every step?" + accent link "Adjust approval mode↗" under the action row. Withheld for unattended sources (cron/heartbeat/taskrunner — the picker governs this slot, not the job), in the ghost/collapsed composer state (the picker is unmounted there, so the link would be a dead end), while the B2 callout is up (one pointer at a time), and permanently once the user has found the picker (via the link, or by changing the mode).ApprovalModePicker.tsxgains anopenSignalprop): the footer picker opens with a 2s spotlight ring on its trigger, so the user learns the control's home rather than a one-off popup.normal, a one-timerole="dialog"callout anchors above the picker ("Want fewer approval prompts?") with "See options" (opens the picker) and "Got it". Either action, or picking a mode, dismisses it forever (localStorage). Counting is in-memory per dashboard session by design — stale history should not fire it.The callout is a Radix
Popover/PopoverAnchor/PopoverContent(portalled,collisionPadding={8}) — the same collision machinery the picker's own menu already uses — because the composer control row is a horizontally scrollable container that clips absolute children, and an earlier hand-rolled fixed-position draft overflowed 390px viewports. Keyboard path: focus capture is deliberately suppressed while an editable element has focus (the callout mounts from an async approval completion, so the user may be mid-word in the composer), Escape dismisses, and dismissal returns focus to the picker trigger. The trigger wears the spotlight ring while the callout is up, anchoring the "this is where" copy visually. ANY menu open — trigger click included — or either button dismisses the callout for good (and permanent dismissal also retires the A1 hint: the callout has delivered the lesson). Escape or clicking anywhere outside hides it for the sitting only — reflexive gestures must not spend the one-time teaching moment unseen (verified end-to-end: the capture harness clicks outside, asserts the callout hides, then re-fires on the next approval). One localStorage flag carries both retirements (hint + callout).New i18n keys (
approval_hint_question,approval_hint_adjust,nudge_title,nudge_body,nudge_dismiss,nudge_view_options) across all 13 catalogs with{{productName}}interpolation; en-XA regenerated. The nudge body says "Choosing "Trust" here…" to disambiguate from the approval bar's own per-command Trust ▾ button.A pre-push review round (GPT + Opus mirrors + a new-user usability reviewer) drove seven fixes now in this diff: the narrow-viewport clamp, no-op re-pick not retiring the hint, dialog autoFocus + Escape, an owned
nudge_dismisskey instead of borrowingmarkdownPanel.got_it, the "here" copy anchor, hint retirement on link click, and hint suppression while the callout is up.Tests
website/src/test/ApprovalModePicker.nudge.test.tsxandwebsite/src/test/ChatInput.approvalNudge.test.tsx(28 tests total), mutation-verified:openSignalbump opens the menu + spotlight ring; ring clears on timeout; a mount-time signal does not replay on slot-switch remount.normal, not once dismissed; "See options" / "Got it" / Escape / ANY menu open (trigger click included, so a closed menu cannot resurrect the callout) each dismiss; focus is NOT stolen while an editable element has focus; the trigger wears the spotlight ring while the callout is up.Manual verification
Self-checking capture harness (
website/capture/approval-mode-discover.*+website/scripts/capture-approval-mode-discover.mjs): mounts the realChatInputwith a seeded pending approval, drives the REAL hint click and three REAL "Allow once" clicks (API answered by route interception), asserts each state before writing a frame. 7/7 frames pass, including a 390px bounding-box containment check for the clamp.Screenshots / video
Light theme + 390px narrow-viewport clamp
Related Issues
no linked issue: product improvement chosen from design mockups by the maintainer in-session; no tracking issue was filed.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)