Keep dragged cards visible during reactive moves

Preserve a presentation-only copy in the target list while sortable restores the source DOM and Blaze reconciles the database move. Remove it as soon as the real card arrives, on rejection, or at a safety timeout.

Add a browser regression with an intentionally delayed move so the visual gap is deterministic.

Thanks to mimZD and xet7 !

Fixes #6430,
This commit is contained in:
Lauri Ojansivu committed 2026-08-25 17:07:19 +03:00
1 parent 38b60fa193
commit b687f37272
4 files changed
+275 -44

No files matched your search

+5
View File
@@ -9,6 +9,11 @@
background: #ccc;
border-radius: 17px;
}
.minicard-wrapper.card-drop-preview {
background: transparent;
border-radius: 0;
pointer-events: none;
}
.minicard-wrapper.ui-sortable-helper {
cursor: grabbing;
transform: rotate(4deg);
+74 -23
View File
@@ -7,12 +7,16 @@ import {
suspendBoardDragscroll,
resumeBoardDragscroll,
} from '/client/lib/boardDragscroll';
import {
createCardDropPreview,
retireCardDropPreview,
} from '/client/lib/cardDropPreview';
import Cards from '/models/cards';
import {
isDegenerateSortGap,
computeRepairedDropIndex,
} from '/models/lib/cardSortRepair';
require('/client/lib/jquery-ui.js')
require('/client/lib/jquery-ui.js');
const { calculateIndex } = Utils;
@@ -187,7 +191,7 @@ function saveListWidth(list, width) {
// the per-list width, so EVERY list re-renders at the new width.
if (isFixedListWidth(boardId)) {
if (user) {
Meteor.call('setFixedListWidth', boardId, width, error => {
Meteor.call('setFixedListWidth', boardId, width, (error) => {
if (error) console.error('Error saving fixed list width:', error);
});
} else {
@@ -211,7 +215,7 @@ function saveListWidth(list, width) {
}
} else if (user) {
// Shared width: server also enforces board membership.
Meteor.call('applyListWidth', boardId, listId, width, width, error => {
Meteor.call('applyListWidth', boardId, listId, width, width, (error) => {
if (error) console.error('Error saving shared list width:', error);
});
}
@@ -237,9 +241,11 @@ Template.list.onCreated(function () {
// callback, we basically solve all issues related to reactive updates. A
// comment below provides further details.
Template.list.onRendered(function () {
const boardBodyEl = this.firstNode?.parentElement?.closest?.('.board-body') ||
const boardBodyEl =
this.firstNode?.parentElement?.closest?.('.board-body') ||
document.querySelector('.board-body');
const boardView = boardBodyEl && Blaze.getView(boardBodyEl, 'Template.boardBody');
const boardView =
boardBodyEl && Blaze.getView(boardBodyEl, 'Template.boardBody');
const boardComponent = boardView?.templateInstance?.();
// Initialize list resize functionality immediately
@@ -329,11 +335,21 @@ Template.list.onRendered(function () {
.children(itemsSelector)
.not(ui.item)
.toArray()
.map(el => Blaze.getData(el))
.map((el) => Blaze.getData(el))
.filter(Boolean);
}
const listData = Blaze.getData(ui.item.parents('.list').get(0));
const listId = listData._id;
const targetContainer = ui.item.parent().get(0);
const cardDomElement = ui.item.get(0);
const droppedCard = Blaze.getData(cardDomElement);
// #6430: sortable('cancel') is still necessary to keep jQuery UI from
// fighting Blaze, but it visibly returns the card to its source until a
// large board finishes its reactive flush. Leave a presentation-only
// clone in the drop slot until the real reactive card arrives.
const dropPreview = !MultiSelection.isActive()
? createCardDropPreview(cardDomElement, targetContainer)
: null;
const currentBoard = Utils.getCurrentBoard();
const defaultSwimlaneId = currentBoard.getDefaultSwimline()._id;
let targetSwimlaneId = null;
@@ -343,8 +359,9 @@ Template.list.onRendered(function () {
Utils.boardView() === 'board-view-swimlanes' ||
currentBoard.isTemplatesBoard()
) {
targetSwimlaneId = Blaze.getData(ui.item.parents('.swimlane').get(0))
._id;
targetSwimlaneId = Blaze.getData(
ui.item.parents('.swimlane').get(0),
)._id;
} else if (listData.swimlaneId) {
targetSwimlaneId = listData.swimlaneId;
}
@@ -371,14 +388,19 @@ Template.list.onRendered(function () {
nextCardData,
nCards,
);
repairedDrop.updates.forEach(u => {
repairedDrop.updates.forEach((u) => {
Cards.update(u._id, { $set: { sort: u.sort } });
});
sortIndex = { base: repairedDrop.base, increment: repairedDrop.increment };
sortIndex = {
base: repairedDrop.base,
increment: repairedDrop.increment,
};
}
if (MultiSelection.isActive()) {
ReactiveCache.getCards(MultiSelection.getMongoSelector(), { sort: ['sort'] }).forEach((card, i) => {
ReactiveCache.getCards(MultiSelection.getMongoSelector(), {
sort: ['sort'],
}).forEach((card, i) => {
const newSwimlaneId = targetSwimlaneId
? targetSwimlaneId
: card.swimlaneId || defaultSwimlaneId;
@@ -390,12 +412,22 @@ Template.list.onRendered(function () {
);
});
} else {
const cardDomElement = ui.item.get(0);
const card = Blaze.getData(cardDomElement);
const card = droppedCard;
const newSwimlaneId = targetSwimlaneId
? targetSwimlaneId
: card.swimlaneId || defaultSwimlaneId;
card.move(currentBoard._id, newSwimlaneId, listId, sortIndex.base);
const moveResult = card.move(
currentBoard._id,
newSwimlaneId,
listId,
sortIndex.base,
);
retireCardDropPreview(
dropPreview,
card._id,
targetContainer,
moveResult,
);
}
if (boardComponent) boardComponent.setIsDragging(false);
},
@@ -404,11 +436,20 @@ Template.list.onRendered(function () {
// overflow: auto), NOT .board-canvas, which only overflows vertically —
// the old code compared against a scrollLeftMax of 0 and never fired, so
// dragging a card toward an off-screen list never auto-scrolled the board.
const { computeEdgeScroll, findLaneUnderPointer } = require('/imports/lib/boardAutoScroll');
const {
computeEdgeScroll,
findLaneUnderPointer,
} = require('/imports/lib/boardAutoScroll');
let scrolled = false;
const lanes = document.querySelectorAll('.js-lists');
const rects = Array.prototype.map.call(lanes, el => el.getBoundingClientRect());
const laneIndex = findLaneUnderPointer(rects, event.clientX, event.clientY);
const rects = Array.prototype.map.call(lanes, (el) =>
el.getBoundingClientRect(),
);
const laneIndex = findLaneUnderPointer(
rects,
event.clientX,
event.clientY,
);
if (laneIndex !== -1) {
const nextLeft = computeEdgeScroll({
pointer: event.clientX,
@@ -442,11 +483,16 @@ Template.list.onRendered(function () {
// horizontal case above, which already picks the lane under the pointer.
let scrolledList = false;
const listBodies = document.querySelectorAll('.list-body');
const listRects = Array.prototype.map.call(listBodies, el =>
el.getBoundingClientRect());
const listRects = Array.prototype.map.call(listBodies, (el) =>
el.getBoundingClientRect(),
);
// findLaneUnderPointer is a plain rectangle hit-test; the name is about
// where it was first used, not what it can be given.
const listIndex = findLaneUnderPointer(listRects, event.clientX, event.clientY);
const listIndex = findLaneUnderPointer(
listRects,
event.clientX,
event.clientY,
);
if (listIndex !== -1) {
const nextListTop = computeEdgeScroll({
pointer: event.clientY,
@@ -462,7 +508,9 @@ Template.list.onRendered(function () {
scrolledList = true;
}
}
const canvas = scrolledList ? null : document.querySelector('.board-canvas');
const canvas = scrolledList
? null
: document.querySelector('.board-canvas');
if (canvas) {
const crect = canvas.getBoundingClientRect();
const nextTop = computeEdgeScroll({
@@ -621,7 +669,8 @@ Template.list.onCreated(function () {
const getEventPageX = (e) => {
const oe = e.originalEvent || e;
if (oe.touches && oe.touches.length) return oe.touches[0].pageX;
if (oe.changedTouches && oe.changedTouches.length) return oe.changedTouches[0].pageX;
if (oe.changedTouches && oe.changedTouches.length)
return oe.changedTouches[0].pageX;
return e.pageX;
};
@@ -707,7 +756,9 @@ Template.list.onCreated(function () {
// as the handler, throwing "handler.apply is not a function" on every
// touch. Use the native API so { passive: false } actually applies and
// preventDefault() works during the resize.
$resizeHandle[0].addEventListener('touchstart', startResize, { passive: false });
$resizeHandle[0].addEventListener('touchstart', startResize, {
passive: false,
});
document.addEventListener('touchmove', doResize, { passive: false });
document.addEventListener('touchend', stopResize, { passive: false });
+62
View File
@@ -0,0 +1,62 @@
// Keep a successful card drop visually stable while Blaze catches up with the
// optimistic Minimongo update. jQuery UI's required sortable('cancel') puts the
// real node back at its source first; on a large board the following Blaze flush
// can take long enough for that round trip to look like a one-second flicker.
export function createCardDropPreview(item, targetContainer) {
if (!item || !targetContainer) return null;
const preview = item.cloneNode(true);
preview.removeAttribute('id');
preview.removeAttribute('href');
preview.removeAttribute('style');
preview.setAttribute('aria-hidden', 'true');
preview.setAttribute('tabindex', '-1');
preview.classList.remove('ui-sortable-helper', 'ui-sortable-placeholder');
preview.classList.add('card-drop-preview', 'placeholder');
targetContainer.insertBefore(preview, item.nextSibling);
return preview;
}
export function retireCardDropPreview(
preview,
cardId,
targetContainer,
moveResult,
) {
if (!preview) return;
let observer;
let timeout;
const remove = () => {
if (observer) observer.disconnect();
if (timeout) clearTimeout(timeout);
if (preview.isConnected) preview.remove();
};
const realCardArrived = () => {
const cards = targetContainer.querySelectorAll(
`[data-card-id="${CSS.escape(cardId)}"]`,
);
if (
Array.from(cards).some(
(card) =>
card !== preview && !card.classList.contains('card-drop-preview'),
)
) {
remove();
return true;
}
return false;
};
if (!realCardArrived()) {
observer = new MutationObserver(realCardArrived);
observer.observe(targetContainer, { childList: true });
// Never leave stale UI behind if a rejected write or lost connection does
// not produce the expected reactive card.
timeout = setTimeout(remove, 10_000);
}
Promise.resolve(moveResult).catch(remove);
}
+134 -21
View File
@@ -11,6 +11,8 @@
* persist — it was reported that such cards "go back to their place".
* Verified here that the new order is saved both for a few cards and at
* scale.
* - #6430: cancelling jQuery UI's DOM move must leave a visual card in the drop
* slot until Blaze renders the real card there.
*/
const { test, expect } = require('../fixtures');
@@ -22,10 +24,71 @@ async function dbOrder(boardId, listId) {
return db
.find('cards', { boardId, listId }, { title: 1, sort: 1 })
.sort((a, b) => a.sort - b.sort)
.map(c => c.title);
.map((c) => c.title);
}
test.describe('Card drag-sort reordering', () => {
test('#6430 a cross-list drop never leaves its target visually empty', async ({
loggedInPage,
user,
browserName,
}) => {
test.skip(
browserName !== 'chromium',
'drag-sort harness validated on Chromium',
);
const board = db.seedBoard({
ownerId: user.id,
title: 'DragNoFlicker',
listCount: 2,
cardTitlesPerList: [['Moving Card'], []],
});
try {
await openBoard(loggedInPage, board.boardId, board.slug);
await waitForMeteor(loggedInPage);
const source = loggedInPage
.locator('.js-minicard')
.filter({ hasText: 'Moving Card' })
.first();
const target = loggedInPage.locator(
`#js-list-${board.listIds[1]} .js-minicards`,
);
await source.waitFor({ timeout: 30_000 });
// Make the reconciliation gap deterministic. The object attached to the
// rendered source node is the same object list.js reads in sortable's
// stop callback, so delaying this method emulates a costly large-board
// flush without changing production timing.
await source.evaluate((element) => {
const card = Blaze.getData(element);
const move = card.move.bind(card);
card.move = (...args) =>
new Promise((resolve, reject) => {
setTimeout(() => move(...args).then(resolve, reject), 750);
});
});
await dragCardOnto(loggedInPage, source, target, { place: 'center' });
// At all times after mouse-up the target contains either the temporary
// preview or the real reactive card. The old cancel-then-render sequence
// left it empty for the duration of a large board's Blaze flush.
await expect(target.locator('[data-card-id]')).toHaveCount(1);
await expect(target.locator('.card-drop-preview')).toHaveCount(1);
await expect
.poll(
() => target.locator('.js-minicard:not(.card-drop-preview)').count(),
{
timeout: 15_000,
},
)
.toBe(1);
await expect(target.locator('.card-drop-preview')).toHaveCount(0);
} finally {
db.cleanup({ boardIds: [board.boardId] });
}
});
test('#3826 cards that have a parent can be reordered and the new order persists', async ({
loggedInPage,
user,
@@ -34,35 +97,60 @@ test.describe('Card drag-sort reordering', () => {
// The reorder logic is browser-independent; the jQuery-UI drag-sort harness
// is validated on Chromium. Scoped to Chromium so cross-browser drag-timing
// differences cannot flake the otherwise-green matrix.
test.skip(browserName !== 'chromium', 'drag-sort harness validated on Chromium');
test.skip(
browserName !== 'chromium',
'drag-sort harness validated on Chromium',
);
// list 0 = parents, list 1 = sub-tasks (each given a parentId).
const board = db.seedBoard({
ownerId: user.id,
title: 'DragSort',
listCount: 2,
cardTitlesPerList: [['Parent P'], ['A Card', 'B Card', 'C Card', 'D Card']],
cardTitlesPerList: [
['Parent P'],
['A Card', 'B Card', 'C Card', 'D Card'],
],
});
try {
const parentId = db.findOne('cards', { boardId: board.boardId, title: 'Parent P' }, { _id: 1 })._id;
const parentId = db.findOne(
'cards',
{ boardId: board.boardId, title: 'Parent P' },
{ _id: 1 },
)._id;
const subListId = board.listIds[1];
db.updateMany('cards', { boardId: board.boardId, listId: subListId }, { $set: { parentId } });
db.updateMany(
'cards',
{ boardId: board.boardId, listId: subListId },
{ $set: { parentId } },
);
await openBoard(loggedInPage, board.boardId, board.slug);
await waitForMeteor(loggedInPage);
const list = `#js-list-${subListId}`;
await loggedInPage.locator(`${list} .js-minicard`).first().waitFor({ timeout: 30_000 });
await loggedInPage
.locator(`${list} .js-minicard`)
.first()
.waitFor({ timeout: 30_000 });
// Drag the last sub-task card to the front of the list.
await dragCardOnto(
loggedInPage,
loggedInPage.locator(`${list} .js-minicard`).filter({ hasText: 'D Card' }).first(),
loggedInPage.locator(`${list} .js-minicard`).filter({ hasText: 'A Card' }).first(),
loggedInPage
.locator(`${list} .js-minicard`)
.filter({ hasText: 'D Card' })
.first(),
loggedInPage
.locator(`${list} .js-minicard`)
.filter({ hasText: 'A Card' })
.first(),
{ place: 'before' },
);
// The reorder must be saved (it used to revert for parented cards).
await expect
.poll(async () => (await dbOrder(board.boardId, subListId))[0], { timeout: 15_000 })
.poll(async () => (await dbOrder(board.boardId, subListId))[0], {
timeout: 15_000,
})
.toBe('D Card');
} finally {
db.cleanup({ boardIds: [board.boardId] });
@@ -91,7 +179,10 @@ test.describe('Card drag-sort reordering', () => {
user,
browserName,
}) => {
test.skip(browserName !== 'chromium', 'drag-sort harness validated on Chromium');
test.skip(
browserName !== 'chromium',
'drag-sort harness validated on Chromium',
);
// Enough lists that the lane overflows horizontally, which is the board the
// report is about, and cards in several of them so one can be found away
// from the edges.
@@ -99,12 +190,20 @@ test.describe('Card drag-sort reordering', () => {
ownerId: user.id,
title: 'DragNoPan',
listCount: 12,
cardTitlesPerList: [['One', 'Two'], ['Three', 'Four'], ['Five', 'Six'], ['Seven', 'Eight']],
cardTitlesPerList: [
['One', 'Two'],
['Three', 'Four'],
['Five', 'Six'],
['Seven', 'Eight'],
],
});
try {
await openBoard(loggedInPage, board.boardId, board.slug);
await waitForMeteor(loggedInPage);
await loggedInPage.locator('.js-minicard').first().waitFor({ timeout: 30_000 });
await loggedInPage
.locator('.js-minicard')
.first()
.waitFor({ timeout: 30_000 });
// Pan the lane away from its left edge first, so a pan in EITHER direction
// during the drag is visible as a change (scrollLeft cannot go below 0).
@@ -112,7 +211,10 @@ test.describe('Card drag-sort reordering', () => {
const lane = document.querySelector('.js-lists');
const canvas = document.querySelector('.board-canvas');
if (lane) lane.scrollLeft = 120;
return { lane: lane ? lane.scrollLeft : null, canvas: canvas ? canvas.scrollTop : null };
return {
lane: lane ? lane.scrollLeft : null,
canvas: canvas ? canvas.scrollTop : null,
};
});
expect(startScroll.lane).toBeGreaterThan(0);
@@ -120,30 +222,41 @@ test.describe('Card drag-sort reordering', () => {
// MARGIN px inside the lane and the canvas, so the edge auto-scroll never
// fires and any movement of the scroll positions is panning.
const MARGIN = 70;
const plan = await loggedInPage.evaluate(margin => {
const plan = await loggedInPage.evaluate((margin) => {
const lane = document.querySelector('.js-lists');
const canvas = document.querySelector('.board-canvas');
if (!lane || !canvas) return null;
const l = lane.getBoundingClientRect();
const c = canvas.getBoundingClientRect();
const box = { left: l.left + margin, right: l.right - margin,
top: Math.max(l.top, c.top) + margin,
bottom: Math.min(l.bottom, c.bottom) - margin };
const box = {
left: l.left + margin,
right: l.right - margin,
top: Math.max(l.top, c.top) + margin,
bottom: Math.min(l.bottom, c.bottom) - margin,
};
let best = null;
for (const el of document.querySelectorAll('.js-minicard')) {
const r = el.getBoundingClientRect();
const x = r.left + r.width / 2;
const y = r.top + 12;
const room = Math.min(x - box.left, box.right - x, y - box.top, box.bottom - y);
if (room > 0 && (!best || room > best.room)) best = { x, y, room, box };
const room = Math.min(
x - box.left,
box.right - x,
y - box.top,
box.bottom - y,
);
if (room > 0 && (!best || room > best.room))
best = { x, y, room, box };
}
return best;
}, MARGIN);
// A window too small to hold a drag that stays clear of every edge would
// measure the auto-scroll instead of panning, and would say nothing about
// the bug. Better skipped than misleading.
test.skip(!plan || plan.room < 30,
'no card sits far enough from the lane/canvas edges in this viewport');
test.skip(
!plan || plan.room < 30,
'no card sits far enough from the lane/canvas edges in this viewport',
);
// A short diagonal, entirely inside the safe box: the gesture that used to
// drag the card and pan the lane at the same time.