feat(panes): add drag-and-drop pane rearrangement#3090
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds drag-and-drop support to the panes package using react-dnd, including a DnD provider, drag sources on pane headers, drop targets on panes and tab items, a visual DropZoneOverlay, and a new store operation Changes
Sequence DiagramsequenceDiagram
actor User
participant PaneHeader as PaneHeader (Drag Source)
participant Pane as Pane (Drop Target)
participant TabItem as TabItem
participant Store as Store
participant Workspace as Workspace
User->>PaneHeader: drag start (pane)
PaneHeader->>Pane: dragging over
Pane->>Pane: compute drop position (left/right/top/bottom)
Pane->>Pane: show DropZoneOverlay
alt hover over inactive tab
User->>TabItem: hover
TabItem->>Workspace: select tab
end
User->>Pane: drop
Pane->>Store: movePaneToSplit({sourcePaneId, targetPaneId, position})
Store->>Store: update layouts, panes, activePaneId, activeTabId
Store->>Workspace: trigger re-render with updated layout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
saddlepaddle
force-pushed
the
saddlepaddle/panes-dnd
branch
from
d88f2e4 to
9247503
Compare
saddlepaddle
force-pushed
the
saddlepaddle/panes-dnd
branch
from
9247503 to
eac6324
Compare
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Add DnD support using react-dnd so users can drag panes by their headers and drop them onto other panes to split the layout. Each pane handles its own drop logic via useDrop's drop() callback — no shared state needed. - PaneHeader: useDrag makes headers draggable - Pane: useDrop with hover() for zone detection + drop() calls movePaneToSplit - TabItem: useDrop with hover() switches tabs during drag - DropZoneOverlay: animated split-zone indicator (top/right/bottom/left) - Store: movePaneToSplit action for same-tab and cross-tab moves
saddlepaddle
force-pushed
the
saddlepaddle/panes-dnd
branch
from
eac6324 to
3dc5a0b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/panes/src/react/components/Workspace/components/TabBar/components/TabItem/TabItem.tsx (1)
75-82: Consider simplifying the ref callback.The
MutableRefObjectcast on line 78 is a workaround for TypeScript's strict ref typing. This is acceptable, but you could simplify by just usingconnectDropdirectly ifnodeRefisn't used elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/panes/src/react/components/Workspace/components/TabBar/components/TabItem/TabItem.tsx` around lines 75 - 82, The ref callback is over-complicated: simplify by removing the MutableRefObject cast and either stop using nodeRef if it's unused or assign to it safely; specifically, if nodeRef.current is not referenced elsewhere, delete nodeRef and replace setDropRef with const setDropRef = connectDrop so the drop connector is used directly; otherwise, setDropRef should be implemented without the cast by doing if (nodeRef) nodeRef.current = node; connectDrop(node) (keep the existing connectDrop usage) and remove the (nodeRef as React.MutableRefObject...) cast.packages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsx (1)
41-48: UnusednodeRefvariable.The
nodeRefis populated but never read after assignment. If there's no planned use for it, consider removing it to simplify the ref callback.Simplified ref callback
- const nodeRef = useRef<HTMLDivElement>(null); - const setRef = useCallback( - (node: HTMLDivElement | null) => { - (nodeRef as React.MutableRefObject<HTMLDivElement | null>).current = node; - connectDrag(node); - }, - [connectDrag], - ); + const setRef = useCallback( + (node: HTMLDivElement | null) => { + connectDrag(node); + }, + [connectDrag], + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsx` around lines 41 - 48, Remove the unused nodeRef and simplify the ref callback: delete the nodeRef useRef declaration and change the setRef callback (currently named setRef) to only accept (node: HTMLDivElement | null) and call connectDrag(node); keep connectDrag in the dependency array of useCallback so the ref updates correctly. This preserves the ref behavior without storing an unused ref variable.packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx (1)
146-176: Unused dependency in useDrop.
tab.idis included in the dependency array but is not referenced in the hook callback. Consider removing it to accurately reflect the actual dependencies.Remove unused dependency
- [pane.id, tab.id, store], + [pane.id, store],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx` around lines 146 - 176, The useDrop hook's dependency array includes tab.id which is not referenced inside the hook callback; update the dependency array for the useDrop call (the one creating [{ isOver, canDrop }, connectDrop]) to remove tab.id and keep only the actual dependencies used (e.g., pane.id and store) so that dropRef, dropPositionRef, setDropPosition and the movePaneToSplit call behave correctly and the hook doesn't re-run unnecessarily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/panes/package.json`:
- Around line 20-21: Update the package.json dependencies by removing
"react-dnd" and "react-dnd-html5-backend" and replacing them with a
React-19-compatible library (for example "@hello-pangea/dnd" or "dnd-kit"); then
update all references/imports in the codebase that currently import from
"react-dnd" or "react-dnd-html5-backend" to the chosen library's import paths
and adapt any API differences in files using DragDropContext, useDrag/useDrop,
or HTML5Backend equivalents to the chosen library's APIs so runtime errors like
findDOMNode and ref incompatibilities are resolved.
In
`@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx`:
- Around line 187-191: The current render block mutates dropPositionRef.current
and calls setDropPosition(null) when !isOver, which performs state updates
during render; move this cleanup into a useEffect that runs after render: create
a useEffect that depends on isOver (and optionally dropPositionRef and
dropPosition) and inside it check if !isOver && dropPositionRef.current !==
null, then set dropPositionRef.current = null and call setDropPosition(null) if
dropPosition !== null; update the logic around the isOver / dropPositionRef /
dropPosition checks accordingly and remove the setDropPosition call from the
render path in Pane (look for isOver, dropPositionRef, dropPosition,
setDropPosition).
---
Nitpick comments:
In
`@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsx`:
- Around line 41-48: Remove the unused nodeRef and simplify the ref callback:
delete the nodeRef useRef declaration and change the setRef callback (currently
named setRef) to only accept (node: HTMLDivElement | null) and call
connectDrag(node); keep connectDrag in the dependency array of useCallback so
the ref updates correctly. This preserves the ref behavior without storing an
unused ref variable.
In
`@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx`:
- Around line 146-176: The useDrop hook's dependency array includes tab.id which
is not referenced inside the hook callback; update the dependency array for the
useDrop call (the one creating [{ isOver, canDrop }, connectDrop]) to remove
tab.id and keep only the actual dependencies used (e.g., pane.id and store) so
that dropRef, dropPositionRef, setDropPosition and the movePaneToSplit call
behave correctly and the hook doesn't re-run unnecessarily.
In
`@packages/panes/src/react/components/Workspace/components/TabBar/components/TabItem/TabItem.tsx`:
- Around line 75-82: The ref callback is over-complicated: simplify by removing
the MutableRefObject cast and either stop using nodeRef if it's unused or assign
to it safely; specifically, if nodeRef.current is not referenced elsewhere,
delete nodeRef and replace setDropRef with const setDropRef = connectDrop so the
drop connector is used directly; otherwise, setDropRef should be implemented
without the cast by doing if (nodeRef) nodeRef.current = node; connectDrop(node)
(keep the existing connectDrop usage) and remove the (nodeRef as
React.MutableRefObject...) cast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: adc3a033-8b4e-41ff-86db-535fd000b14b
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
packages/panes/package.jsonpackages/panes/src/core/store/store.test.tspackages/panes/src/core/store/store.tspackages/panes/src/react/components/Workspace/Workspace.tsxpackages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsxpackages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/DropZoneOverlay/DropZoneOverlay.tsxpackages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/DropZoneOverlay/index.tspackages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsxpackages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/components/DefaultHeaderContent/DefaultHeaderContent.tsxpackages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/components/DefaultHeaderContent/index.tspackages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/index.tspackages/panes/src/react/components/Workspace/components/TabBar/components/TabItem/TabItem.tsx
Comment on lines
+20
to
+21
| "react-dnd": "^16.0.1", | ||
| "react-dnd-html5-backend": "^16.0.1", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is react-dnd version 16 compatible with React 19?
💡 Result:
No, react-dnd version 16 is not fully compatible with React 19. While some users report successful upgrades where it functions (per DepFixer verification), the official GitHub issue #3655 documents compatibility problems including runtime errors like "TypeError: _reactDom.default.findDOMNode is not a function" and TypeScript ref typing issues due to React 19 removing findDOMNode and changing ref typings. The library's last release (v16.0.1) was ~4 years ago in 2022, before React 19, with no official React 19 support or updated peer dependencies. Workarounds like custom drag ref hooks exist, but official compatibility is lacking. Consider alternatives like @hello-pangea/dnd or dnd-kit which have added React 19 support.
Citations:
- 1: https://depfixer.com/compatibility/react-19-react-dnd-16
- 2: Support for React 19 react-dnd/react-dnd#3655
- 3: https://github.com/react-dnd/react-dnd/
- 4: https://www.npmjs.com/package/react-dnd
- 5: https://www.npmjs.com/package/react-dnd?activeTab=versions
- 6: https://github.com/react-dnd/react-dnd
- 7: https://ithile.com/micro-tools/npm-version-checker/react-dnd/
Replace react-dnd@^16.0.1 with a React 19 compatible alternative.
react-dnd v16.0.1 (released 2022) does not support React 19. This causes runtime errors including "TypeError: _reactDom.default.findDOMNode is not a function" due to React 19 removing the findDOMNode API, and TypeScript ref typing incompatibilities. Consider using @hello-pangea/dnd or dnd-kit, both of which have official React 19 support.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/panes/package.json` around lines 20 - 21, Update the package.json
dependencies by removing "react-dnd" and "react-dnd-html5-backend" and replacing
them with a React-19-compatible library (for example "@hello-pangea/dnd" or
"dnd-kit"); then update all references/imports in the codebase that currently
import from "react-dnd" or "react-dnd-html5-backend" to the chosen library's
import paths and adapt any API differences in files using DragDropContext,
useDrag/useDrop, or HTML5Backend equivalents to the chosen library's APIs so
runtime errors like findDOMNode and ref incompatibilities are resolved.
Comment on lines
+187
to
+191
| // Clear drop position when not hovering | ||
| if (!isOver && dropPositionRef.current !== null) { | ||
| dropPositionRef.current = null; | ||
| if (dropPosition !== null) setDropPosition(null); | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid setState during render.
Calling setDropPosition(null) during render can lead to unexpected behavior and violates React's render purity principle. Move this cleanup logic to a useEffect.
Proposed fix using useEffect
+ // Clear drop position when not hovering
+ useEffect(() => {
+ if (!isOver) {
+ dropPositionRef.current = null;
+ setDropPosition(null);
+ }
+ }, [isOver]);
- // Clear drop position when not hovering
- if (!isOver && dropPositionRef.current !== null) {
- dropPositionRef.current = null;
- if (dropPosition !== null) setDropPosition(null);
- }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx`
around lines 187 - 191, The current render block mutates dropPositionRef.current
and calls setDropPosition(null) when !isOver, which performs state updates
during render; move this cleanup into a useEffect that runs after render: create
a useEffect that depends on isOver (and optionally dropPositionRef and
dropPosition) and inside it check if !isOver && dropPositionRef.current !==
null, then set dropPositionRef.current = null and call setDropPosition(null) if
dropPosition !== null; update the logic around the isOver / dropPositionRef /
dropPosition checks accordingly and remove the setDropPosition call from the
render path in Pane (look for isOver, dropPositionRef, dropPosition,
setDropPosition).
Comment on lines
+62
to
+73
| const [{ isOver }, connectDrop] = useDrop( | ||
| () => ({ | ||
| accept: PANE_DRAG_TYPE, | ||
| hover: () => { | ||
| if (!isActive) onSelect(); | ||
| }, | ||
| collect: (monitor) => ({ | ||
| isOver: monitor.isOver(), | ||
| }), | ||
| }), | ||
| [isActive, onSelect], | ||
| ); |
Contributor
There was a problem hiding this comment.
hover() fires repeatedly during drag, causing excessive onSelect calls.
The hover callback fires on every mouse move while dragging over this tab. Since onSelect() calls store.getState().setActiveTab(tabId) (per context snippet), this triggers continuous state updates. Consider debouncing or tracking whether the tab switch already occurred during this drag session.
🛠️ Proposed fix using a ref to prevent repeated calls
+ const hasActivatedRef = useRef(false);
+
const [{ isOver }, connectDrop] = useDrop(
() => ({
accept: PANE_DRAG_TYPE,
hover: () => {
- if (!isActive) onSelect();
+ if (!isActive && !hasActivatedRef.current) {
+ hasActivatedRef.current = true;
+ onSelect();
+ }
},
collect: (monitor) => ({
isOver: monitor.isOver(),
}),
}),
- [isActive, onSelect],
+ [isActive, onSelect, hasActivatedRef],
);
+
+ // Reset when drag leaves or ends
+ if (!isOver) {
+ hasActivatedRef.current = false;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const [{ isOver }, connectDrop] = useDrop( | |
| () => ({ | |
| accept: PANE_DRAG_TYPE, | |
| hover: () => { | |
| if (!isActive) onSelect(); | |
| }, | |
| collect: (monitor) => ({ | |
| isOver: monitor.isOver(), | |
| }), | |
| }), | |
| [isActive, onSelect], | |
| ); | |
| const [{ isOver }, connectDrop] = useDrop( | |
| () => ({ | |
| accept: PANE_DRAG_TYPE, | |
| hover: () => { | |
| if (!isActive) onSelect(); | |
| }, | |
| collect: (monitor) => ({ | |
| isOver: monitor.isOver(), | |
| }), | |
| }), | |
| [isActive, onSelect], | |
| ); | |
| const hasActivatedRef = useRef(false); | |
| const [{ isOver }, connectDrop] = useDrop( | |
| () => ({ | |
| accept: PANE_DRAG_TYPE, | |
| hover: () => { | |
| if (!isActive && !hasActivatedRef.current) { | |
| hasActivatedRef.current = true; | |
| onSelect(); | |
| } | |
| }, | |
| collect: (monitor) => ({ | |
| isOver: monitor.isOver(), | |
| }), | |
| }), | |
| [isActive, onSelect], | |
| ); | |
| // Reset when drag leaves or ends | |
| if (!isOver) { | |
| hasActivatedRef.current = false; | |
| } |
saddlepaddle
commented
Apr 1, 2026
| import type { SplitPosition } from "../../../../../../../../../types"; | ||
|
|
||
| interface DropZoneOverlayProps { | ||
| position: SplitPosition | null; |
10 tasks
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.
Summary
react-dndso users can drag panes by their headers and drop onto other panes to rearrange the layoutuseDrop'sdrop()callback — no shared state neededmovePaneToSplitstore action handles same-tab and cross-tab pane moves atomicallyChanges
useDragmakes headers draggableuseDropwithhover()for zone detection +drop()callsmovePaneToSplitdirectlyuseDropwithhover()switches tabs during dragDndProviderwrapper (replaced@dnd-kitDndContext)movePaneToSplitaction + tests@dnd-kit/core, addedreact-dnd+react-dnd-html5-backendPaneDragItemtype (drag item is just{ paneId: string })Test plan
Summary by cubic
Add drag-and-drop pane rearrangement with
react-dnd, letting users drag a pane header and drop onto another pane to split or reorder. Works across tabs with animated zone hints and no shared drag state.New Features
useDrag; header actions don’t start a drag.useDropand show a 4-zone overlay (top/right/bottom/left) while hovering.movePaneToSplitstore action for same- and cross-tab moves; removes empty source tabs, focuses the dropped pane/tab, and ignores self-drops.DndProviderwithHTML5Backend.Dependencies
react-dndandreact-dnd-html5-backend.Written for commit fe6b528. Summary will update on new commits.
Summary by CodeRabbit