OUT-3124 | Task dragging lacks responsiveness#1200
Conversation
Replaces react-dnd (HTML5 drag API + custom drag preview) with @dnd-kit/core for smoother, pointer-tracked task dragging that aligns with the App Library interaction. Drop business logic (redux update + PATCH) is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR migrates task drag-and-drop from Confidence Score: 4/5Safe to merge; all P2 findings are style/quality issues that do not affect the existing drop business logic or Redux state. No P0 or P1 defects found. Three P2 issues: mid-file static imports mislabeled as lazy, touch-action: manipulation vs. the dnd-kit-recommended none, and KeyboardSensor wired without a coordinateGetter. Core drag/drop logic, same-column suppression, cascade modal flow, and Redux dispatch are all correct. src/hoc/dndKit/TaskDndContext.tsx (import ordering, KeyboardSensor), src/hoc/dndKit/DraggableTask.tsx (touchAction) Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant DraggableTask
participant TaskDndContext
participant DroppableArea
participant TaskBoard
User->>DraggableTask: pointerdown / touchstart
Note over DraggableTask: MouseSensor: 6px distance<br/>TouchSensor: 150ms delay
DraggableTask->>TaskDndContext: onDragStart(event)
TaskDndContext->>TaskDndContext: setActiveTask(task)
TaskDndContext-->>User: DragOverlay renders preview (snapCenterToCursor)
User->>DroppableArea: pointer moves over column/section
DroppableArea->>DroppableArea: isOver=true, highlight if different column
User->>TaskDndContext: pointerup (onDragEnd)
TaskDndContext->>TaskDndContext: setActiveTask(null)
alt over is null OR same workflowStateId
TaskDndContext-->>User: no-op, overlay dismissed
else different column
TaskDndContext->>TaskBoard: onDropItem({taskId, targetWorkflowStateId})
TaskBoard->>TaskBoard: updateWorkflowStateIdByTaskId (Redux)
TaskBoard->>TaskBoard: updateTask / clientUpdateTask (PATCH)
alt task has open subtasks AND target is completed
TaskBoard-->>User: CompletionCascadeModal
end
end
Reviews (1): Last reviewed commit: "feat(OUT-3124): migrate task drag-and-dr..." | Re-trigger Greptile |
| // Split out so we can lazy-import DragOverlay without breaking SSR. | ||
| import { DragOverlay } from '@dnd-kit/core' | ||
| import { snapCenterToCursor } from '@dnd-kit/modifiers' |
There was a problem hiding this comment.
Mid-file imports are not actually lazy
DragOverlay and snapCenterToCursor are static import statements, which the ES module system hoists to the top of the module regardless of their position in source. The comment "lazy-import DragOverlay without breaking SSR" is inaccurate — this achieves nothing different from placing them at the top of the file, and most linters (import/first) will flag it. A true lazy-load would require dynamic() from Next.js or React.lazy. Please move these imports to the top of the file to avoid confusion.
| ...style, | ||
| opacity: isDragging ? 0.35 : 1, | ||
| cursor: disabled ? undefined : 'grab', | ||
| touchAction: 'manipulation', |
There was a problem hiding this comment.
touch-action: manipulation may prevent touch dragging
@dnd-kit's TouchSensor documentation recommends touch-action: none on draggable elements so the browser does not intercept touch events for scrolling before the sensor can claim them. manipulation still permits panning, so on certain browsers/devices the browser may begin a scroll pass during the 150ms delay window, preventing the sensor from ever reaching its activation threshold. If you specifically need page scrolling to work while a finger rests on a card (before the drag activates), this is a deliberate trade-off worth calling out in a comment; otherwise none is the safer default for drag reliability.
| touchAction: 'manipulation', | |
| touchAction: 'none', |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const sensors = useSensors( | ||
| useSensor(MouseSensor, { activationConstraint: { distance: 6 } }), | ||
| useSensor(TouchSensor, { activationConstraint: { delay: 150, tolerance: 8 } }), | ||
| useSensor(KeyboardSensor), |
There was a problem hiding this comment.
KeyboardSensor lacks a coordinateGetter for column/section layout
The default KeyboardSensor moves the drag overlay in 2-D pixel space using arrow keys, which doesn't correspond to the column (board) or section (list) model used here. Keyboard users who attempt to drag-and-drop a task will see the overlay shift but won't be able to land it on a target column, silently failing to trigger onDropItem. Consider either wiring up a custom coordinateGetter that snaps to column/section droppable bounds, or removing KeyboardSensor until that work is done so keyboard users get no false affordance.
|
Deployment failed with the following error: Learn More: https://vercel.link/multiple-function-regions |
- Hoist DragOverlay/snapCenterToCursor imports to module top (ES modules hoist them anyway; the mid-file placement was misleading). - Drop KeyboardSensor: without a column/section coordinateGetter it produced a non-functional drag affordance for keyboard users. - Document the deliberate touchAction: 'manipulation' choice so the trade-off vs. dnd-kit's recommended 'none' is explicit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
react-dnd(HTML5 drag API + custom 1×1 GIF preview overlay) to@dnd-kit/corefor smooth, pointer-tracked dragging that matches the App Library interaction.src/hoc/dndKit/(TaskDndContext,DraggableTask,DroppableArea,TaskDragPreview) instead of using@dnd-kit/sortable, since column membership — not in-column ordering — is the only drop concern.updateWorkflowStateIdByTaskIddispatch and sameupdateTask/clientUpdateTaskPATCH, including the open-subtask cascade modal flow.react-dnd,react-dnd-html5-backend,react-dnd-touch-backend, and the customModifiedBackend/DndWrapper/DragDropHandler/Droppable/CustomDragLayer/CardDragLayerplumbing.Notable details
MouseSensoractivation distance is 6px andTouchSensoruses a 150ms delay so taps on task cards still navigate via the wrappingCustomLink/NextLink.DragOverlayusesdropAnimation={null}+snapCenterToCursorto follow the pointer.threshold.y: 0.18) and disabled in board view to avoid hijacking the column's horizontal scroll.@tanstack/react-virtual) compatibility verified:useDraggablere-attaches refs cleanly as virtualized rows mount/unmount; theDragOverlayportal keeps the preview rendered after the source row scrolls out of view.Test plan
Testing criteria
Upon testing, Found out we need to disabled drag for subtasks and shared task on CU view.
Also enable horizontal scrolling on drag.