Repository navigation
fix: the drag now tracks the cursor - #28
Merged
Merged
Conversation
Reported as: it should go where you click, but the handle drifts further out the further
out you drag. That growth with distance is the diagnostic — a constant offset and a
proportional one look identical at the centre and nothing alike at the rim.
THE CONVERSION WAS WRONG TWICE, in the two ways that compound:
· the pitch was `box.width / cols`, which is not the pitch. The field's box is `size` px
and includes one dot of overhang at each end, so the centre-to-centre distance is
`layout().pitchX`. The hand-rolled figure was about 3% out before anything else.
· and the centre-to-cell step multiplied by the half-width a SECOND time. `cx` appears
once to name the middle cell and again as a scale factor, so the whole thing was four
times too sensitive.
Measured on a 9×9 at 260px, the error against the true cell: 0 at the centre, 6 cells at
col 6, and 12 cells — clamped — at the rim. That is the reported behaviour exactly: the
handle reaches the edge while the cursor is halfway out, and dragging further does nothing,
because there is nothing past the clamp.
BOTH NUMBERS NOW COME FROM THE ENGINE'S `layout()`. That is the library's own rule — `field()`
is the only place geometry is computed — applied to a control that has to agree with the
thing it is pointing at. And `cellCentre` is now SHARED: the handle is drawn where the
hit-test will look, so they cannot drift apart, which is a stronger guarantee than both being
correct.
THE HANDLE IS POSITIONED IN PX, not as a percentage of the box. A cell's centre is half a dot
in from the edge, so `col / (cols - 1)` as a percentage is wrong by half a dot — 16px here at
the rim — and wrong by a different amount everywhere else. The overlay also moved inside a
wrapper sized by the field, so `inset: 0` IS the host box.
THE REGRESSION WALKS ALL 81 CELLS and requires the identity, plus a test that the error does
not grow outward. A hand-picked sample of cells passes the old code: it is only wrong past
the middle, and every sample anyone would pick by hand is in the middle.
275 tests (was 269), oxlint 0, registry ok, both changelog ledgers.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (2)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe traversal demo now uses geometry from ChangesTraversal geometry
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The drag geometry appears aligned with the rendered matrix, with no identified issue requiring a fix before merge. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to The inspected change remains a bounded browser-demo interaction. Pointer movement updates local rendering state, existing bounds and cleanup remain intact, and no new privileged operation or material security risk was identified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Reported as "it should go where you click, but the handle drifts further out the further out
you drag". That growth with distance is the diagnostic — a constant offset and a
proportional one look identical at the centre and nothing alike at the rim.
Change outline
The conversion was wrong twice, in the two ways that compound:
box.width / colsis not the pitch. The box issizepx and includes one dot of overhang at each end, so centre-to-centre islayout().pitchX— the hand-rolled figure was ~3% out before anything elsecxappears once to name the middle cell and again as a scale factor, so the whole thing was four times too sensitiveMeasured on a 9×9 at 260px — error against the true cell:
That is the reported behaviour exactly: the handle reaches the edge while the cursor is
halfway out, and dragging further does nothing, because there is nothing past the clamp.
Both numbers now come from the engine's
layout()— the library's own rule thatfield()is the only place geometry is computed — applied to a control that has to agree with the thing
it points at. And
cellCentreis now shared: the handle is drawn where the hit-test willlook, which is stronger than both being independently correct.
The handle is positioned in px, not as a percentage. A cell's centre is half a dot in
from the edge, so
col / (cols - 1)as a percentage is wrong by half a dot — 16px here — andby a different amount everywhere else.
Special things to note
does not grow outward. A hand-picked sample of cells passes the old code: it is only
wrong past the middle, and every sample anyone would pick by hand is in the middle.
Input.dispatchMouseEventalong a line andcomparing the handle's measured centre to the cursor at each step: ≤0.3px throughout.
🤖 Generated with Claude Code
Summary by CodeRabbit