From de28f87c62e718d9a0361e1b7dab56c75fb530e1 Mon Sep 17 00:00:00 2001 From: chamiakJ Date: Mon, 9 Jun 2025 07:19:15 +0530 Subject: [PATCH] refactor(task-drag-and-drop): remove unused drag-and-drop hook and simplify task group handling - Deleted `useTaskDragAndDrop` hook to streamline drag-and-drop functionality. - Updated `TaskGroupWrapperOptimized` to remove drag-and-drop context and simplify rendering. - Refactored `TaskListTable` to integrate drag-and-drop directly, enhancing performance and maintainability. - Adjusted task rendering logic to ensure proper handling of task states during drag operations. --- .../src/hooks/useTaskDragAndDrop.ts | 146 -------- .../taskList/task-group-wrapper-optimized.tsx | 75 ++--- .../task-list-table-wrapper.tsx | 2 +- .../task-list-table/task-list-table.tsx | 312 ++++++++++++++---- 4 files changed, 264 insertions(+), 271 deletions(-) delete mode 100644 worklenz-frontend/src/hooks/useTaskDragAndDrop.ts diff --git a/worklenz-frontend/src/hooks/useTaskDragAndDrop.ts b/worklenz-frontend/src/hooks/useTaskDragAndDrop.ts deleted file mode 100644 index cab0a361..00000000 --- a/worklenz-frontend/src/hooks/useTaskDragAndDrop.ts +++ /dev/null @@ -1,146 +0,0 @@ -import { useMemo, useCallback } from 'react'; -import { - DndContext, - DragEndEvent, - DragOverEvent, - DragStartEvent, - PointerSensor, - useSensor, - useSensors, - KeyboardSensor, - TouchSensor, -} from '@dnd-kit/core'; -import { sortableKeyboardCoordinates } from '@dnd-kit/sortable'; -import { useAppDispatch } from '@/hooks/useAppDispatch'; -import { useAppSelector } from '@/hooks/useAppSelector'; -import { updateTaskStatus } from '@/features/tasks/tasks.slice'; -import { ITaskListGroup } from '@/types/tasks/taskList.types'; -import { IProjectTask } from '@/types/project/projectTasksViewModel.types'; - -export const useTaskDragAndDrop = () => { - const dispatch = useAppDispatch(); - const { taskGroups, groupBy } = useAppSelector(state => ({ - taskGroups: state.taskReducer.taskGroups, - groupBy: state.taskReducer.groupBy, - })); - - // Memoize sensors configuration for better performance - const sensors = useSensors( - useSensor(PointerSensor, { - activationConstraint: { - distance: 8, - }, - }), - useSensor(KeyboardSensor, { - coordinateGetter: sortableKeyboardCoordinates, - }), - useSensor(TouchSensor, { - activationConstraint: { - delay: 250, - tolerance: 5, - }, - }) - ); - - const handleDragStart = useCallback((event: DragStartEvent) => { - // Add visual feedback for drag start - const { active } = event; - if (active) { - document.body.style.cursor = 'grabbing'; - } - }, []); - - const handleDragOver = useCallback((event: DragOverEvent) => { - // Handle drag over logic if needed - // This can be used for visual feedback during drag - }, []); - - const handleDragEnd = useCallback( - (event: DragEndEvent) => { - // Reset cursor - document.body.style.cursor = ''; - - const { active, over } = event; - - if (!active || !over || !taskGroups) { - return; - } - - try { - const activeId = active.id as string; - const overId = over.id as string; - - // Find the task being dragged - let draggedTask: IProjectTask | null = null; - let sourceGroupId: string | null = null; - - for (const group of taskGroups) { - const task = group.tasks?.find((t: IProjectTask) => t.id === activeId); - if (task) { - draggedTask = task; - sourceGroupId = group.id; - break; - } - } - - if (!draggedTask || !sourceGroupId) { - console.warn('Could not find dragged task'); - return; - } - - // Determine target group - let targetGroupId: string | null = null; - - // Check if dropped on a group container - const targetGroup = taskGroups.find((group: ITaskListGroup) => group.id === overId); - if (targetGroup) { - targetGroupId = targetGroup.id; - } else { - // Check if dropped on another task - for (const group of taskGroups) { - const targetTask = group.tasks?.find((t: IProjectTask) => t.id === overId); - if (targetTask) { - targetGroupId = group.id; - break; - } - } - } - - if (!targetGroupId || targetGroupId === sourceGroupId) { - return; // No change needed - } - - // Update task status based on group change - const targetGroupData = taskGroups.find((group: ITaskListGroup) => group.id === targetGroupId); - if (targetGroupData && groupBy === 'status') { - const updatePayload: any = { - task_id: draggedTask.id, - status_id: targetGroupData.id, - }; - - if (draggedTask.parent_task_id) { - updatePayload.parent_task = draggedTask.parent_task_id; - } - - dispatch(updateTaskStatus(updatePayload)); - } - } catch (error) { - console.error('Error handling drag end:', error); - } - }, - [taskGroups, groupBy, dispatch] - ); - - // Memoize the drag and drop configuration - const dragAndDropConfig = useMemo( - () => ({ - sensors, - onDragStart: handleDragStart, - onDragOver: handleDragOver, - onDragEnd: handleDragEnd, - }), - [sensors, handleDragStart, handleDragOver, handleDragEnd] - ); - - return dragAndDropConfig; -}; \ No newline at end of file diff --git a/worklenz-frontend/src/pages/projects/projectView/taskList/task-group-wrapper-optimized.tsx b/worklenz-frontend/src/pages/projects/projectView/taskList/task-group-wrapper-optimized.tsx index 71257305..0eda5ec7 100644 --- a/worklenz-frontend/src/pages/projects/projectView/taskList/task-group-wrapper-optimized.tsx +++ b/worklenz-frontend/src/pages/projects/projectView/taskList/task-group-wrapper-optimized.tsx @@ -3,11 +3,6 @@ import { createPortal } from 'react-dom'; import Flex from 'antd/es/flex'; import useIsomorphicLayoutEffect from '@/hooks/useIsomorphicLayoutEffect'; -import { - DndContext, - pointerWithin, -} from '@dnd-kit/core'; - import { ITaskListGroup } from '@/types/tasks/taskList.types'; import { useAppSelector } from '@/hooks/useAppSelector'; @@ -16,7 +11,6 @@ import TaskListBulkActionsBar from '@/components/taskListCommon/task-list-bulk-a import TaskTemplateDrawer from '@/components/task-templates/task-template-drawer'; import { useTaskSocketHandlers } from '@/hooks/useTaskSocketHandlers'; -import { useTaskDragAndDrop } from '@/hooks/useTaskDragAndDrop'; interface TaskGroupWrapperOptimizedProps { taskGroups: ITaskListGroup[]; @@ -28,14 +22,6 @@ const TaskGroupWrapperOptimized = ({ taskGroups, groupBy }: TaskGroupWrapperOpti // Use extracted hooks useTaskSocketHandlers(); - const { - activeId, - sensors, - handleDragStart, - handleDragEnd, - handleDragOver, - resetTaskRowStyles, - } = useTaskDragAndDrop({ taskGroups, groupBy }); // Memoize task groups with colors const taskGroupsWithColors = useMemo(() => @@ -46,18 +32,17 @@ const TaskGroupWrapperOptimized = ({ taskGroups, groupBy }: TaskGroupWrapperOpti [taskGroups, themeMode] ); - // Add drag styles + // Add drag styles without animations useEffect(() => { const style = document.createElement('style'); style.textContent = ` .task-row[data-is-dragging="true"] { opacity: 0.5 !important; - transform: rotate(5deg) !important; z-index: 1000 !important; position: relative !important; } .task-row { - transition: transform 0.2s ease, opacity 0.2s ease; + /* Remove transitions during drag operations */ } `; document.head.appendChild(style); @@ -67,45 +52,31 @@ const TaskGroupWrapperOptimized = ({ taskGroups, groupBy }: TaskGroupWrapperOpti }; }, []); - // Handle animation cleanup after drag ends - useIsomorphicLayoutEffect(() => { - if (activeId === null) { - const timeoutId = setTimeout(resetTaskRowStyles, 50); - return () => clearTimeout(timeoutId); - } - }, [activeId, resetTaskRowStyles]); + // Remove the animation cleanup since we're simplifying the approach return ( - - - {taskGroupsWithColors.map(taskGroup => ( - - ))} + + {taskGroupsWithColors.map(taskGroup => ( + + ))} - {createPortal(, document.body, 'bulk-action-container')} + {createPortal(, document.body, 'bulk-action-container')} - {createPortal( - {}} />, - document.body, - 'task-template-drawer' - )} - - + {createPortal( + {}} />, + document.body, + 'task-template-drawer' + )} + ); }; diff --git a/worklenz-frontend/src/pages/projects/projectView/taskList/task-list-table/task-list-table-wrapper/task-list-table-wrapper.tsx b/worklenz-frontend/src/pages/projects/projectView/taskList/task-list-table/task-list-table-wrapper/task-list-table-wrapper.tsx index 3a3e46a2..63ff9f40 100644 --- a/worklenz-frontend/src/pages/projects/projectView/taskList/task-list-table/task-list-table-wrapper/task-list-table-wrapper.tsx +++ b/worklenz-frontend/src/pages/projects/projectView/taskList/task-list-table/task-list-table-wrapper/task-list-table-wrapper.tsx @@ -249,7 +249,7 @@ const TaskListTableWrapper = ({ className={`border-l-[3px] relative after:content after:absolute after:h-full after:w-1 after:z-10 after:top-0 after:left-0`} color={color} > - + diff --git a/worklenz-frontend/src/pages/projects/projectView/taskList/task-list-table/task-list-table.tsx b/worklenz-frontend/src/pages/projects/projectView/taskList/task-list-table/task-list-table.tsx index 059454c3..8690d895 100644 --- a/worklenz-frontend/src/pages/projects/projectView/taskList/task-list-table/task-list-table.tsx +++ b/worklenz-frontend/src/pages/projects/projectView/taskList/task-list-table/task-list-table.tsx @@ -12,8 +12,8 @@ import { useSortable } from '@dnd-kit/sortable'; import { CSS } from '@dnd-kit/utilities'; import { DraggableAttributes, UniqueIdentifier } from '@dnd-kit/core'; import { SyntheticListenerMap } from '@dnd-kit/core/dist/hooks/utilities'; -import { DragOverlay } from '@dnd-kit/core'; -import { SortableContext, verticalListSortingStrategy } from '@dnd-kit/sortable'; +import { DragOverlay, DndContext, PointerSensor, useSensor, useSensors, KeyboardSensor, TouchSensor } from '@dnd-kit/core'; +import { SortableContext, verticalListSortingStrategy, sortableKeyboardCoordinates } from '@dnd-kit/sortable'; import { createPortal } from 'react-dom'; import { DragEndEvent } from '@dnd-kit/core'; import { List, Card, Avatar, Dropdown, Empty, Divider, Button } from 'antd'; @@ -50,19 +50,20 @@ import StatusDropdown from '@/components/task-list-common/status-dropdown/status import PriorityDropdown from '@/components/task-list-common/priorityDropdown/priority-dropdown'; import AddCustomColumnButton from './custom-columns/custom-column-modal/add-custom-column-button'; import { fetchSubTasks, reorderTasks, toggleTaskRowExpansion, updateCustomColumnValue } from '@/features/tasks/tasks.slice'; +import { useSocket } from '@/socket/socketContext'; +import { SocketEvents } from '@/shared/socket-events'; import { useAuthService } from '@/hooks/useAuth'; import ConfigPhaseButton from '@/features/projects/singleProject/phase/ConfigPhaseButton'; import PhaseDropdown from '@/components/taskListCommon/phase-dropdown/phase-dropdown'; import CustomColumnModal from './custom-columns/custom-column-modal/custom-column-modal'; import { toggleProjectMemberDrawer } from '@/features/projects/singleProject/members/projectMembersSlice'; import SingleAvatar from '@/components/common/single-avatar/single-avatar'; -import { useSocket } from '@/socket/socketContext'; -import { SocketEvents } from '@/shared/socket-events'; interface TaskListTableProps { taskList: IProjectTask[] | null; tableId: string; activeId?: string | null; + groupBy?: string; } interface DraggableRowProps { @@ -71,44 +72,50 @@ interface DraggableRowProps { groupId: string; } -// Add a simplified EmptyRow component that doesn't use hooks -const EmptyRow = () => null; - -// Simplify DraggableRow to eliminate conditional hook calls +// Remove the EmptyRow component and fix the DraggableRow const DraggableRow = ({ task, children, groupId }: DraggableRowProps) => { - // Return the EmptyRow component without using any hooks - if (!task?.id) return ; - + // Always call hooks in the same order - never conditionally const { attributes, listeners, setNodeRef, transform, transition, isDragging } = useSortable({ - id: task.id as UniqueIdentifier, + id: task?.id || 'empty-task', // Provide fallback ID data: { type: 'task', task, groupId, }, + disabled: !task?.id, // Disable dragging for invalid tasks + transition: null, // Disable sortable transitions }); + // If task is invalid, return null to not render anything + if (!task?.id) { + return null; + } + const style = { transform: CSS.Transform.toString(transform), - transition, + transition: isDragging ? 'none' : transition, // Disable transition during drag opacity: isDragging ? 0.3 : 1, position: 'relative' as const, zIndex: isDragging ? 1 : 'auto', backgroundColor: isDragging ? 'var(--dragging-bg)' : undefined, - }; - - // Handle border styling separately to avoid conflicts - const borderStyle = { - borderStyle: isDragging ? 'solid' : undefined, - borderWidth: isDragging ? '1px' : undefined, - borderColor: isDragging ? 'var(--border-color)' : undefined, - borderBottomWidth: document.documentElement.getAttribute('data-theme') === 'light' && !isDragging ? '2px' : undefined + // Handle border styling to avoid conflicts between shorthand and individual properties + ...(isDragging ? { + borderTopWidth: '1px', + borderRightWidth: '1px', + borderBottomWidth: '1px', + borderLeftWidth: '1px', + borderStyle: 'solid', + borderColor: 'var(--border-color)', + } : { + // Only set borderBottomWidth when not dragging to avoid conflicts + borderBottomWidth: document.documentElement.getAttribute('data-theme') === 'light' ? '2px' : undefined + }) }; return ( = ({ taskList, tableId, activeId }) => { +const TaskListTable: React.FC = ({ taskList, tableId, activeId, groupBy }) => { const { t } = useTranslation('task-list-table'); const dispatch = useAppDispatch(); const currentSession = useAuthService().getCurrentSession(); const { socket } = useSocket(); + // Add drag state + const [dragActiveId, setDragActiveId] = useState(null); + + // Configure sensors for drag and drop + const sensors = useSensors( + useSensor(PointerSensor, { + activationConstraint: { + distance: 8, + }, + }), + useSensor(KeyboardSensor, { + coordinateGetter: sortableKeyboardCoordinates, + }), + useSensor(TouchSensor, { + activationConstraint: { + delay: 250, + tolerance: 5, + }, + }) + ); + const themeMode = useAppSelector(state => state.themeReducer.mode); const columnList = useAppSelector(state => state.taskReducer.columns); const visibleColumns = columnList.filter(column => column.pinned); @@ -1525,27 +1553,8 @@ const TaskListTable: React.FC = ({ taskList, tableId, active // Use the tasks from the current group if available, otherwise fall back to taskList prop const displayTasks = currentGroup?.tasks || taskList || []; - const handleDragEnd = (event: DragEndEvent) => { - const { active, over } = event; - if (!over || active.id === over.id) return; - - const activeIndex = displayTasks.findIndex(task => task.id === active.id); - const overIndex = displayTasks.findIndex(task => task.id === over.id); - - if (activeIndex !== -1 && overIndex !== -1) { - dispatch( - reorderTasks({ - activeGroupId: tableId, - overGroupId: tableId, - fromIndex: activeIndex, - toIndex: overIndex, - task: displayTasks[activeIndex], - updatedSourceTasks: displayTasks, - updatedTargetTasks: displayTasks, - }) - ); - } - }; + // Remove the local handleDragEnd as it conflicts with the main DndContext + // All drag handling is now done at the TaskGroupWrapperOptimized level const handleCustomColumnSettings = (columnKey: string) => { if (!columnKey) return; @@ -1554,12 +1563,169 @@ const TaskListTable: React.FC = ({ taskList, tableId, active dispatch(toggleCustomColumnModalOpen(true)); }; + // Drag and drop handlers + const handleDragStart = (event: any) => { + setDragActiveId(event.active.id); + }; + + const handleDragEnd = (event: DragEndEvent) => { + const { active, over } = event; + setDragActiveId(null); + + if (!over || !active || active.id === over.id) { + return; + } + + const activeTask = displayTasks.find(task => task.id === active.id); + if (!activeTask) { + console.error('Active task not found:', { activeId: active.id, displayTasks: displayTasks.map(t => ({ id: t.id, name: t.name })) }); + return; + } + + console.log('Found activeTask:', { + id: activeTask.id, + name: activeTask.name, + status_id: activeTask.status_id, + status: activeTask.status, + priority: activeTask.priority, + project_id: project?.id, + team_id: project?.team_id, + fullProject: project + }); + + // Use the tableId directly as the group ID (it should be the group ID) + const currentGroupId = tableId; + + console.log('Drag operation:', { + activeId: active.id, + overId: over.id, + tableId, + currentGroupId, + displayTasksLength: displayTasks.length + }); + + // Check if this is a reorder within the same group + const overTask = displayTasks.find(task => task.id === over.id); + if (overTask) { + // Reordering within the same group + const oldIndex = displayTasks.findIndex(task => task.id === active.id); + const newIndex = displayTasks.findIndex(task => task.id === over.id); + + console.log('Reorder details:', { oldIndex, newIndex, activeTask: activeTask.name }); + + if (oldIndex !== newIndex && oldIndex !== -1 && newIndex !== -1) { + // Get the actual sort_order values from the tasks + const fromSortOrder = activeTask.sort_order || oldIndex; + const overTaskAtNewIndex = displayTasks[newIndex]; + const toSortOrder = overTaskAtNewIndex?.sort_order || newIndex; + + console.log('Sort order details:', { + oldIndex, + newIndex, + fromSortOrder, + toSortOrder, + activeTaskSortOrder: activeTask.sort_order, + overTaskSortOrder: overTaskAtNewIndex?.sort_order + }); + + // Create updated task list with reordered tasks + const updatedTasks = [...displayTasks]; + const [movedTask] = updatedTasks.splice(oldIndex, 1); + updatedTasks.splice(newIndex, 0, movedTask); + + console.log('Dispatching reorderTasks with:', { + activeGroupId: currentGroupId, + overGroupId: currentGroupId, + fromIndex: oldIndex, + toIndex: newIndex, + taskName: activeTask.name + }); + + // Update local state immediately for better UX + dispatch(reorderTasks({ + activeGroupId: currentGroupId, + overGroupId: currentGroupId, + fromIndex: oldIndex, + toIndex: newIndex, + task: activeTask, + updatedSourceTasks: updatedTasks, + updatedTargetTasks: updatedTasks + })); + + // Send socket event for backend sync + if (socket && project?.id && active.id && activeTask.id) { + // Helper function to validate UUID or return null + const validateUUID = (value: string | undefined | null): string | null => { + if (!value || value.trim() === '') return null; + // Basic UUID format check (8-4-4-4-12 characters) + const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + return uuidRegex.test(value) ? value : null; + }; + + const body = { + from_index: fromSortOrder, + to_index: toSortOrder, + project_id: project.id, + from_group: currentGroupId, + to_group: currentGroupId, + group_by: groupBy || 'status', // Use the groupBy prop + to_last_index: false, + task: { + id: activeTask.id, // Use activeTask.id instead of active.id to ensure it's valid + project_id: project.id, + status: validateUUID(activeTask.status_id || activeTask.status), + priority: validateUUID(activeTask.priority) + }, + team_id: project.team_id || currentSession?.team_id || '' + }; + + // Validate required fields before sending + if (!body.task.id) { + console.error('Cannot send socket event: task.id is missing', { activeTask, active }); + return; + } + + console.log('Validated values:', { + from_index: body.from_index, + to_index: body.to_index, + status: body.task.status, + priority: body.task.priority, + team_id: body.team_id, + originalStatus: activeTask.status_id || activeTask.status, + originalPriority: activeTask.priority, + originalTeamId: project.team_id, + sessionTeamId: currentSession?.team_id, + finalTeamId: body.team_id + }); + + console.log('Sending socket event:', body); + socket.emit(SocketEvents.TASK_SORT_ORDER_CHANGE.toString(), body); + } else { + console.error('Cannot send socket event: missing required data', { + hasSocket: !!socket, + hasProjectId: !!project?.id, + hasActiveId: !!active.id, + hasActiveTaskId: !!activeTask.id, + activeTask, + active + }); + } + } + } + }; + return (
- t.id).filter(Boolean) || []) as string[]} - strategy={verticalListSortingStrategy} + + t?.id).map(t => t.id).filter(Boolean) || []) as string[]} + strategy={verticalListSortingStrategy} + >
@@ -1611,25 +1777,29 @@ const TaskListTable: React.FC = ({ taskList, tableId, active {displayTasks && displayTasks.length > 0 ? ( - displayTasks.map(task => { - const updatedTask = findTaskInGroups(task.id || '') || task; + displayTasks + .filter(task => task?.id) // Filter out tasks without valid IDs + .map(task => { + const updatedTask = findTaskInGroups(task.id || '') || task; - return ( - - {renderTaskRow(updatedTask)} - {updatedTask.show_sub_tasks && ( - <> - {updatedTask?.sub_tasks?.map(subtask => renderTaskRow(subtask, true))} - - + return ( + + {renderTaskRow(updatedTask)} + {updatedTask.show_sub_tasks && ( + <> + {updatedTask?.sub_tasks?.map(subtask => + subtask?.id ? renderTaskRow(subtask, true) : null + )} + + - - )} - - ); - }) + + )} + + ); + }) ) : (
- -
+ +
@@ -1643,17 +1813,15 @@ const TaskListTable: React.FC = ({ taskList, tableId, active - {activeId && displayTasks?.length ? ( - - {renderTaskRow(displayTasks.find(t => t.id === activeId))} -
+ {dragActiveId ? ( +
+ Moving task... +
) : null} + {/* Add task row is positioned outside of the scrollable area */}