fix(sidebar): correct task reorder drop position after auto-scroll - #3170
Open
claude[bot] wants to merge 1 commit into
Open
fix(sidebar): correct task reorder drop position after auto-scroll#3170claude[bot] wants to merge 1 commit into
claude[bot] wants to merge 1 commit into
Conversation
The sidebar drag handlers tracked the pointer as `initialPointerY + delta.y`, but dnd-kit's `delta` is scroll-adjusted: it adds however far the list's scroll container has moved since the drag began. The above/below decision then compared that scroll-offset Y against `over.rect`, whose getters read in viewport space. Once the list had auto-scrolled by half a row or more, the insertion indicator flipped to the wrong side and the drop landed one slot off. Record the list's scrollTop at drag start and subtract the scroll delta on each move so the tracked pointer stays in viewport space, matching the rect it is compared against. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AymqGXQwXJbLX2JmA5FPFh
Contributor
Greptile SummaryThis PR corrects sidebar reorder positioning during auto-scroll by keeping the tracked drag pointer in the same viewport coordinate space as the hovered row rectangle.
Confidence Score: 5/5The PR appears safe to merge; the coordinate correction is localized and consistently feeds both visual and final reorder decisions. No actionable failure remains: the corrected pointer uses the list鈥檚 actual scroll container, stays in viewport coordinates, and is shared by the insertion indicator and drop calculation.
|
| Filename | Overview |
|---|---|
| apps/emdash-desktop/src/core/features/workbench/browser/sidebar/sidebar-virtual-list.tsx | Captures the initial sidebar scroll offset and removes accumulated list scrolling from drag pointer calculations so indicator and drop placement remain aligned. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Drag starts] --> B[Capture pointer Y and scrollTop]
B --> C[Drag move supplies adjusted delta]
C --> D[Subtract list scroll delta]
D --> E[Viewport-space pointer Y]
E --> F[Compare with hovered row midpoint]
F --> G[Render insertion indicator]
F --> H[Choose final insertion index]
Reviews (1): Last reviewed commit: "fix(sidebar): correct task reorder drop ..." | Re-trigger Greptile
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
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.
Requested by Raban von Spiegel 路 Slack thread
Before: Dragging a task (or project) in the left sidebar far enough for the list to auto-scroll made the insertion indicator jump to the wrong side of the hovered row, and the drop landed one slot away from where the indicator had been. Short drags that did not scroll the list behaved correctly, which is why the reordering felt "glitchy" rather than broken.
After: The indicator stays on the side of the row the pointer is actually on, and the drop lands where the indicator shows, regardless of how far the list scrolled during the drag.
How: The drag handlers tracked the pointer as
initialPointerY + delta.y, but dnd-kit'sdeltais scroll-adjusted (pointer travel plus the scroll container's movement since drag start), whileover.rectreads in viewport space. Once the list scrolled by half a row the above/below comparison flipped. The fix records the list'sscrollTopat drag start and subtracts the scroll delta on each move, so the tracked pointer stays in viewport space to match the rect it is compared against. Both the insertion indicator and the drop decision read the same corrected value.Related issues
Reported on Discord ("reordering tasks is a bit glitchy").
Testing
oxfmt --checkon the changed filepnpm run lintinapps/emdash-desktopplustooling/oxlint/scripts/check-allowlists.mjsnx run @emdash/emdash-desktop:typecheck(browser, node, and release tsconfigs)vitest run --project node src/core/features/workbench/browser/sidebar(15 tests pass)No new unit test: the corrected math lives inline in the component's drag handlers and there is no existing test harness for the virtualized sidebar's DnD wiring.
Checklist
messages and, when possible, the PR title
馃 Generated with Claude Code
https://claude.ai/code/session_01AymqGXQwXJbLX2JmA5FPFh
Generated by Claude Code