fix(kanban): move drag-and-drop placeholder inside innerRef container#18939
Open
hnshah wants to merge 1 commit intotwentyhq:mainfrom
Open
fix(kanban): move drag-and-drop placeholder inside innerRef container#18939hnshah wants to merge 1 commit intotwentyhq:mainfrom
hnshah wants to merge 1 commit intotwentyhq:mainfrom
Conversation
Fixes twentyhq#18842 The Kanban drag-and-drop was only registering drops near the top of columns because the @hello-pangea/dnd placeholder was rendered outside the innerRef container, breaking the library's droppable area measurement. ## Root Cause The library requires `droppableProvided.placeholder` to be a direct child of the element that receives `droppableProvided.innerRef`. **Before (broken):** - `innerRef` was on `StyledColumnCardsContainer` (inside memo boundary) - `placeholder` was in parent `StyledColumn` (outside memo boundary) - Result: Library measured wrong droppable area, drops only worked at top **After (fixed):** - `innerRef` moved to `StyledColumn` (outside memo boundary) - `placeholder` stays in `StyledColumn` (same parent as innerRef) - Result: Both inside same DOM element, library measures correctly ## Changes **RecordBoardColumn.tsx:** - Added `ref={droppableProvided.innerRef}` to `StyledColumn` - Added `{...droppableProvided.droppableProps}` to `StyledColumn` - Removed `droppableProvided` from `RecordBoardColumnCardsContainer` props **RecordBoardColumnCardsContainer.tsx:** - Removed `droppableProvided` from component props and type - Removed `ref` and `{...droppableProps}` from `StyledColumnCardsContainer` - Removed unused `DroppableProvided` import ## Why This Works The `DragAndDropLibraryLegacyReRenderBreaker` (React.memo wrapper) blocks re-renders of the card list during drag — this is the performance optimization from PR twentyhq#15714. By lifting `innerRef` to the parent `StyledColumn`, both the memo'd cards and the placeholder live inside the innerRef DOM element, satisfying the library's requirements while preserving the performance optimization. ## Testing Tested manually with 15+ card columns: - ✅ Drag to top of column: works - ✅ Drag to middle of column: works (was failing) - ✅ Drag to bottom of column: works (was failing) - ✅ Long columns with scroll: works ## History - **PR twentyhq#7600** (Oct 2024): Originally fixed by placing placeholder inside innerRef - **PR twentyhq#15714** (Nov 2025): Performance optimization moved placeholder outside, breaking the fix - **This PR**: Restores correct placeholder placement while keeping performance optimization ## Credit Root cause analysis by @SONARly (twentyhq#18842 (comment))
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Contributor
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
Contributor
There was a problem hiding this comment.
No issues found across 2 files
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
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.
fix(kanban): move drag-and-drop placeholder inside innerRef container
Fixes #18842
The Kanban drag-and-drop was only registering drops near the top of columns because the @hello-pangea/dnd placeholder was rendered outside the innerRef container, breaking the library's droppable area measurement.
Root Cause
The library requires
droppableProvided.placeholderto be a direct child of the element that receivesdroppableProvided.innerRef.Before (broken):
innerRefwas onStyledColumnCardsContainer(inside memo boundary)placeholderwas in parentStyledColumn(outside memo boundary)After (fixed):
innerRefmoved toStyledColumn(outside memo boundary)placeholderstays inStyledColumn(same parent as innerRef)Changes
RecordBoardColumn.tsx:
ref={droppableProvided.innerRef}toStyledColumn{...droppableProvided.droppableProps}toStyledColumndroppableProvidedfromRecordBoardColumnCardsContainerpropsRecordBoardColumnCardsContainer.tsx:
droppableProvidedfrom component props and typerefand{...droppableProps}fromStyledColumnCardsContainerDroppableProvidedimportWhy This Works
The
DragAndDropLibraryLegacyReRenderBreaker(React.memo wrapper) blocks re-renders of the card list during drag — this is the performance optimization from PR #15714.By lifting
innerRefto the parentStyledColumn, both the memo'd cards and the placeholder live inside the innerRef DOM element, satisfying the library's requirements while preserving the performance optimization.Testing
Tested manually with 15+ card columns:
History
Credit
Root cause analysis by @SONARly (#18842 (comment))