fix: Fix task reordering when completed tasks are hidden

The index produced by drag-and-drop refers to the rendered task list,
which excludes completed tasks when hideCompletedTasks is enabled, but
selectNextTaskPosition resolved that index against the unfiltered task
array, computing positions between the wrong (often hidden) neighbors.

Lift the completed-tasks visibility state up to TaskLists so the drag
end handler can pass it through the move action, and apply the same
filter in selectNextTaskPosition that the rendered list uses.

Fixes #1718

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

client/src/components/cards/CardModal/TaskLists/Item.jsx

@@ -10,7 +10,6 @@ import classNames from 'classnames';
 import { useSelector } from 'react-redux';
 import { Draggable } from 'react-beautiful-dnd';
 import { Button, Icon } from 'semantic-ui-react';
-import { useToggle } from '../../../../lib/hooks';
 
 import selectors from '../../../../selectors';
 import { usePopupInClosableContext } from '../../../../hooks';
@@ -20,7 +19,7 @@ import TaskList from '../../../task-lists/TaskList';
 
 import styles from './Item.module.scss';
 
-const Item = React.memo(({ id, index }) => {
+const Item = React.memo(({ id, index, isCompletedVisible, onCompletedVisibleToggle }) => {
   const selectTaskListById = useMemo(() => selectors.makeSelectTaskListById(), []);
 
   const taskList = useSelector((state) => selectTaskListById(state, id));
@@ -30,11 +29,9 @@ const Item = React.memo(({ id, index }) => {
     return !!boardMembership && boardMembership.role === BoardMembershipRoles.EDITOR;
   });
 
-  const [isCompletedVisible, toggleCompletedVisible] = useToggle();
-
   const handleToggleCompletedVisibleClick = useCallback(() => {
-    toggleCompletedVisible();
-  }, [toggleCompletedVisible]);
+    onCompletedVisibleToggle(id);
+  }, [id, onCompletedVisibleToggle]);
 
   const EditPopup = usePopupInClosableContext(EditStep);
 
@@ -104,6 +101,8 @@ const Item = React.memo(({ id, index }) => {
 Item.propTypes = {
   id: PropTypes.string.isRequired,
   index: PropTypes.number.isRequired,
+  isCompletedVisible: PropTypes.bool.isRequired,
+  onCompletedVisibleToggle: PropTypes.func.isRequired,
 };
 
 export default Item;

client/src/components/cards/CardModal/TaskLists/TaskLists.jsx

@@ -3,7 +3,7 @@
  * Licensed under the Fair Use License: https://github.com/plankanban/planka/blob/master/LICENSE.md
  */
 
-import React, { useCallback } from 'react';
+import React, { useCallback, useState } from 'react';
 import { useDispatch, useSelector } from 'react-redux';
 import { DragDropContext, Droppable } from 'react-beautiful-dnd';
 import { closePopup } from '../../../../lib/popup';
@@ -20,6 +20,14 @@ const TaskLists = React.memo(() => {
   const taskListIds = useSelector(selectors.selectTaskListIdsForCurrentCard);
 
   const dispatch = useDispatch();
+  const [completedVisibilities, setCompletedVisibilities] = useState({});
+
+  const handleCompletedVisibleToggle = useCallback((taskListId) => {
+    setCompletedVisibilities((prevCompletedVisibilities) => ({
+      ...prevCompletedVisibilities,
+      [taskListId]: !prevCompletedVisibilities[taskListId],
+    }));
+  }, []);
 
   const handleDragStart = useCallback(() => {
     document.body.classList.add(globalStyles.dragging);
@@ -45,16 +53,18 @@ const TaskLists = React.memo(() => {
           dispatch(entryActions.moveTaskList(id, destination.index));
 
           break;
-        case DroppableTypes.TASK:
-          dispatch(
-            entryActions.moveTask(id, parseDndId(destination.droppableId), destination.index),
-          );
+        case DroppableTypes.TASK: {
+          const taskListId = parseDndId(destination.droppableId);
+          const isCompletedVisible = !!completedVisibilities[taskListId];
+
+          dispatch(entryActions.moveTask(id, taskListId, destination.index, isCompletedVisible));
 
           break;
+        }
         default:
       }
     },
-    [dispatch],
+    [completedVisibilities, dispatch],
   );
 
   return (
@@ -64,7 +74,13 @@ const TaskLists = React.memo(() => {
           // eslint-disable-next-line react/jsx-props-no-spreading
           <div {...droppableProps} ref={innerRef}>
             {taskListIds.map((taskListId, index) => (
-              <Item key={taskListId} id={taskListId} index={index} />
+              <Item
+                key={taskListId}
+                id={taskListId}
+                index={index}
+                isCompletedVisible={!!completedVisibilities[taskListId]}
+                onCompletedVisibleToggle={handleCompletedVisibleToggle}
+              />
             ))}
             {placeholder}
           </div>

client/src/entry-actions/tasks.js

@@ -35,12 +35,13 @@ const handleTaskUpdate = (task) => ({
   },
 });
 
-const moveTask = (id, taskListId, index) => ({
+const moveTask = (id, taskListId, index, isCompletedVisible) => ({
   type: EntryActionTypes.TASK_MOVE,
   payload: {
     id,
     taskListId,
     index,
+    isCompletedVisible,
   },
 });
 

client/src/sagas/core/services/tasks.js

@@ -68,8 +68,14 @@ export function* handleTaskUpdate(task) {
   yield put(actions.handleTaskUpdate(task));
 }
 
-export function* moveTask(id, taskListId, index) {
-  const position = yield select(selectors.selectNextTaskPosition, taskListId, index, id);
+export function* moveTask(id, taskListId, index, isCompletedVisible) {
+  const position = yield select(
+    selectors.selectNextTaskPosition,
+    taskListId,
+    index,
+    id,
+    isCompletedVisible,
+  );
 
   yield call(updateTask, id, {
     taskListId,

client/src/sagas/core/watchers/tasks.js

@@ -22,8 +22,10 @@ export default function* tasksWatchers() {
     takeEvery(EntryActionTypes.TASK_UPDATE_HANDLE, ({ payload: { task } }) =>
       services.handleTaskUpdate(task),
     ),
-    takeEvery(EntryActionTypes.TASK_MOVE, ({ payload: { id, taskListId, index } }) =>
-      services.moveTask(id, taskListId, index),
+    takeEvery(
+      EntryActionTypes.TASK_MOVE,
+      ({ payload: { id, taskListId, index, isCompletedVisible } }) =>
+        services.moveTask(id, taskListId, index, isCompletedVisible),
     ),
     takeEvery(EntryActionTypes.TASK_DELETE, ({ payload: { id } }) => services.deleteTask(id)),
     takeEvery(EntryActionTypes.TASK_DELETE_HANDLE, ({ payload: { task } }) =>

client/src/selectors/positioning.js

@@ -116,14 +116,23 @@ export const selectNextTaskPosition = createSelector(
   (_, taskListId) => taskListId,
   (_, __, index) => index,
   (_, __, ___, excludedId) => excludedId,
-  ({ TaskList }, taskListId, index, excludedId) => {
+  (_, __, ___, ____, isCompletedVisible) => isCompletedVisible,
+  ({ TaskList }, taskListId, index, excludedId, isCompletedVisible) => {
     const taskListModel = TaskList.withId(taskListId);
 
     if (!taskListModel) {
       return taskListModel;
     }
 
-    return nextPosition(taskListModel.getTasksQuerySet().toRefArray(), index, excludedId);
+    let tasks = taskListModel.getTasksQuerySet().toRefArray();
+
+    // when an index is provided it refers to the rendered list, which excludes
+    // completed tasks if they are hidden, so the same filter must be applied here
+    if (!isUndefined(index) && taskListModel.hideCompletedTasks && !isCompletedVisible) {
+      tasks = tasks.filter((task) => !task.isCompleted);
+    }
+
+    return nextPosition(tasks, index, excludedId);
   },
 );