From 011afa6011a14d05b08a8ad0338b8205b4a18d97 Mon Sep 17 00:00:00 2001 From: Valeriy Proklov <99179211+Val4evr@users.noreply.github.com> Date: Sat, 6 Jun 2026 14:22:57 +0200 Subject: [PATCH] Allow kanban cross-column drag when sorting is enabled (#21025) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This PR allows kanban cards to be dragged across columns while sorting is enabled. Previously, any board drag while a sort was active opened the “Remove sorting?” modal. That makes sense for same-column reordering, because manual reorder conflicts with the active sort. But for cross-column moves, the user is changing the grouped field, not trying to manually reorder the destination column. With this change: - Same-column drag with sorting enabled still opens the existing remove-sorting modal. - Cross-column drag with sorting enabled updates only the group field. - The destination column keeps using the active sort to determine where the card appears. - Unsorted board drag behavior continues to update `position` as before. ## Why On sorted kanban boards, moving a card to another column is a valid workflow even though manual reordering is not. The previous guard blocked both cases because it only checked whether sorting was active, not whether the card stayed inside the same column. ## Implementation The drop behavior now distinguishes between: - sorted same-column drops, which remain blocked - sorted cross-column drops, which are allowed without a position update - unsorted drops, which keep the existing position-update behavior A small helper captures that decision and has focused unit coverage. ## Validation - Manually verified sorted cross-column drag persists after refresh. - Manually verified sorted same-column drag still opens the remove-sorting modal. - Manually verified unsorted same-column drag still reorders cards. - Manually verified unsorted cross-column drag still moves cards. - Ran focused Jest coverage for the sorted board drop decision. - Ran formatting and oxlint checks on touched frontend files. - Ran `twenty-front` typecheck. - Ran `twenty-front` production build. Co-authored-by: Félix Malfait Co-authored-by: Charles Bochet --- .../components/RecordBoardDragDropContext.tsx | 12 ++++- .../getBoardCardDropBehavior.test.ts | 42 +++++++++++++++ .../utils/getBoardCardDropBehavior.ts | 17 ++++++ .../hooks/useProcessBoardCardDrop.ts | 13 ++++- .../hooks/useUpdateDroppedRecordOnBoard.ts | 52 +++++++++++-------- 5 files changed, 109 insertions(+), 27 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/record-board/utils/__tests__/getBoardCardDropBehavior.test.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-board/utils/getBoardCardDropBehavior.ts diff --git a/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoardDragDropContext.tsx b/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoardDragDropContext.tsx index 8d086c637e..bb0034b37d 100644 --- a/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoardDragDropContext.tsx +++ b/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoardDragDropContext.tsx @@ -8,6 +8,7 @@ import { originalDragSelectionComponentState } from '@/object-record/record-drag import { RECORD_INDEX_REMOVE_SORTING_MODAL_ID } from '@/object-record/record-index/constants/RecordIndexRemoveSortingModalId'; import { currentRecordSortsComponentState } from '@/object-record/record-sort/states/currentRecordSortsComponentState'; +import { getBoardCardDropBehavior } from '@/object-record/record-board/utils/getBoardCardDropBehavior'; import { useModal } from '@/ui/layout/modal/hooks/useModal'; import { useAtomComponentSelectorCallbackState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentSelectorCallbackState'; import { useAtomComponentStateCallbackState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateCallbackState'; @@ -82,8 +83,13 @@ export const RecordBoardDragDropContext = ({ } const existingRecordSorts = store.get(currentRecordSorts); + const boardCardDropBehavior = getBoardCardDropBehavior({ + hasRecordSorts: existingRecordSorts.length > 0, + sourceDroppableId: result.source.droppableId, + destinationDroppableId: result.destination.droppableId, + }); - if (existingRecordSorts.length > 0) { + if (boardCardDropBehavior.shouldBlockDrop) { store.set(isRecordBoardDropProcessingCallbackState, false); endRecordDrag(); openModal(RECORD_INDEX_REMOVE_SORTING_MODAL_ID); @@ -91,7 +97,9 @@ export const RecordBoardDragDropContext = ({ } try { - processBoardCardDrop(result, originalDragSelection); + processBoardCardDrop(result, originalDragSelection, { + shouldUpdatePosition: boardCardDropBehavior.shouldUpdatePosition, + }); } catch (error) { store.set(isRecordBoardDropProcessingCallbackState, false); endRecordDrag(); diff --git a/packages/twenty-front/src/modules/object-record/record-board/utils/__tests__/getBoardCardDropBehavior.test.ts b/packages/twenty-front/src/modules/object-record/record-board/utils/__tests__/getBoardCardDropBehavior.test.ts new file mode 100644 index 0000000000..24443c6452 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-board/utils/__tests__/getBoardCardDropBehavior.test.ts @@ -0,0 +1,42 @@ +import { getBoardCardDropBehavior } from '@/object-record/record-board/utils/getBoardCardDropBehavior'; + +describe('getBoardCardDropBehavior', () => { + it('should block same-column drops when record sorting is active', () => { + expect( + getBoardCardDropBehavior({ + hasRecordSorts: true, + sourceDroppableId: 'new', + destinationDroppableId: 'new', + }), + ).toEqual({ + shouldBlockDrop: true, + shouldUpdatePosition: false, + }); + }); + + it('should allow cross-column drops without position updates when record sorting is active', () => { + expect( + getBoardCardDropBehavior({ + hasRecordSorts: true, + sourceDroppableId: 'new', + destinationDroppableId: 'won', + }), + ).toEqual({ + shouldBlockDrop: false, + shouldUpdatePosition: false, + }); + }); + + it('should allow drops with position updates when record sorting is inactive', () => { + expect( + getBoardCardDropBehavior({ + hasRecordSorts: false, + sourceDroppableId: 'new', + destinationDroppableId: 'won', + }), + ).toEqual({ + shouldBlockDrop: false, + shouldUpdatePosition: true, + }); + }); +}); diff --git a/packages/twenty-front/src/modules/object-record/record-board/utils/getBoardCardDropBehavior.ts b/packages/twenty-front/src/modules/object-record/record-board/utils/getBoardCardDropBehavior.ts new file mode 100644 index 0000000000..35654e5cc4 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-board/utils/getBoardCardDropBehavior.ts @@ -0,0 +1,17 @@ +export const getBoardCardDropBehavior = ({ + hasRecordSorts, + sourceDroppableId, + destinationDroppableId, +}: { + hasRecordSorts: boolean; + sourceDroppableId: string; + destinationDroppableId: string; +}) => { + const isMovingInsideSameRecordGroup = + sourceDroppableId === destinationDroppableId; + + return { + shouldBlockDrop: hasRecordSorts && isMovingInsideSameRecordGroup, + shouldUpdatePosition: !hasRecordSorts, + }; +}; diff --git a/packages/twenty-front/src/modules/object-record/record-drag/hooks/useProcessBoardCardDrop.ts b/packages/twenty-front/src/modules/object-record/record-drag/hooks/useProcessBoardCardDrop.ts index 323fd99173..64c2e84487 100644 --- a/packages/twenty-front/src/modules/object-record/record-drag/hooks/useProcessBoardCardDrop.ts +++ b/packages/twenty-front/src/modules/object-record/record-drag/hooks/useProcessBoardCardDrop.ts @@ -40,9 +40,15 @@ export const useProcessBoardCardDrop = () => { ); const processBoardCardDrop = useCallback( - (boardCardDropResult: DropResult, selectedRecordIds: string[]) => { + ( + boardCardDropResult: DropResult, + selectedRecordIds: string[], + options?: { shouldUpdatePosition?: boolean }, + ) => { if (!isDefined(selectFieldMetadataItem)) return; + const shouldUpdatePosition = options?.shouldUpdatePosition ?? true; + processGroupDrop({ groupDropResult: boardCardDropResult, store, @@ -51,7 +57,10 @@ export const useProcessBoardCardDrop = () => { recordIndexRecordIdsByGroupCallbackFamilyState, onUpdateRecord: ({ recordId, position }, targetRecordGroupValue) => { updateDroppedRecordOnBoard( - { recordId, position }, + { + recordId, + position: shouldUpdatePosition ? position : undefined, + }, targetRecordGroupValue, ); }, diff --git a/packages/twenty-front/src/modules/object-record/record-drag/hooks/useUpdateDroppedRecordOnBoard.ts b/packages/twenty-front/src/modules/object-record/record-drag/hooks/useUpdateDroppedRecordOnBoard.ts index 5d36da4485..a2b1fda11f 100644 --- a/packages/twenty-front/src/modules/object-record/record-drag/hooks/useUpdateDroppedRecordOnBoard.ts +++ b/packages/twenty-front/src/modules/object-record/record-drag/hooks/useUpdateDroppedRecordOnBoard.ts @@ -45,10 +45,6 @@ export const useUpdateDroppedRecordOnBoard = () => { recordStoreFamilyState.atomFamily(recordId), ) as Record | null | undefined; - if (!isDefined(newPosition)) { - return; - } - if (!isDefined(initialRecord)) { return; } @@ -94,7 +90,10 @@ export const useUpdateDroppedRecordOnBoard = () => { const isSamePosition = initialRecord.position === newPosition; - if (movingInsideSameRecordGroup && isSamePosition) { + if ( + movingInsideSameRecordGroup && + (!isDefined(newPosition) || isSamePosition) + ) { return; } @@ -126,25 +125,32 @@ export const useUpdateDroppedRecordOnBoard = () => { ); } - const targetGroupRecordsWithIds = extractRecordPositions( - currentRecordIdsInTargetRecordGroup, - store, - ); + if (isDefined(newPosition)) { + const targetGroupRecordsWithIds = extractRecordPositions( + currentRecordIdsInTargetRecordGroup, + store, + ); - const newTargetRecordGroupWithIds = [ - ...targetGroupRecordsWithIds, - { - id: recordId, - position: newPosition, - }, - ]; + const newTargetRecordGroupWithIds = [ + ...targetGroupRecordsWithIds, + { + id: recordId, + position: newPosition, + }, + ]; - newTargetRecordGroupWithIds.sort(sortByProperty('position', 'asc')); + newTargetRecordGroupWithIds.sort(sortByProperty('position', 'asc')); - store.set( - recordIndexRecordIdsByGroupCallbackFamilyState(targetRecordGroupId), - newTargetRecordGroupWithIds.map((record) => record.id), - ); + store.set( + recordIndexRecordIdsByGroupCallbackFamilyState(targetRecordGroupId), + newTargetRecordGroupWithIds.map((record) => record.id), + ); + } else { + store.set( + recordIndexRecordIdsByGroupCallbackFamilyState(targetRecordGroupId), + [...currentRecordIdsInTargetRecordGroup, recordId], + ); + } upsertRecordsInStore({ partialRecords: [ @@ -155,7 +161,7 @@ export const useUpdateDroppedRecordOnBoard = () => { (initialRecord as { __typename?: string })?.__typename ?? 'Record', [selectFieldMetadataItem.name]: targetRecordGroupValue, - position: newPosition, + ...(isDefined(newPosition) && { position: newPosition }), } as ObjectRecord, ], }); @@ -164,7 +170,7 @@ export const useUpdateDroppedRecordOnBoard = () => { idToUpdate: recordId, updateOneRecordInput: { [selectFieldMetadataItem.name]: targetRecordGroupValue, - position: newPosition, + ...(isDefined(newPosition) && { position: newPosition }), }, }); },