From 8615accb66ee2339701d036d61ad8c7aee6c77e9 Mon Sep 17 00:00:00 2001 From: crmaris <8526910+crmaris@users.noreply.github.com> Date: Thu, 2 Jul 2026 12:27:36 +0300 Subject: [PATCH] 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 --- .../cards/CardModal/TaskLists/Item.jsx | 11 ++++--- .../cards/CardModal/TaskLists/TaskLists.jsx | 30 ++++++++++++++----- client/src/entry-actions/tasks.js | 3 +- client/src/sagas/core/services/tasks.js | 10 +++++-- client/src/sagas/core/watchers/tasks.js | 6 ++-- client/src/selectors/positioning.js | 13 ++++++-- 6 files changed, 53 insertions(+), 20 deletions(-) diff --git a/client/src/components/cards/CardModal/TaskLists/Item.jsx b/client/src/components/cards/CardModal/TaskLists/Item.jsx index a93be777..2c39e22e 100644 --- a/client/src/components/cards/CardModal/TaskLists/Item.jsx +++ b/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; diff --git a/client/src/components/cards/CardModal/TaskLists/TaskLists.jsx b/client/src/components/cards/CardModal/TaskLists/TaskLists.jsx index 9a110321..1e1ec320 100644 --- a/client/src/components/cards/CardModal/TaskLists/TaskLists.jsx +++ b/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
{taskListIds.map((taskListId, index) => ( - + ))} {placeholder}
diff --git a/client/src/entry-actions/tasks.js b/client/src/entry-actions/tasks.js index c10159f7..9c05c337 100755 --- a/client/src/entry-actions/tasks.js +++ b/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, }, }); diff --git a/client/src/sagas/core/services/tasks.js b/client/src/sagas/core/services/tasks.js index a4dd0d8f..1e35c380 100644 --- a/client/src/sagas/core/services/tasks.js +++ b/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, diff --git a/client/src/sagas/core/watchers/tasks.js b/client/src/sagas/core/watchers/tasks.js index 875b50de..28396fee 100644 --- a/client/src/sagas/core/watchers/tasks.js +++ b/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 } }) => diff --git a/client/src/selectors/positioning.js b/client/src/selectors/positioning.js index 58e8f906..35b4434d 100755 --- a/client/src/selectors/positioning.js +++ b/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); }, );