redesign(documents): document-first viewer with sticky evidence rail#489
Conversation
- Desktop: widen evidence rail 420->480px, make it sticky with internal scroll; widen page container to 1440px and centre the document column; hero info cards flow 3-up. - Tablet (md): deliberate layout instead of the phone stack - full-width PDF, evidence + indexed text 2-up, details/summary 2-up. - Phone: document-first order (anchors, PDF, evidence, collapsible text). - Remove the Index health block and duplicated actions (preview-toolbar Open/Download, per-chunk Open source); keep the Canvas/Browser toggle as a single quiet control. - Fix latent shell bug: overflow-x-hidden on #main-content made it the sticky scrollport while the window scrolls, disabling every sticky descendant on sm+; use overflow-x-clip there instead. - Update ui-smoke ordering assertions for the document-first phone flow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe document viewer removes index health state and UI, improves chunk highlighting, changes PDF and evidence ordering, restructures responsive source and evidence layouts, updates overflow behavior, and revises related smoke test assertions. ChangesDocument Viewer Updates
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/DocumentViewer.tsx`:
- Around line 2715-2754: Update the deep-link scroll logic associated with
IndexedTextPanel so it targets the active visible panel using its
branch-specific idPrefix rather than the first data-source-chunk-id match in DOM
order. For the mobile source-text details element, expand the collapsed <details
id="source-text-mobile"> before calling scrollIntoView; keep desktop scrolling
scoped to the visible desktop panel.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 961ba547-febb-4e1b-8130-74db714d9475
📒 Files selected for processing (3)
src/components/DocumentViewer.tsxsrc/components/clinical-dashboard/global-mockup-search-shell.tsxtests/ui-smoke.spec.ts
Both IndexedTextPanel copies render the pinned chunk, so the scroll-to-pin querySelector returned the first (hidden) match: display:none on lg+ and a collapsed <details> on phones. Skip display:none matches, ignore chunks inside a closed <details>, and expand the mobile section when the pinned chunk only exists there, so ?page=&chunk= deep links land on the copy the user can see. Addresses CodeRabbit review on #489. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…55b68b' into codex/pr-489-fixes # Conflicts: # src/components/DocumentViewer.tsx
# Conflicts: # docs/branch-review-ledger.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/ui-tools.spec.ts`:
- Around line 534-537: Update the compact hero mobile heading size assertion in
the relevant UI tools test to enforce the exact 26px value, replacing the
insufficient zero-precision toBeCloseTo check with toBe(26). Keep the icon
dimension assertions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b8ca9298-c0cf-4091-b09d-97260a421dda
📒 Files selected for processing (3)
docs/branch-review-ledger.mdtests/ui-smoke.spec.tstests/ui-tools.spec.ts
✅ Files skipped from review due to trivial changes (1)
- docs/branch-review-ledger.md
All actionable threads are resolved on the current head; dismissing the stale blocking review for verified merge readiness.
# Conflicts: # docs/branch-review-ledger.md # tests/ui-tools.spec.ts
…esign Reconcile two redesigns of /documents/[id]. #489 restructured layout (document-first hero, sticky evidence rail, section anchors); this branch adds the content-quality fixes (#492): smart summary formatter, badge system, flattened pinned evidence, flowing indexed source text, and removal of the meta "Document details" card. Conflict resolution in DocumentViewer.tsx: - Keep #489's sticky-rail <aside> (grid + lg:sticky); nest my extraction- warnings InlineNotice inside it (md:col-span-2 lg:col-span-1). - Drop #489's trimmed "Document details" counts card (user feedback: meta content); my high-yield summary section already owns id="source-summary". - Re-add the indexHealth state + fetch wiring #489 removed, so extraction warnings stay visible and the demoted "Indexing details" disclosure works. - Span the high-yield summary and indexing-details full width on md now that their grid pair (the details card) is gone. Verified: tsc, eslint, prettier, 108 unit tests, no duplicate ids, zero horizontal overflow at 768/1440 light+dark. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
Layout/IA overhaul of the document viewer (
/documents/[id]) so opening a document feels document-first. No changes to PDF rendering or data fetching.canUsePrivateApisdisclosures.#main-contentusedoverflow-x-hidden, which forcedoverflow-y: autoand made it the sticky scrollport while the window actually scrolls — silently disabling everyposition: stickydescendant on sm+ (including the existing PDF toolbar). Switched tooverflow-x-clipon sm+ (identical clipping, no scroll container); phone keepsoverflow-x-hidden.#source-evidence/highlighted-source-passage,pinned-source-evidence,pdf-preview,pdf-toolbar,form.document-viewer-composer) and the?page=&chunk=deep-link pinning.Verification
npm run verify:pr-local— not run as the single command; ran its constituents:npm run verify:cheapgreen (lint, typecheck, 1489 vitest passed) andformat:check(see note) locally.npm run verify:ui— 128 passed / 2 failed, both failures unrelated to this diff and pre-existing onmain:ui-tools.spec.ts:489(mobile hero sizing) fails on a sub-pixel assertion (26 vs 25.6, 0.4px) at 390px, where this diff is a literal no-op (onlysm+overflow changed;max-smunchanged) — a known WindowsgetBoundingClientRectrounding artifact.ui-smoke.spec.ts:1891(differentials/api/searchrouting at 1280px) — this diff touches no differentials/search code.main's CI (Linux); leaving them for the CI gate to confirm.npm run eval:retrieval:quality— N/A (no retrieval/ranking/selection/chunking/scoring change).Note on
format:check: locally flags onlyCLAUDE.mdand.claude/settings.json— both outside this diff, byte-identical toorigin/main, flagged solely because Windows checks them out CRLF (text=auto) while CI (Linux/LF) passes them. The three changed files are LF-clean and pass prettier.main's current CI is green with the same file contents.Clinical Governance Preflight
N/A — UI-only layout/IA change. No ingestion, answer generation, search/ranking, source rendering, document-access, privacy, production-env, or clinical-output behavior was touched. Supabase target and server-only key handling unchanged.
Notes
🤖 Generated with Claude Code