Simplify the Guide Centre and reduce its bottom dock to the guided-tour action - #2191
Conversation
…r action The Guide Centre stacked six competing blocks on one phone screen and showed the same call to action three times at once. The bottom chrome was two fixed rows — a "Review guided tour" pill above a full "Search the guide" composer — so a reference dialog read as a chat surface and covered two rows of content. Bottom dock - Remove the search composer entirely, along with SearchResults and searchGuideTopics. Eight topics behind a dedicated "All topics" tab do not need keyword lookup, and the composer was the largest piece of chrome here. - The dock is now the guided-tour action alone. It keeps the shared answer-footer-search-dock geometry, scrim and scroll-hide behaviour, but moves to the compact scrim variant: the default 10rem height was sized for a composer plus an action row and tinted far more page than one control needs. - That action becomes the filled primary control on phones too. The outlined translucent framing existed because the pill was a dock addon beside a composer; with nothing to compete with and nothing left to cover, the surface's single call to action should read as one. - Previous is no longer sm:-only, so a phone user can step back through the tour with the room the composer freed. Content - Drop the in-content tour card and the footer's Browse all topics / Guide home / Ask a question buttons — all duplicated the tab bar or the dock. - Drop the "Answer anatomy" contents disclosure from Guide home, where it duplicated the All topics tab; it stays on topic pages as the in-context switcher. - Drop the "Top source" placeholder card, keeping its "Learn about sources" action as a text link under the illustrative answer. - Before you use an answer no longer renders three green ticks and an empty circle by hardcoded index. That implied live verification progress on a clinical surface with no state behind it. - Quick tasks no longer highlight "Verify an answer" as pre-selected. Sheet gains a footerVariant prop so a dialog can stamp data-footer-variant on the footer wrapper without reaching around the component. Tests updated in place, including new guards that the dialog contains no input and offers exactly one guided-tour action outside the tab bar. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013w5BcUoMbgPDk345sRNdzF
…ader Shortening the guide pages broke the scroll-hide the dock exists for: the button stopped disappearing entirely. Measured at 390x820, Guide home has a 329px scroll range while the collapsing chrome was the 153px Sheet header plus the 96px dock reserve. `collapseHasSafeRunway` refuses a hide that would eat that much of the remaining runway — correctly, since it would clamp the page under the finger — so it refused every hide. The budget was also charging chrome that does not exist here. `readChromeCollapseMetrics` resolves `universal-header-collapse` against the document, which from inside a fullscreen modal is the shell header behind the dialog; it releases nothing but was counted anyway. The dock now reads its budget straight off `[data-guide-content]`'s padding and reports `collapseKind: "reserve-only"`. The Sheet header no longer collapses. That is worth having on its own merits: "Close guide" and the view tabs live in it, so collapsing it inert left a reader who had scrolled down with no way out of the dialog. It is also not a symmetry violation — that rule is scoped to chrome sharing a scroll container, and this header is a sibling of the scroll body, not inside it. Verified in a browser at 390x820: scrolling down hides the dock and drops the content reserve to 0, scrolling up restores it, and the header stays reachable throughout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013w5BcUoMbgPDk345sRNdzF
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 16 minutes Limit details: You’ve used the included review currently available. Your 84 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (2)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. 📝 WalkthroughWalkthroughGuide Centre search was removed. The dialog now uses a compact guided-tour dock with a pinned header. Sheet supports a ChangesGuide Centre dock
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to This is a localized Guide Centre simplification with no actionable merge-blocking risk remaining after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant GuideDialog
participant ScrollHideReporter
participant Sheet
GuideDialog->>ScrollHideReporter: report dock reserve and scroll state
ScrollHideReporter->>GuideDialog: update dock visibility
GuideDialog->>Sheet: render compact footer
Sheet-->>GuideDialog: expose data-footer-variant
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
BigSimmo
changed the title
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #12558 (failure). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
… sweep
The Production UI failure was the tab sweep in the guide-centre test, not the
dock. Reproduced at 390x820: pressing Tab scrolls the scroll container, and a
scroll back up legitimately reveals the dock. The very first Tab already put
scrollTop at 0 with aria-hidden "false", so by the time focus reached the dock
button it was correctly visible and correctly focusable — the sweep asserted
against a revealed dock and failed.
That is also why pinning tabIndex={-1} on the dock buttons could not fix it:
dockHidden flips back to false mid-sweep, so inert and tabIndex have both
already cleared, exactly as they should.
Worse, the sweep could not have tested the intended property even when green.
Instrumenting it showed the dock was never once observed hidden during the
loop, so every iteration compared a visible dock against "must not be focused"
and passed vacuously.
Replace it with a direct probe of the real invariant: while the dock is
hidden, focus its button and assert focus does not move into it. No scrolling,
no dependence on tab order, and it asserts the hidden state itself first so a
revealed dock cannot make the check vacuous. Applied to both the ui-smoke
journey and tests/guide-centre-chrome.spec.ts, which carried the same loop.
Verified against the running app with the probe passing and the full guide
journey green: dock hides on scroll down and is inert and unfocusable, header
stays pinned with Close guide reachable, dock restores on scroll up, no
composer or input, topic navigation and tour step focus intact.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013w5BcUoMbgPDk345sRNdzF
2 participants
Summary
Two commits, both scoped to the Guide Centre dialog (
GuideDialog, opened from Settings → "Guide & help").Simplify the Guide Centre and reduce the bottom dock to the tour action. The dock held a "Review guided tour" pill and a full "Search the guide" composer, so a reference dialog read as a chat surface and covered two rows of content. Guide search is removed entirely — composer,
SearchResults, andsearchGuideTopics— because eight topics behind a dedicated "All topics" tab do not need keyword lookup. The dock keeps the sharedanswer-footer-search-dockgeometry, scrim and scroll-hide, but moves to the compact scrim variant (the 10rem default was sized for composer + action row) and its single control takes the filled primary treatment rather than the quiet addon framing that existed only because a composer shared the dock.Same commit, content side: "Guided tour" appeared three times at once (tab, in-content card, docked pill) — the card is gone, as are the footer's "Browse all topics" / "Guide home" / "Ask a question" buttons and the Guide-home contents disclosure, all of which duplicated the tab bar. The "Top source" card was placeholder text ("Illustrative guideline · source page", "Updated · Section · Page") wrapped in a full accent card; its only real function, the sources link, is now a text link under the illustrative answer. "Before you use an answer" rendered three green ticks and an empty circle by hardcoded index — static decoration reading as live verification progress on a clinical surface — and is now a plain checklist. "Verify an answer" no longer renders permanently pre-selected.
Sheetgains afooterVariantprop so a dialog can stampdata-footer-varianton the footer wrapper without reaching around the component.Hide only the dock on scroll, and pin the Guide Centre header. Shortening the pages broke the scroll-hide the dock exists for. Measured at 390×820, Guide home has a 329px scroll range while the collapsing chrome was the 153px Sheet header plus the 96px dock reserve;
collapseHasSafeRunwayrefuses a hide that would eat that much runway — correctly, since it would clamp the page — so it refused every hide. The budget was also charging chrome that does not exist here:readChromeCollapseMetricsresolvesuniversal-header-collapseagainst the document, which from inside a fullscreen modal is the shell header behind the dialog. The dock now reads its budget straight off[data-guide-content]'s padding and reportscollapseKind: "reserve-only".The header no longer collapses, which is worth having on its own merits: "Close guide" and the view tabs live in it, so collapsing it inert left a reader who had scrolled down with no way out of the dialog. This is not a header/footer symmetry violation — that rule is scoped to chrome sharing a scroll container, and this header is a sibling of the scroll body, not inside it.
Net 357 insertions / 601 deletions across 10 files.
docs/search-chrome-behaviour.mdis updated for all of it, including why the compact scrim and the filled pill are now correct where the opposite was correct before.Verification
npm run verify:pr-local—failed: (none); all 18 stages completed, includinglint,typecheck,test,build,format:changed, and the docs/ledger contracts.Test Files 683 passed (683)·Tests 7436 passed | 4 skipped (7440).UI verification not run:
npm run verify:uiand the browser stages ofnpm run verify:phone-chromecannot run in this container.check:playwright-browser-revisionreports the known drift in/issues#255— the image ships Chromium 1194 and Playwright 1.62.1 pins 1234 — and perAGENTS.mda mismatched Chromium path must not be forced. The gate's non-browser stages did pass (check:installed-lock-parity,check:runtime, and 130/130 phone-chrome static and unit contracts). Browser proof for this PR has to come from CI Production UI.Behaviour was instead confirmed manually at 390×820 against the pre-installed Chromium via an explicit
executablePath— a visual check, not the gate. Scrolling down hides the dock and drops the content reserve to0px; scrolling up restores it; the header and "Close guide" stay reachable throughout. The dock reportsinputs: 0, one button, a transparent band, a 112px (compact) scrim, and a filled--commandpill at 48px minimum height.Not applicable to this diff:
eval:retrieval:quality,eval:rag,eval:quality,check:production-readiness,check:deployment-readiness,verify:release. No retrieval, ranking, ingestion, answer-generation, privacy, environment or deployment surface is touched.Risk and rollout
Sheetgains one optional prop with no default behaviour change, so no other dialog is affected. The scroll-hide change is local to this dialog's own scroll reporting and touches no shared hook.git reverteither commit independently, or both. The second commit only makes sense on top of the first — reverting the first alone would restore the long pages that made the old two-owner collapse affordable, so revert both if backing the feature out entirely.Clinical Governance Preflight
Not applicable.
classifyPullRequestFiles(scripts/pr-policy.mjs) returnsclinicalRisk: false,operationalRisk: false,ragRanking: false,ui: truefor these ten paths. The diff touches no ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.guide-content.tsis static UI help copy, not clinical evidence, and the illustrative answer it renders remains explicitly labelled illustrative with no real source attached.Notes
localStorageunder the same key; no migration.inputand no[data-guide-universal-search], and must offer exactly one guided-tour action outside the tab bar — the singulargetByRolequery is itself the guard against the duplicate card returning.tests/guide-centre-chrome.spec.tsnow resolves--commandthrough a probe element rather than hard-coding a hex, so a token change moves the expectation instead of failing the gate.Summary by CodeRabbit