fix(document-viewer): stop wide clinical tables being clipped off-screen on phones - #1737
Conversation
The rail <aside> declared `grid` with `md:grid-cols-2` and `lg:grid-cols-1`
but no base track, so below `md` it used an implicit `grid-auto-columns: auto`
column whose minimum is the min-content of its items. One wide descendant
stretched that single track and every card in the rail inherited the blown-out
width; `html { overflow-x: clip }` then amputated the remainder instead of
making it scrollable, so clipped table columns were unreachable by any gesture.
The wide descendant was the SignedImage frame. CSS `aspect-ratio` transfers
size constraints across axes, so `min-h-40` (160px) on a 3:1 table crop became
a *minimum width* of 480px. Measured in Chromium at a 393px viewport: the card
rendered 560px wide. Dropping the min-height, capping the base track, and
giving each rail grid item `min-w-0` each fix it independently; all three are
applied because the failure is invisible in jsdom and silent in review.
The outer document body grid had the identical shape (no base track, only
`lg:grid-cols-[minmax(0,1fr)_480px]`) and is capped for the same reason.
Demo image rows gain their real intrinsic sizes so the wide-crop layout path
is reachable from a browser gate at all — without width/height every demo crop
renders at the 4:3 fallback, which fits any phone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WezqwTuQqG9BRb8wPNkYvp
A wide table crop cannot be read inside a phone card at any faithful size — object-contain in a ratio-driven frame is already the largest honest rendering — so the card's job is an unclipped thumbnail with an unmissable route to the full-screen viewer, and the viewer's job is to actually be readable. Inline card: the expand chip was opacity-0 until hover, so on a phone the one cue that a clipped table opens full screen never appeared at all. It is now visible under `hover: none` and unchanged for hover-capable pointers. Crops wider than 2.2:1 also get a labelled "Open full screen" control, reusing the same Button recipe as AccessibleTable's "Expand table" rather than inventing a fourth affordance. Viewer: the geometry moves to a pure module so it can be proven in the node project — jsdom has no layout, so none of these numbers were testable before. Four behaviours now derive from one measurement: - Opens at source-pixel density (capped at 3x) when the contained fit leaves the stage mostly empty; a near-square figure still opens whole, because its shape is worth seeing first. Anchored top-left, at the first column and row. - Rotation re-fits. `object-contain` sizes the box before rotation and `rotate()` is a pure transform, so a wide table turned upright used to spill out of the stage instead of filling it — which is what made rotate useless for the one case it is most needed. - Panning is clamped, so a zoomed image can no longer be dragged off-screen and lost with no gesture that brings it back. - Double tap toggles fit/legible at the tap point, opt-in so the PDF canvas is untouched. Controls move to the Sheet footer: the panel already carries pb-safe, so they clear the home indicator with no env() double-counting, they stop occluding the bottom rows of a table, and the pointer-events juggling that only existed because the bar sat on the gesture surface goes away. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WezqwTuQqG9BRb8wPNkYvp
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 24 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling 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 (19)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WezqwTuQqG9BRb8wPNkYvp
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2394d903a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/document-image-mobile-view-30xzw8 at starting commit 8fa1695; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/document-image-mobile-view-30xzw8, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #9070 (failure). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why. Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker. If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop. Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them. Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head. Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol. Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it. Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted. Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval. Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed. One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1. Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me. |
|
PR #1737 at tip Confirmed targetOnly one matching open PR: #1737 ( Fixed and pushed
Threads / CI / gates
Merge left to you once required checks settle.   |
Align the wide-crop expand threshold with the real clozapine demo ratio, open the source-image disclosure in the phone journey, convert double-tap anchors into stage coordinates, complete taps only on pointer-up within slop, and re-clamp pan after stage resize. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Review-and-fix summary (PR #1737)Tip: Fixed (pushed)
ThreadsInline review-comment replies returned 403 for this token (
Required CIPrior tip Local gates (decisive)Not run (no separate approval / env): Residual risks
Merge left to you. |
Keep main's phone rail grid-cols-1 fix and double-tap gesture coverage; retain this PR's className/collapse-height rail chrome and pinch-gating tests. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Behind-but-clean vs origin/main (fc399e2); clears GitHub DIRTY/CONFLICTING so squash auto-merge can proceed. No content conflict. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
3 participants
Summary
Two iPhone screenshots of a clinical table in Documents → Tables and diagrams showed the table cut off at the right edge of the screen, with no way to scroll or pan to the missing columns. On a clinical reference tool that is a correctness problem rather than a cosmetic one: the visible half of a pain-mechanism or opioid-adverse-effect table reads as the whole table.
fix(document-viewer): stop phone image cards overflowing off-screen. The rail<aside>declaredgridwithmd:grid-cols-2andlg:grid-cols-1but no base track, so belowmdit used an implicitgrid-auto-columns: autocolumn sized by its items' min-content. The wide descendant was theSignedImageframe: CSSaspect-ratiotransfers size constraints across axes, somin-h-40(160px) on a 3:1 table crop became a minimum width of 480px. One item stretched the single track and every card in the rail inherited the blown-out width;html/bodyoverflow-x: clipthen amputated the remainder instead of making it scrollable. Measured in Chromium at a 393px viewport: the card rendered 560px wide. Each of three guards fixes it independently (basegrid-cols-1,min-w-0on each rail grid item, dropping the redundantmin-h-40); all three are applied because the failure is invisible in jsdom and silent in review. The outer document body grid had the identical shape and is capped for the same reason. Demo image rows gain their real intrinsic sizes so the wide-crop path is reachable from a browser gate at all — withoutwidth/heightevery demo crop renders at the 4:3 fallback, which fits any phone.feat(document-viewer): make wide clinical tables readable on a phone. A wide crop cannot be read inside a phone card at any faithful size, so the card is an unclipped thumbnail with an unmissable route to the viewer, and the viewer is made genuinely readable. The expand chip wasopacity-0until hover, so on a phone the one cue that a clipped table opens full screen never appeared at all; it is now visible underhover: noneand unchanged for hover-capable pointers, and crops wider than 2.2:1 also get a labelled "Open full screen" control reusingAccessibleTable's existing Button recipe. Viewer geometry moves to a pure module (image-lightbox-geometry.ts) so it can be proven in the node project — jsdom has no layout, so none of these numbers were testable before. Four behaviours derive from one measurement: it opens at source-pixel density (capped 3×, anchored top-left at the first column and row) when the contained fit leaves the stage mostly empty, while a near-square figure still opens whole; rotation re-fits (object-containsizes the box before rotation androtate()is a pure transform, so a wide table turned upright used to spill out of the stage instead of filling it — useless for the one case it is most needed); panning is clamped so a zoomed image can no longer be dragged off-screen and lost; double tap toggles fit/legible at the tap point, opt-in so the PDF canvas is untouched. Controls move to the Sheet footer, whose panel already carriespb-safe— clearing the home indicator with noenv()double-counting, no longer occluding the bottom rows of a table, and removing the pointer-events juggling that only existed because the bar sat on the gesture surface.A note for reviewers on why no existing gate caught this:
expectNoPageHorizontalOverflowmeasuresdocumentElement.scrollWidth - clientWidth, and clipped overflow does not extendscrollWidth. That helper is structurally incapable of reporting this class of bug and passed on every run while the rail was well over twice the viewport width. The new Playwright spec measures each element's owngetBoundingClientRect()against the viewport instead, and asserts the blind check alongside it so nobody later "simplifies" it back.Verification
npm run verify:pr-localVerification not run (as a single command):
check:installed-lock-parityfails closed in this remote container —playwright: installed 1.62.0 does not match locked 1.62.1. That is the standing container defect recorded as#255indocs/outstanding-issues.md, unrelated to this diff, and it halts the gate at step 2 of 15 for any scope. Every selected step was therefore run by hand:The one failing test is
tests/pr-handoff-stop.test.ts > emits handoff context only when the marker file exists. It was confirmed failing on a clean tree with this branch's changes stashed, so it pre-dates this PR and is untouched by it.New and updated coverage:
tests/image-lightbox-geometry.test.ts(new, 23 tests) — rotation re-fit, opening scale, pan clamping and tap-anchored zoom against real measurements, plus the degenerate paths that must never emitNaNinto a transform.tests/document-image-phone-layout-contract.test.ts(new) — the class contract that regressed: basegrid-cols-1on both grids,min-w-0on every rail grid item, and nomin-h-*beside an explicitaspectRatio.tests/ui-phone-scroll-document-rail.spec.ts(new) — per-element phone geometry and the rotate/footer behaviour, registered inphoneChromePlan's consumer list.tests/use-viewer-gestures.dom.test.tsx,tests/signed-image.dom.test.tsx,tests/image-lightbox-url-mode.dom.test.tsxextended.npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changedUI verification not run: this remote container cannot launch a browser gate.
@playwright/testis absent,playwright-coreis 1.62.0 against a 1.62.1 lock pin, and/opt/pw-browserscarries onlychromium-1194builds while the pin requestschromium_headless_shell-1234— the same#255limitation, reproduced directly here. Chromium proof is delegated to CI's Production UI job, following the precedent of #1620/#1621/#1622. A synthetic-DOM measurement was used during diagnosis to confirm the CSS mechanism (560px → 369px) — that is a physics check of the layout engine, not gate evidence, and is not offered as such.Risk and rollout
SignedImage's newexpandLabelis opt-in, so the DOM of every existing caller is unchanged. The sharedChunkImagetype gains two optional fields.feat(document-viewer)alone restores the previous viewer while keeping the clipping fix; revertingfix(document-viewer)alone restores the overflow.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy) — untouched; no Supabase config, migration, or env value is in this diff./api/images/{id}/signed-url,next/imagestaysunoptimizedso bearer URLs never enter the/_next/imagecache, theIntersectionObserverdeferral is intact, and the lightbox's URL mode still refuses to seed from the module LRU so an auth-identity change cannot revive a prior identity's image (pinned by the existing tests inimage-lightbox-url-mode.dom.test.tsx, which still pass).isDemoMode()gating is unchanged.rowsTruncated,cropCompleteness, andstructuredExtractionConfidencebranches are unchanged, and the source image is still pixel-faithful: nofilter,invert, orcolor-schemeis applied to source pixels at any zoom or rotation.ragRankingPatterns.Notes
Follow-ups deliberately left out of scope: a repo-wide sweep for the same implicit-grid-track hazard outside the document viewer (real, but needs its own false-positive triage), and tuning
LEGIBILITY_FLOOR/LEGIBLE_OPEN_MAX(0.35 / 3×) against real crops on a physical device — the current values are principled but device QA has not happened, since this container has no browser.Generated by Claude Code