Repository navigation
feat: make cross-column dragged card sortable - #1894
Conversation
PR Summary by QodoPreview and sort cross-column Kanban card drops
AI Description
Diagram
High-Level Assessment
Files changed (48)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
6c29df3 to
e7a167f
Compare
…d-sorting # Conflicts: # apps/web/src/components/kanban-board/column/column-dropzone.tsx # apps/web/src/components/kanban-board/filtered-drag.test.tsx # apps/web/src/components/kanban-board/index.tsx # apps/web/src/components/kanban-board/task-card.tsx
Dragging a card into another column now shows it in place while hovering, and the drop lands where it was shown. This works with mouse, touch and keyboard, so the hold-to-sort modifier and its key listeners are gone. - Stop copying the neighbouring card's priority onto ⌘-dropped cards - Replace the white full-column overlay with a themed hint, shown only on number- or priority-sorted boards where drops are append-only - Drop onDragMove, the always-on layout animation, the sortable transform override and the unused moveBoardTask parameter
While previewing a cross-column move, the dragged card left a fading copy behind in the column it moved out of, which overlapped the cards around it. The dragged card now skips its enter and exit animations.
|
/review |
Code Review by Qodo
1.
|
Address review feedback on the cross-column preview: - Rebuild the preview from the current board and the last cross-column hover, so filters and live updates during a drag stay visible and the drop is placed against fresh data - Let each column's drop zone fill its body, so an empty column still wins collisions against cards in neighbouring columns - Split the drag preview into single-purpose modules with tests beside them, and drop the inline comments
| (sort.field !== "position" && | ||
| sort.field !== "number" && | ||
| sort.field !== "priority") | ||
| } | ||
| sortedByNumber={sort.field === "number"} | ||
| sortedByPriority={sort.field === "priority"} |
There was a problem hiding this comment.
8. Cross-column drags fail whenever a board filter is active 🐞 Bug ≡ Correctness
disableDragDrop now allows dragging whenever sort.field is position, number, or priority, while sortedProject passed to KanbanBoard is still derived from filteredProject, which drops tasks that don't match the active filters. moveBoardTask builds its expectedTasks snapshot only from the visible (filtered) source/destination columns, but the reorder endpoint compares that snapshot against every task currently in those statuses on the server, so any hidden task causes the request to fail with a 409 and the optimistic move to roll back for priority-sorted boards exactly as it already did for number-sorted boards.
Agent Prompt
## Issue description
Enabling cross-column drag for priority-sorted boards (in addition to number-sorted) extends a pre-existing conflict: `sortedProject` fed to `KanbanBoard` is built from `filteredProject`, which omits tasks that don't match active filters. The reorder mutation's `expectedTasks` snapshot (built from the filtered, visible columns) will not match the server's unfiltered view of the same statuses whenever a filter is active, causing the reorder request to be rejected and the optimistic move rolled back.
## Fix Focus Areas
- apps/web/src/routes/_layout/_authenticated/dashboard/workspace/$workspaceId/project/$projectId/board.tsx[330-339]
- apps/web/src/components/kanban-board/move-task.ts[55-90]
## Recommended Fix
Extend `disableDragDrop` to also account for `hasActiveFilters` (or any other condition that hides tasks within a status), disabling drag-and-drop reordering while a filter is active, for both the number- and priority-sorted cases. Alternatively, pass the full unfiltered project's columns (not just the visible subset) into `moveBoardTask` so `expectedTasks` includes every task in the affected statuses.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| (sort.field !== "position" && | ||
| sort.field !== "number" && | ||
| sort.field !== "priority") | ||
| } | ||
| sortedByNumber={sort.field === "number"} | ||
| sortedByPriority={sort.field === "priority"} |
There was a problem hiding this comment.
12. Priority-board reorders in one column snap back 🐞 Bug ≡ Correctness
board.tsx now enables dragging when the board is sorted by priority, but moveBoardTask returns null when appendOnly is set and the source and destination columns are the same. handleDragOver only sets the sort hint for a different column, so a drag inside one column shows the sortable shift animation, then snaps back on drop with no hint explaining why.
Agent Prompt
## Issue description
On priority-sorted boards, dragging a card within its own column animates a reorder, but nothing is saved and the card snaps back. No hint is shown.
## Fix Focus Areas
- apps/web/src/components/kanban-board/index.tsx[207-221]
- apps/web/src/components/kanban-board/drag-preview.ts[13-21]
## Recommended Fix
When `isAutomaticallySorted` is true, also show the sort hint while the card hovers its own column. Another option is to disable sortable reordering within the source column on automatically sorted boards.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const handleDragOver = ({ active, over }: DragOverEvent) => { | ||
| if (!isAutomaticallySorted) { | ||
| if (over) dragPreview.hover(active, over); | ||
| return; | ||
| } |
There was a problem hiding this comment.
9. Previewed moves can silently revert on drop 🐞 Bug ☼ Reliability
handleDragOver updates the cross-column preview on every hover without checking the guards in handleDragEnd: disableDragDrop (keyboard drags are still allowed), isReordering, and an in-flight ["tasks", id] fetch. Each successful reorder invalidates and refetches the board. A user who drags again during that window sees the card settle in the new column, then jump back on drop with no feedback.
Agent Prompt
## Issue description
The cross-column preview appears even when `handleDragEnd` will reject the drop (drag disabled, reorder pending, board fetching). The card visibly moves, then jumps back with no message.
## Fix Focus Areas
- apps/web/src/components/kanban-board/index.tsx[207-237]
## Recommended Fix
Pull the rejection checks (`disableDragDrop`, `isReordering`, fetchStatus === 'fetching') into a helper. Call it in `handleDragOver` and skip `dragPreview.hover` when it is true. Optionally show a toast when a drop is discarded.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit c3ffa5e |
Dragging between columns re-rendered every card several times per pointer move, and an empty column could send the preview into an endless loop. - Split TaskCard into a thin sortable wrapper and memoized content, so dnd-kit's per-move updates no longer re-render the card body - Pass the dragged card's id to columns as a prop instead of reading the drag context, which changes on every pointer move - Pick drop targets by what is under the pointer instead of corner distance, which made the preview flip between an empty column and a neighbouring card until React gave up - Drop on the shown position when released between columns
Every board card and list row rendered its full context menu, with eight mutation hooks and every submenu, plus a delete dialog, before anyone opened them. With 200 tasks that dominated switching to the board or list. Both now mount the first time they open and stay mounted afterwards.
Address Qodo review findings: - Skip and clear the cross-column preview and sort hint while the board would refuse the drop (dragging disabled, a reorder in flight, or the board refetching), so a card no longer lands and then jumps back - On number- and priority-sorted boards, stop shifting cards during a drag within a column and show the sort hint on the hovered column, including the card's own, instead of animating a move that snaps back
|
/review |
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
Code review by qodo was updated up to the latest commit 01e3065 |
Releasing over empty space in a column targeted the column itself, which the drop treated as "append" while the preview still showed the card in its earlier slot. Blank space in a column with cards now targets the nearest card, so the preview follows the pointer to the bottom and the drop saves what was shown. Empty columns still target the column.
|
/review |
|
Code review by qodo was updated up to the latest commit 1d40c64 |
|
Code review by qodo was updated up to the latest commit 1d40c64 |
… drops Address Qodo review findings: - Make the whole column, header included, the drop target, so releasing over a header lands at the top of that column instead of being ignored or committed to a previously previewed column - Snap the pointer to a column across the gap between columns - Clear the preview when the pointer leaves every column and cancel a release there, instead of saving the last previewed position
|
/review |
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
|
Code review by qodo was updated up to the latest commit c905658 |
Address Qodo review findings: - Choose the column whose visible area holds the pointer, or the nearer column across a gap, instead of the first match, so the right half of a gap no longer drops into the left column - Only consider cards visible inside that column, so cards scrolled out of view no longer accept drops released above or below the board
|
/review |
|
Code review by qodo was updated up to the latest commit 62d172d |
| disabled: disableDragDrop, | ||
| data: { isFinalColumn }, | ||
| }); | ||
| const { handleKeyDown } = useTaskCardClick(task, listeners); |
There was a problem hiding this comment.
4. Board cards repeat workspace subscriptions 🐞 Bug ➹ Performance
TaskCard and TaskCardContent each call useTaskCardClick, which subscribes to workspace and selection state, while TaskCardContent also calls useActiveWorkspace directly. Every displayed card now runs the workspace hook three times instead of once, multiplying subscriptions and update work on boards with many cards.
Agent Prompt
## Issue description
The card wrapper and content independently call `useTaskCardClick`, and the content separately calls `useActiveWorkspace`. This triples workspace-hook calls per displayed card.
## Fix Focus Areas
- apps/web/src/components/kanban-board/task-card/index.tsx[26-31]
- apps/web/src/components/kanban-board/task-card/task-card-content.tsx[81-81]
- apps/web/src/components/kanban-board/task-card/task-card-content.tsx[139-139]
- apps/web/src/components/kanban-board/task-card/use-task-card-click.ts[8-18]
## Recommended Fix
Create the click handlers once per card and pass them to the content, reusing the workspace value needed there rather than establishing duplicate hook subscriptions.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 62d172d |
|
Hey, I changed this a little and added some performance improvements. The Linear approach was fine, however I believe we don't need to press Ctrl to re-order the tasks so I made it by default possible to re-order the tasks. Thanks for the contribution! |
|
Code review by qodo was updated up to the latest commit 62d172d |
|
Code review by qodo was updated up to the latest commit 62d172d |
### Features - make cross-column dragged card sortable: usekaneo#1894 - **i18n:** add zh-TW locale: usekaneo#1893 - **integrations:** add label-based sync in advanced settings: usekaneo#1908 ### Bug Fixes - **web:** show the task label editor on narrow screens: usekaneo#1924 - convert ineligible contributions to draft pull requests: [291da4a](usekaneo@291da4a) - **i18n:** translate the zh-CN strings added since the last sync: usekaneo#1916 - **ci:** exclude skipped events from eligibility concurrency: usekaneo#1917 ### Credits Huge thanks to @VictorOnwukwe, @kenny-ish, @ApplesBear-X, @tinsever, and @FunnyQ for helping!
2 participants
Description
This PR enables the sorting of a card when dragging from one status column to another.
UX notes:
When a card is dragged into another column, an overlay appears, prompting them to hold the "⌘" key if they want to sort the added card
Type of Change
How Has This Been Tested?
Screenshots (if applicable)
Task.placement.sorting.PR.video.mov
Checklist
Additional Notes
None