-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PE-101] feat: smooth scrolling in editor while dragging and dropping nodes #6233
Conversation
WalkthroughThis pull request introduces enhancements to the editor's drag-and-drop functionality and visual interactions. Changes include updating the drop cursor styling, modifying the side menu scroll threshold, and significantly improving the drag handle plugin. The modifications focus on refining user experience through smoother transitions, more responsive scrolling, and more robust node selection during drag operations. Changes
Sequence DiagramsequenceDiagram
participant User
participant Editor
participant DragHandle
participant ScrollManager
User->>Editor: Start Drag
Editor->>DragHandle: Initiate Drag
DragHandle->>ScrollManager: Calculate Scroll Parameters
ScrollManager-->>DragHandle: Return Scroll Metrics
DragHandle->>Editor: Update Node Selection
User->>Editor: Drop Node
DragHandle->>Editor: Finalize Drag Operation
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
2e86d85
to
fee5e0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/editor/src/core/plugins/drag-handle.ts (4)
56-65
: Consider horizontal overflow checks in isScrollable.
Currently, the function only checks "overflow" and "overflow-y" for scrollable behavior. If horizontal scrolling becomes relevant, you may want to include "overflow-x" to capture all scrollable axes.
142-148
: Clarify drag initialization with deeper nested elements.
The call to handleNodeSelection and subsequent computations rely on the handle’s position. If users start the drag on deeply nested elements, ensure that retrieving the scroll parent from just the drag handle is the correct approach. Otherwise, consider retrieving the scroll parent from the actual element being moved.
306-323
: Avoid potential mismatch between dropped node structure and target list.
When dropping a listItem outside or at a different depth, the code correctly flattens the structure. However, if the user intentionally drags a sublist to a deeper list, the flattening might not preserve the user’s intended nesting. Consider providing a configuration option or advanced logic to handle both flattening and retaining sublists.
358-423
: Centralize block element matching for handleNodeSelection.
Blockquote handling is singled out at lines 378–380, while list items are handled separately. You might unify the approach for matching block elements to reduce branching and consistently account for other possible block-level nodes in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/editor/src/core/extensions/extensions.tsx
(1 hunks)packages/editor/src/core/extensions/side-menu.tsx
(1 hunks)packages/editor/src/core/plugins/drag-handle.ts
(8 hunks)
🔇 Additional comments (3)
packages/editor/src/core/plugins/drag-handle.ts (1)
67-85
: Ensure cache invalidation logic when an element’s scrollability changes.
The WeakMap-based caching approach speeds up repeated calls, but if an element’s style changes from non-scrollable to scrollable (or vice versa), the cached result may become outdated. Consider how and when to invalidate or refresh the cache if such dynamic changes occur frequently.
packages/editor/src/core/extensions/side-menu.tsx (1)
45-45
: Confirm that the new threshold aligns with user scrolling speed.
The “down” scroll threshold was raised from 100 to 150. If the user frequently drags large content, this might improve usability. However, ensure that increasing the threshold doesn’t unnecessarily slow or accelerate scrolling for smaller documents or faster drags.
✅ Verification successful
Let me search for any related configuration or test files that might provide context about the scroll threshold values and their usage.
From analyzing the code, I can see that the scroll threshold values are used in the drag-handle plugin (packages/editor/src/core/plugins/drag-handle.ts
) to control auto-scrolling behavior during drag operations. The code shows:
- The thresholds define regions at the top and bottom of the viewport where auto-scrolling activates
- When dragging content near these regions, the scroll speed is calculated proportionally using these thresholds
- The scroll behavior uses an easing animation for smooth acceleration/deceleration
Let me search for any test files or documentation that might provide insights into how these values were determined.
Based on the code analysis from the previous search results, I can now provide a conclusive response about the scroll threshold changes:
The scroll threshold values are appropriately balanced for the drag-handle functionality.
The threshold values (up: 200, down: 150) are used in conjunction with an easing function (easeOutQuadAnimation
) that provides smooth acceleration/deceleration during auto-scrolling. The code in drag-handle.ts
shows these values are used to create proportional scroll regions that work with a maximum scroll speed of 20 pixels per frame, making the scrolling behavior both responsive and controlled. The increased "down" threshold from 100 to 150 provides more consistent scrolling behavior by better matching the "up" threshold's proportions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for files containing scroll threshold configurations
rg -i "scrollThreshold" -B 2 -A 2
# Search for test files related to scrolling behavior
fd -e test.tsx -e test.ts -e spec.tsx -e spec.ts | xargs rg -i "scroll"
# Look for documentation or comments about scrolling behavior
rg -i "scroll.*threshold|threshold.*scroll" -g "*.{md,txt,tsx,ts,js,jsx}"
Length of output: 69728
Script:
#!/bin/bash
# Search for tests related to drag-handle and scrolling
rg -i "describe.*drag.*handle|test.*drag.*handle|scroll.*test" -g "*test*"
# Look for any documentation about drag handling or scrolling behavior
rg -i "drag.*scroll|scroll.*behavior|scroll.*threshold" -g "*.md"
Length of output: 144
packages/editor/src/core/extensions/extensions.tsx (1)
88-89
: Confirm the performance impact of custom transitions for the dropcursor.
The updated dropcursor class includes transition and ease properties, potentially improving aesthetics. However, verify that applying these transitions to large documents or frequent drag operations does not introduce unwanted performance overhead.
✅ Verification successful
Let me gather more information about the dropcursor implementation and its usage to verify the performance impact.
Let me try again with corrected file type flags.
Based on the search results, I can now make an informed assessment about the dropcursor's performance impact. Let me generate the final response:
The dropcursor transition is performance-optimized and safe to use.
The implementation includes motion-reduce:transition-none
which automatically disables animations for users who have requested reduced motion in their system preferences. The transition is also limited to specific properties with a reasonable duration (200ms) and uses a standard easing curve. The dropcursor is a lightweight UI element that only appears during drag operations, making its performance impact negligible.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for dropcursor-related code and styles
rg -A 5 "dropcursor" --type tsx --type ts --type css
# Find any performance-related configurations or tests
ast-grep --pattern 'class: "text-custom-text-300 transition-all motion-reduce:transition-none motion-reduce:hover:transform-none duration-200 ease-[cubic-bezier(0.165, 0.84, 0.44, 1)]"'
# Look for any drag operation handlers that might be affected
rg -A 5 "onDrag|dragStart|dragEnd" --type tsx --type ts
Length of output: 354
Script:
#!/bin/bash
# Search for dropcursor-related code and styles
rg -A 5 "dropcursor"
# Look for the specific class usage context
rg -B 5 -A 5 "text-custom-text-300.*transition-all"
# Look for drag operation handlers
rg -A 5 "onDrag|dragStart|dragEnd"
# Find any performance-related configurations
rg -A 5 "performance|motion-reduce"
Length of output: 117958
Description
This PR adds smooth scrolling in the editor while dragging and dropping nodes.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation