Show row resize guideline and keep scroll on page 2#172
Conversation
Two user-reported bugs interacted on tables that live on page 2 of a multi-page document. Row resize drag guideline was painted off-screen because `editor.ts` draws the guideline AFTER `docCanvas.render()` has restored the canvas context to identity, but `dragGuideline.x/y` are in unscaled document coordinates. With a `scrollTop ≈ 1118`, the horizontal line landed at canvas pixel y ≈ 1605, well below the visible area. Vertical (column) guidelines accidentally worked because horizontal scroll is normally zero. Convert the coords with `(coord - scroll) * scaleFactor` so both axes track the cursor under any scroll or zoom-to-fit. Resize on page 2 with the caret on page 1 snapped the viewport back to page 1 because `applyBorderDrag` requested a full render through the path that always sets `needsScrollIntoView = true`. The user did not move the caret, so the next paint scrolled it into view, undoing their scroll. Add a `requestRenderNoCursorScroll` callback for layout-only changes that don't move the caret and have the resize path use it. While there, extract `getTableOriginYForPageLine` in `pagination.ts` so renderer (`DocCanvas`) and hit-test (`TextEditor.resolveTableFromMouse`) share the same virtual-origin formula. The resolver had been dropping the `rowSplitOffset` term that the renderer applied — a latent bug that would offset the row-border hit area by the page-1 fragment height when a row split lands as the first PageLine on the next page. The helper bounds-checks `lineIndex` so future split-rendering regressions fail loudly instead of silently producing NaN coordinates. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Without this entry, knip used `main`/`module` from `packages/docs/package.json` (which point at `dist/`) as its only entry roots, so once the docs package was built `verify:entropy` flagged `packages/docs/scripts/verify-ime-browser.mjs` as unused dead code and broke the pre-push harness. Add a workspace block matching the sheets pattern so knip scans `src/`, `test/`, and `scripts/*.mjs` as legitimate entries. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✨ 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. Review rate limit: 0/1 reviews remaining, refill in 49 minutes and 30 seconds.Comment |
Verification: verify:selfResult: ✅ PASS in 129.3s
Verification: verify:integrationResult: ✅ PASS |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
1 participant
Summary
getTableOriginYForPageLine, so the row-border hit area can no longer drift away from the painted border on a split-row continuation page.Root causes
Guideline off-screen (
editor.ts): The drag-guideline draw runs afterdocCanvas.render()has restored the canvas context to identity, butdragGuideline.x/yare in unscaled document coordinates. Drawing at canvas pixel y = absolute_doc_y placed the horizontal line below the visible viewport when scrolled (e.g.scrollTop ≈ 1118→ line drawn at y = 1605, canvas height ~787). The vertical (column) guideline accidentally worked becausescrollLeftis normally 0. Convert to canvas pixels with(coord - scroll) * scaleFactorfor both axes — also correct under mobile zoom-to-fit.Viewport snap on resize (
editor.ts+text-editor.ts):applyBorderDragrequested a render through the path that always setsneedsScrollIntoView = true, so the next paint scrolled the cursor (still on page 1) into view. AddedrequestRenderNoCursorScrollfor layout-only changes that don't move the caret.Latent split-row hit-test bug (
pagination.ts+doc-canvas.ts+text-editor.ts): The renderer usedpageY + pl.y - rowYOffsets[lineIndex] - rowSplitOffsetto position a continuation fragment, but the resolver usedbandTop - rowYOffsets[firstPl.lineIndex](norowSplitOffset). On a page that starts with a row-split continuation, the resolver's virtual table origin diverged from the renderer's by the page-1 fragment height, so row-border hit-testing missed every row by that offset. ExtractedgetTableOriginYForPageLine(pageY, pl, rowYOffsets)so both paths share the same formula. The helper bounds-checkslineIndexso any future split-rendering regression fails loudly instead of silently producing NaN coordinates.Pre-push harness fix (
knip.json): With docs registered in knip workspaces, knip usedmain/modulefrompackages/docs/package.json(which point atdist/) as its only entry roots — so once docs was built, knip flaggedscripts/verify-ime-browser.mjsas unused dead code and brokeverify:entropyin the pre-push harness. Add apackages/docsworkspace block matching the sheets pattern.Test plan
pnpm verify:fast(684/684 tests pass)pnpm verify:entropypasses after building docsscrollTop=1118(was snapping back to 0)table-origin-y.test.tsregression: helper returns the same virtual origin for the split fragment and follow-up rows on the same page; reverting therowSplitOffsetterm in the helper makes this test fail.🤖 Generated with Claude Code