Repository navigation
#246 — bound fmt-tooltip content + viewport-relative height clamp - #286
Conversation
…ght clamp The #fmt-tooltip is pointer-events:none and hides on mouseleave/blur, so content laid below its fixed 22rem max-height fold (overflow:auto) could never be scrolled to or read — silently unreachable for every input modality (observed on the playground dim_payers panel, ~221px below the fold). The fix bounds the content instead of folding it: - interaction.js: showFmtTip trims the reconstruction to a viewport-derived line budget (20px/line + 96px chrome over-approximations, 20-line ceiling, 1-line floor) and closes with a truthful elision row counting exactly what was trimmed — the tip is a glance surface; the grid below the badge carries the full data. - report.css: the 22rem cap becomes viewport-relative (calc(100vh - 16px), the #240 defect-D width-cap pattern) so positionTipNear's post-flip top clamp (landed in PR #244) always fully contains the box on the height axis at any viewport height; overflow:hidden because a scrollbar nothing can operate is a lie. - headless guards (RED pre-fix): no-fold + truthful-elision + viewport-containment pins at the default viewport, the full pathological-viewport-height containment pin promised by the PR #244 guard comment (the platform floors tiny window requests, so the pins measure against the live clientHeight), and the existing ov-tip right-edge containment guard extended to the height axis. Goldens regenerated per the ci.yml example-report-check recipes (the three report examples inline report.css + interaction.js; the explore pages don't carry the fmt tip and are unchanged). Closes #246 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you: Chapters generated by Stage for commit a9668a4 on Jun 12, 2026 8:35am UTC. |
📄 Rendered report previewAll golden examples regenerated cleanly. 🟡 Golden examplesCommitted to
🐶 Live dogfood previewThis PR doesn't touch ▶ Open ↗ opens the report in your browser in one click — The Pages preview may take ~1 min to update after this comment Alternative: GitHub CLI# gh CLI >= 2.63 extracts into ./report-preview-playground/.
gh run download 27403539430 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.htmlPosted by |
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where format tooltips with long content could become unreachable or folded by bounding the displayed content to a viewport-derived line budget and appending a truthful elision row. The changes span templates/interaction.js for the line budget logic, templates/report.css for the updated viewport-relative height constraints, and tests/headless_toggle.rs for headless integration tests. Feedback on this PR highlights a potential accessibility issue in fmtTipLineBudget() where hardcoded pixel values (20px per line and 96px for chrome) do not scale with custom browser font sizes or zoom levels, which could cause the elision row and bottom lines to be clipped due to overflow: hidden. A dynamic scaling solution based on the computed root font size is recommended.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Warning Review limit reached
More reviews will be available in 45 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ort guard The Linux CI runner HONORS tiny window requests (the ~111px request yielded a 24px viewport) while macOS floors the outer window at ~375px (232px viewport) — so the macOS-derived 50..=352 premise band excluded the Linux reality and the premise assert panicked in CI. Request a pathological-but-sane ~300px window instead, landing both platforms in a comparable sub-352px band (macOS ≈232 via its floor; Linux ≈213-300 depending on chrome height), and widen the premise to 100..=352. Re-verified RED against the pre-fix templates at the new band (the no-fold pin fires; the old fixed 222px box folds 1178px of content). Test-only change — no golden impact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR #286 review (Gemini, HIGH): the 20px/line + 96px chrome constants in fmtTipLineBudget were layout estimates — correct for today's px-sized .sql-block lines, but wrong under a forced minimum font size (Chrome a11y), browser-zoom rounding, wrapped long lines (.code-line is white-space:normal), or any future rem migration — and overflow:hidden turned any mismatch into silent data loss. Both recommended directions, composed: - (a) measurement-driven trim: showFmtTip renders the ceiling-capped block (20 lines), then removes rendered lines until the LIVE laid-out content fits the viewport-capped box (scrollHeight <= clientHeight+1, the repo's 1px convention), measuring after a left reset (the #232 D18 lesson). The elision row participates in the measurement and its count stays truthful however the trim happened. - (b) report.css restores overflow:auto as the safety net: an exotic residual mismatch degrades to a visible scrollbar — a signal — never a silent clip. Headless: fmt_tip_long_fixture_is_bounded_not_folded gains a font-scale phase (inject .sql-block{font-size:24px}, re-show, re-assert no-fold + truthful elision + strictly fewer lines shown than at normal size — the non-vacuity proof). Both #246 tests pass; full toggle suite 81/81. Goldens regenerated per the ci.yml recipes; intended-only audit clean (all changed lines are this rewrite ×3 report goldens; explore pages unchanged). Chrome snapshot refreshed and diff-audited the same way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Overnight orchestration wrap-upMerged as Gate history worth reading: (1) the initial pathological-viewport guard failed in CI because Linux Chrome honors a tiny window request (24px viewport) while macOS floors it (~232px) — fixed with a same-band launch (900×300) + RED re-proof at the new band; (2) Gemini raised a HIGH-priority concern that the hardcoded line-budget + the new What shipped: the fmt-tooltip's below-fold content is now reachable-by-design — viewport-fitted glance surface with truthful elision; the full data remains in the grid below. 🤖 Generated with Claude Code |
Summary
Fixes the fmt-tooltip below-fold reachability defect and lands the pathological-viewport-height containment pin — the remaining scope of issue 246 after PR #244 landed the post-flip top clamp itself.
The
#fmt-tooltipispointer-events: noneand hides on mouseleave/blur, so content laid below its fixed22remmax-heightfold (overflow: auto) could never be scrolled to or read — silently unreachable for every input modality (observed on the playground dim_payers panel, ~221px below the fold).The fix (bounds the content instead of folding it)
templates/interaction.js—showFmtTiptrims the reconstruction to a viewport-derived line budget (20px/line + 96px chrome over-approximations; 20-line ceiling, 1-line floor) and closes with a truthful elision row (… +N more lines) counting exactly what was trimmed. The tip is a glance surface; the grid below the badge carries the full data (the issue's candidate directions 1+2, honoring the #145 — surface incremental-model unit-test semantics #146/#159 — strategy-correct incremental expect-semantics tooltip #161 tooltip contract).templates/report.css— the22remcap becomes viewport-relative (calc(100vh - 16px), the bug: founder review-2 render defects — tooltip overflow, given-header tooltips still absent (3rd report), covered-by dark contrast, overrides-badge clip, baseline banner dark contrast #240 defect-D width-cap pattern) sopositionTipNear's post-flip top clamp always fully contains the box on the height axis at any viewport height.overflow: hiddenbecause a scrollbar nothing can operate is a lie. The elision row usesvar(--text)onvar(--code-bg)(the code-line pairing the 8-theme AA matrix already covers) at fixed px (Sakura's 62.5% root makes 1rem = 10px).Headless guards (TDD RED-first)
Both new tests were run and failed against the unfixed CSS/JS before the fix landed:
fmt_tip_long_fixture_is_bounded_not_folded— 60-line reconstruction; RED pre-fix on the no-fold pin (scrollHeight1178 vsclientHeight~220). Pins: no fold, truthful elision accounting (shown + counted === 60, within the 20-line ceiling), head-of-block trim, full viewport containment. Relational pins, not exact counts — the headless default window maps to different viewport heights per platform.fmt_tip_contained_at_pathological_viewport_height— the full pathological-height pin promised by the PR #240 — review-2 render defects: tooltip overflow, given-header tooltips, covered-by contrast, overrides-badge clip, baseline banner #244 guard comment. RED pre-fix (probe: bubblescrollHeight1178 / box 222px at a 232px viewport — Chrome floors the ~111px window request on macOS, so every pin measures against the liveclientHeightwith a premise assert keeping the viewport below the old 352px fold). Pins: height-axis containment, no fold, truthful elision.ov_tip_long_value_contained_in_bubble_and_viewport_at_right_edgecontainment guard is extended to the height axis (r.bottom <= vh + 1), completing AC bullet 2.Goldens
Regenerated per the ci.yml
example-report-checkrecipes. Intended-only audit viagit diff --text -U0 -- examples/: 129 changed lines = 43 unique × the 3 report goldens (they inlinereport.css+interaction.js), every one from this change. The explore pages don't carry the fmt tip and are byte-identical.Gates (run directly, not via lefthook)
cargo fmt --all --check✓cargo clippy --all-targets --locked -- -D warnings✓cargo nextest run --locked— 1489 passed ✓ (chrome snapshot refreshed; diff audited = this change only)cargo test --test bdd --locked— 179 scenarios / 1174 steps ✓cargo test --test headless_zero_egress --locked -- --ignored— 11 passed ✓cargo test --test headless_toggle --locked -- --ignored— 81 passed ✓RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --locked(with and without--document-private-items) ✓cargo deny check✓Closes #246
🤖 Generated with Claude Code