From 66b3ef1bd8961e3eb9969faa21f2ea311b5855ca Mon Sep 17 00:00:00 2001 From: ac8318740 <171465680+ac8318740@users.noreply.github.com> Date: Thu, 26 Feb 2026 04:48:52 +0000 Subject: [PATCH] fix(drag-reorder): fix LexoRank bugs in filtered views and near-ceiling ranks Several interrelated drag-to-reorder bugs fixed: - computeSortOrder() now scans the full vault (ignoring swimlane filters) so invisible tasks in filtered views aren't leapfrogged during reorder - Filter out tasks with undefined or non-LexoRank sort_orders (legacy numeric timestamps) from neighbor lookup to prevent rankBetween() from producing garbage values via ensureRank() fallbacks - Fix safeGenNext() overflow: values starting with 'z' (near ceiling) now extend via the decimal part instead of appending an integer digit, which caused LexoRank.between() to wrap around - Add sanity check in rankBetween() to detect out-of-range results from LexoRank.between() and fall back to safeGenNext() - Reconstruct dropTarget from visible cards when column-level drop fires with null target (user drops in empty space below last card) - Consolidate group/swimlane/sort_order into single atomic frontmatter write per task to prevent interleaved corruption - Add DropOperationQueue to serialize rapid drops on the same task file - Add optimistic DOM reorder on drop for instant visual feedback - Add post-drop render suppression to prevent stale Bases data from overwriting optimistic positions - Add esbuild copy-to-vault plugin for faster dev iteration - Add .serena/ to .gitignore --- .gitignore | 11 +- esbuild.config.mjs | 41 ++- src/bases/KanbanView.ts | 537 +++++++++++++++++++++++++++--------- src/bases/TaskListView.ts | 85 +++--- src/bases/sortOrderUtils.ts | 225 +++++++++++++-- 5 files changed, 695 insertions(+), 204 deletions(-) diff --git a/.gitignore b/.gitignore index 7f8a97d1..e5f16712 100644 --- a/.gitignore +++ b/.gitignore @@ -11,11 +11,11 @@ node_modules # Environment variables (contains OAuth client IDs) .env -# Don't include the compiled main.js file in the repo. -# They should be uploaded to GitHub releases instead. -main.js -# Keep docs site source JS tracked. -!docs-builder/src/js/main.js +# Don't include the compiled main.js file in the repo. +# They should be uploaded to GitHub releases instead. +main.js +# Keep docs site source JS tracked. +!docs-builder/src/js/main.js # Don't include the generated styles.css file in the repo. # It's built from source files in styles/ directory during CI/build process. @@ -86,3 +86,4 @@ tasknotes-e2e-vault/*.ics AGENTS.md /docs-builder/dist .ops/ +.serena/ diff --git a/esbuild.config.mjs b/esbuild.config.mjs index efd5f576..42c80241 100644 --- a/esbuild.config.mjs +++ b/esbuild.config.mjs @@ -1,7 +1,9 @@ import esbuild from "esbuild"; import process from "process"; import builtins from "builtin-modules"; -import { readFileSync } from "fs"; +import { readFileSync, existsSync, copyFileSync, mkdirSync } from "fs"; +import { resolve, join } from "path"; +import { homedir } from "os"; const banner = `/* @@ -12,6 +14,41 @@ if you want to view the source, please visit the github repository of this plugi const prod = (process.argv[2] === "production"); +// Read copy destinations from .copy-files.local (same logic as copy-files.mjs) +function getCopyDestinations() { + const localFile = resolve(process.cwd(), '.copy-files.local'); + if (!existsSync(localFile)) return []; + const expandTilde = (p) => p.startsWith('~/') ? join(homedir(), p.slice(2)) : p; + return readFileSync(localFile, 'utf8') + .split('\n') + .map(p => p.trim()) + .filter(p => p && !p.startsWith('#')) + .map(expandTilde); +} + +// esbuild plugin: copy build outputs to vault after each rebuild +const copyToVaultPlugin = { + name: 'copy-to-vault', + setup(build) { + const destinations = getCopyDestinations(); + if (destinations.length === 0) return; + const filesToCopy = ['main.js', 'styles.css', 'manifest.json']; + build.onEnd((result) => { + if (result.errors.length > 0) return; + for (const dest of destinations) { + mkdirSync(dest, { recursive: true }); + for (const file of filesToCopy) { + const src = resolve(process.cwd(), file); + if (existsSync(src)) { + copyFileSync(src, join(dest, file)); + } + } + console.log(`\x1b[32m✓ Copied to ${dest}\x1b[0m`); + } + }); + } +}; + // Plugin to import markdown files as strings const markdownPlugin = { name: 'markdown', @@ -54,7 +91,7 @@ const context = await esbuild.context({ treeShaking: true, outfile: "main.js", minify: prod, - plugins: [markdownPlugin], + plugins: [markdownPlugin, ...(!prod ? [copyToVaultPlugin] : [])], }); if (prod) { diff --git a/src/bases/KanbanView.ts b/src/bases/KanbanView.ts index 54db0c90..3d70b501 100644 --- a/src/bases/KanbanView.ts +++ b/src/bases/KanbanView.ts @@ -17,6 +17,8 @@ import { stripPropertyPrefix, isSortOrderInSortConfig, computeSortOrder, + generateEndRank, + DropOperationQueue, } from "./sortOrderUtils"; export class KanbanView extends BasesViewBase { @@ -36,12 +38,17 @@ export class KanbanView extends BasesViewBase { private dragContainer: HTMLElement | null = null; // Container holding siblings during drag private currentInsertionIndex: number = -1; // Current gap/slot position private dragSourceColumnEl: HTMLElement | null = null; // Source column element (height-locked during drag) + private dragTargetColumnEl: HTMLElement | null = null; // Target column element (max-height expanded during drag) private draggedSourceColumns: Map = new Map(); // Track source column per task for batch operations private draggedSourceSwimlanes: Map = new Map(); // Track source swimlane per task for batch operations private taskInfoCache = new Map(); private columnTasksCache = new Map(); private containerListenersRegistered = false; private columnScrollers = new Map>(); // columnKey -> scroller + private suppressRenderUntil: number = 0; + private postDropTimer: number | null = null; + private dropQueue = new DropOperationQueue(); + private activeDropCount = 0; // Touch drag state for mobile private touchDragActive = false; @@ -102,26 +109,37 @@ export class KanbanView extends BasesViewBase { * Override to preserve scroll position during re-renders. */ onDataUpdated(): void { - // Defer re-render while a drag is in progress — re-rendering - // destroys column/card elements and their event listeners, which - // causes the drop event to never fire. + // During drag: defer render (destroying DOM kills drop events) if (this.draggedTaskPath) { + this.debugLog("ON-DATA-UPDATED: deferred (drag active)", { draggedTask: this.draggedTaskPath.split("/").pop() }); this.pendingRender = true; - // Silently defer — don't use Notice here (fires during drag) return; } - // Save scroll state before re-render - const savedState = this.getEphemeralState(); + // Post-drop suppression: skip renders until metadataCache has settled. + // postDropTimer will fire the guaranteed render. + if (this.activeDropCount > 0 || Date.now() < this.suppressRenderUntil) { + this.debugLog("ON-DATA-UPDATED: suppressed", { activeDropCount: this.activeDropCount, msRemaining: this.suppressRenderUntil - Date.now() }); + return; + } + // If we're past the suppression window and Bases fires naturally, + // cancel postDropTimer — Bases has fresh data, render now. + if (this.postDropTimer) { + this.debugLog("ON-DATA-UPDATED: cancelling postDropTimer, rendering with fresh Bases data"); + clearTimeout(this.postDropTimer); + this.postDropTimer = null; + } else { + this.debugLog("ON-DATA-UPDATED: normal render (no suppression active)"); + } + + const savedState = this.getEphemeralState(); try { this.render(); } catch (error) { console.error(`[TaskNotes][${this.type}] Render error:`, error); this.renderError(error as Error); } - - // Restore scroll state after render this.setEphemeralState(savedState); } @@ -445,6 +463,28 @@ export class KanbanView extends BasesViewBase { } } + // Re-sort each group by sort_order from live metadata cache + // (Bases' internal data may be stale after a drag-to-reorder write) + const sortOrderField = this.plugin.settings.fieldMapping.sortOrder; + if (isSortOrderInSortConfig(this.dataAdapter, sortOrderField)) { + for (const [, tasks] of groups) { + tasks.sort((a, b) => { + const fmA = this.plugin.app.metadataCache.getFileCache( + this.plugin.app.vault.getAbstractFileByPath(a.path) as TFile + )?.frontmatter; + const fmB = this.plugin.app.metadataCache.getFileCache( + this.plugin.app.vault.getAbstractFileByPath(b.path) as TFile + )?.frontmatter; + const soA = fmA?.[sortOrderField]; + const soB = fmB?.[sortOrderField]; + if (soA != null && soB != null) return String(soA).localeCompare(String(soB)); + if (soA != null) return -1; + if (soB != null) return 1; + return 0; + }); + } + } + // Augment with empty status columns if grouping by status this.augmentWithEmptyStatusColumns(groups, groupByPropertyId); @@ -603,8 +643,8 @@ export class KanbanView extends BasesViewBase { // Render columns without swimlanes const visibleProperties = this.getVisibleProperties(); - // Note: tasks are already sorted by Bases within each group - // Bases handles sort_order sorting when configured in the .base sort config + // Tasks are re-sorted by sort_order in groupTasks() when configured, + // ensuring correct order even if Bases' internal data hasn't refreshed yet. // Get groupBy property ID const groupByPropertyId = this.getGroupByPropertyId(); @@ -1237,22 +1277,79 @@ export class KanbanView extends BasesViewBase { if (e.dataTransfer?.types.includes("text/x-kanban-column")) return; e.preventDefault(); e.stopPropagation(); - column.classList.remove("kanban-view__column--dragover"); - this.cleanupDragShift(); - if (!this.draggedTaskPath) return; + if (!this.draggedTaskPath) { + column.classList.remove("kanban-view__column--dragover"); + this.cleanupDragShift(); + return; + } - // Capture drop position before clearing state - const dropTarget = this.dropTargetPath + // Capture drop position + let dropTarget = this.dropTargetPath ? { taskPath: this.dropTargetPath, above: this.dropAbove } : undefined; + // For cross-column drops, the dropTarget may reference a card from + // the source column (stale from last dragover). Validate that the + // target card actually exists in the target column via DOM query. + const cardsContainer = column.querySelector(".kanban-view__cards") as HTMLElement | null; + const isCrossColumn = this.draggedFromColumn !== groupKey; + if (isCrossColumn && dropTarget) { + const targetInColumn = cardsContainer?.querySelector(`[data-task-path="${CSS.escape(dropTarget.taskPath)}"]`) != null; + if (!targetInColumn) { + // Drop target is stale (from source column). Clear it — + // handleTaskDrop will append to end of target column. + dropTarget = undefined; + } + } + + // Same-column fallback: when dropTarget is null (e.g. user dropped + // in empty space below the last card where the card-level dragover + // never fired), reconstruct the drop target from the column's + // visible non-dragged cards. + if (!dropTarget && !isCrossColumn && cardsContainer) { + const visibleCards = Array.from( + cardsContainer.querySelectorAll(".kanban-view__card-wrapper") + ).filter(el => !this.draggedTaskPaths.includes(el.dataset.taskPath || "")); + + if (visibleCards.length > 0) { + // Use currentInsertionIndex if available; otherwise + // default to after the last visible card. + const idx = this.currentInsertionIndex >= 0 + ? Math.min(this.currentInsertionIndex, visibleCards.length) + : visibleCards.length; + + if (idx === 0) { + // Before the first visible card + dropTarget = { taskPath: visibleCards[0].dataset.taskPath!, above: true }; + } else { + // After the card at idx-1 + dropTarget = { taskPath: visibleCards[idx - 1].dataset.taskPath!, above: false }; + } + } + } + + this.debugLog("COLUMN-DROP", { + draggedTask: this.draggedTaskPath?.split("/").pop(), + sourceColumn: this.draggedFromColumn, + targetColumn: groupKey, + isCrossColumn, + dropTarget: dropTarget ? { file: dropTarget.taskPath.split("/").pop(), above: dropTarget.above } : null, + }); + + // Optimistic DOM reorder: move card to correct position immediately + const paths = this.draggedTaskPaths.length > 0 ? this.draggedTaskPaths : [this.draggedTaskPath!]; + this.performOptimisticReorder(paths, dropTarget, cardsContainer); + + // Now clean up shift CSS — no visual change since DOM is already correct + column.classList.remove("kanban-view__column--dragover"); + this.cleanupDragShift(); + // Update the task's groupBy property in Bases await this.handleTaskDrop(this.draggedTaskPath, groupKey, null, dropTarget); this.draggedTaskPath = null; this.draggedFromColumn = null; - this.dropTargetPath = null; }); // Drag end handler - cleanup in case drop doesn't fire @@ -1290,22 +1387,42 @@ export class KanbanView extends BasesViewBase { cell.addEventListener("drop", async (e: DragEvent) => { e.preventDefault(); e.stopPropagation(); - cell.classList.remove("kanban-view__swimlane-column--dragover"); - this.cleanupDragShift(); - if (!this.draggedTaskPath) return; + if (!this.draggedTaskPath) { + cell.classList.remove("kanban-view__swimlane-column--dragover"); + this.cleanupDragShift(); + return; + } - // Capture drop position before clearing state - const dropTarget = this.dropTargetPath + // Capture drop position + let dropTarget = this.dropTargetPath ? { taskPath: this.dropTargetPath, above: this.dropAbove } : undefined; + // For cross-column/swimlane drops, validate dropTarget is in this cell via DOM query + const cardsContainer = cell.querySelector(".kanban-view__cards") as HTMLElement | null; + const isCrossColumn = this.draggedFromColumn !== columnKey; + const isCrossSwimlane = this.draggedFromSwimlane !== swimLaneKey; + if ((isCrossColumn || isCrossSwimlane) && dropTarget) { + const targetInCell = cardsContainer?.querySelector(`[data-task-path="${CSS.escape(dropTarget.taskPath)}"]`) != null; + if (!targetInCell) { + dropTarget = undefined; + } + } + + // Optimistic DOM reorder: move card to correct position immediately + const paths = this.draggedTaskPaths.length > 0 ? this.draggedTaskPaths : [this.draggedTaskPath!]; + this.performOptimisticReorder(paths, dropTarget, cardsContainer); + + // Now clean up shift CSS — no visual change since DOM is already correct + cell.classList.remove("kanban-view__swimlane-column--dragover"); + this.cleanupDragShift(); + // Update both the groupBy property and swimlane property await this.handleTaskDrop(this.draggedTaskPath, columnKey, swimLaneKey, dropTarget); this.draggedTaskPath = null; this.draggedFromColumn = null; - this.dropTargetPath = null; }); // Drag end handler - cleanup in case drop doesn't fire @@ -1559,9 +1676,19 @@ export class KanbanView extends BasesViewBase { const totalGap = draggedHeight + gap; container.style.setProperty("--tn-drag-gap", `${totalGap}px`); container.style.overflowY = "clip"; - // Add padding to grow the container's content box so - // translateY-shifted cards aren't clipped off the bottom + // Padding grows the container's content box so + // translateY-shifted cards have real layout space. + // Also bump the column's max-height so it can grow + // to accommodate the taller container. container.style.paddingBottom = `${totalGap}px`; + const parentCol = container.closest( + ".kanban-view__column, .kanban-view__swimlane-column" + ); + if (parentCol) { + const currentHeight = parentCol.getBoundingClientRect().height; + parentCol.style.maxHeight = `${currentHeight + totalGap}px`; + this.dragTargetColumnEl = parentCol; + } const siblings = container.querySelectorAll(".kanban-view__card-wrapper"); for (const sib of siblings) { @@ -1631,7 +1758,23 @@ export class KanbanView extends BasesViewBase { above: this.dropAbove }; - // Clean up visual indicators + const container = cardWrapper.parentElement; + + this.debugLog("CARD-DROP", { + draggedTask: this.draggedTaskPath?.split("/").pop(), + targetCard: task.path.split("/").pop(), + sourceColumn: this.draggedFromColumn, + targetColumn: groupKey, + isCrossColumn: this.draggedFromColumn !== groupKey, + above: this.dropAbove, + swimLaneKey, + }); + + // Optimistic DOM reorder: move card to correct position immediately + const paths = this.draggedTaskPaths.length > 0 ? this.draggedTaskPaths : [this.draggedTaskPath!]; + this.performOptimisticReorder(paths, dropTarget); + + // Now clean up shift CSS — no visual change since DOM is already correct this.cleanupDragShift(); col?.classList.remove("kanban-view__column--dragover"); @@ -1639,7 +1782,6 @@ export class KanbanView extends BasesViewBase { this.draggedTaskPath = null; this.draggedFromColumn = null; - this.dropTargetPath = null; }); cardWrapper.addEventListener("dragstart", (e: DragEvent) => { @@ -1773,6 +1915,10 @@ export class KanbanView extends BasesViewBase { } this.containerEl.ownerDocument.body.classList.remove("tn-drag-active"); + // Clear drag state. The drop handler snapshots dropTargetPath and + // clears it before its first await, so this is safe even if + // handleTaskDrop is still running. + this.draggedTaskPath = null; this.draggedFromColumn = null; this.draggedFromSwimlane = null; this.draggedTaskPaths = []; @@ -1800,19 +1946,62 @@ export class KanbanView extends BasesViewBase { // Flush any render that was deferred while dragging. // Use a short delay so the async drop handler can finish first. if (this.pendingRender) { - const win = this.containerEl.ownerDocument.defaultView || window; - win.setTimeout(() => { - if (this.pendingRender) { - this.pendingRender = false; - this.debouncedRefresh(); - } - }, 200); + this.pendingRender = false; + this.debouncedRefresh(); } }); this.setupCardTouchHandlers(cardWrapper, task); } + /** + * Move dragged card(s) to the correct DOM position immediately after drop, + * before CSS shift classes are removed. This prevents the visual flash + * where cards snap to their original position before the re-render. + * Returns false if optimistic reorder could not be performed (e.g. virtual-scrolled column). + */ + private performOptimisticReorder( + draggedPaths: string[], + dropTarget: { taskPath: string; above: boolean } | undefined, + targetContainer?: HTMLElement | null + ): boolean { + if (!dropTarget || draggedPaths.length === 0) return false; + + const targetEl = this.currentTaskElements.get(dropTarget.taskPath); + if (!targetEl) return false; + + const container = targetContainer || targetEl.parentElement; + if (!container) return false; + + // If the target element is not in the container (e.g. cross-column drop + // where dropTarget references a card in the source column), bail out + // gracefully — the post-drop render will fix the DOM. + if (!container.contains(targetEl)) return false; + + for (const path of draggedPaths) { + const draggedEl = this.currentTaskElements.get(path); + if (!draggedEl) return false; // Virtual-scrolled column — can't do optimistic reorder + + // Restore visibility (undo the dragstart collapse) + draggedEl.style.cssText = ""; + draggedEl.classList.remove("kanban-view__card--dragging"); + + // Move to correct DOM position + if (dropTarget.above) { + container.insertBefore(draggedEl, targetEl); + } else { + container.insertBefore(draggedEl, targetEl.nextSibling); + } + } + return true; + } + + /** + * Extract the current visual task order from a cards container's DOM children. + * Returns TaskInfo[] in display order with fresh sort_order values from metadataCache. + */ + + /** * Remove all gap/slot shift classes and custom properties from the current * drag container (and any stale containers from cross-column drags). @@ -1832,6 +2021,12 @@ export class KanbanView extends BasesViewBase { this.dragContainer = null; } + // Reset target column max-height + if (this.dragTargetColumnEl) { + this.dragTargetColumnEl.style.maxHeight = ""; + this.dragTargetColumnEl = null; + } + // Also clean any wrappers on the entire board (safety net for cross-column) this.boardEl?.querySelectorAll( ".kanban-view__card-wrapper--drag-shift, .kanban-view__card-wrapper--shift-down" @@ -1961,7 +2156,14 @@ export class KanbanView extends BasesViewBase { newSwimLaneValue: string | null, dropTarget?: { taskPath: string; above: boolean } ): Promise { + this.activeDropCount++; try { + await this.dropQueue.enqueue(taskPath, async () => { + // Suppress renders immediately — dragend clears draggedTaskPath + // during our awaits, so onDataUpdated needs another way to know + // not to render with stale data. + this.suppressRenderUntil = Date.now() + 10000; // extended window, tightened at end + // Get the groupBy property from the controller const groupByPropertyId = this.getGroupByPropertyId(); if (!groupByPropertyId) return; @@ -1994,75 +2196,72 @@ export class KanbanView extends BasesViewBase { : null; const isSwimlaneListProperty = cleanSwimlane && this.isListTypeProperty(cleanSwimlane); + // Snapshot drag state NOW — dragend fires during our awaits and + // clears these instance properties out from under us. + const snapshotFromColumn = this.draggedFromColumn; + const snapshotFromSwimlane = this.draggedFromSwimlane; + const snapshotSourceColumns = new Map(this.draggedSourceColumns); + const snapshotSourceSwimlanes = new Map(this.draggedSourceSwimlanes); + // Handle batch drag - update all dragged tasks const pathsToUpdate = - this.draggedTaskPaths.length > 1 ? this.draggedTaskPaths : [taskPath]; + this.draggedTaskPaths.length > 1 ? [...this.draggedTaskPaths] : [taskPath]; const isBatchOperation = pathsToUpdate.length > 1; + // Pre-compute sort_order related state + const hasSortOrder = isSortOrderInSortConfig(this.dataAdapter, this.plugin.settings.fieldMapping.sortOrder); + const sortOrderField = this.plugin.settings.fieldMapping.sortOrder; + const cleanGroupByForSort = stripPropertyPrefix(groupByPropertyId); + const cleanSwimLaneForSort = this.swimLanePropertyId + ? stripPropertyPrefix(this.swimLanePropertyId) : null; + + this.debugLog("SORT-ORDER-CHECK", { + hasDropTarget: !!dropTarget, + hasSortOrder, + dropTarget: dropTarget ? { file: dropTarget.taskPath.split("/").pop(), above: dropTarget.above } : null, + }); + for (const path of pathsToUpdate) { // Get the source column and swimlane for this specific task const sourceColumn = isBatchOperation - ? this.draggedSourceColumns.get(path) - : this.draggedFromColumn; + ? snapshotSourceColumns.get(path) + : snapshotFromColumn; const sourceSwimlane = isBatchOperation - ? this.draggedSourceSwimlanes.get(path) - : this.draggedFromSwimlane; + ? snapshotSourceSwimlanes.get(path) + : snapshotFromSwimlane; // Detect same-column drop — skip group property update to avoid // unnecessary writes or value corruption const isSameColumn = sourceColumn === newGroupValue; - - // Update groupBy property (only if changing columns) - if (!isSameColumn) { - if (isGroupByListProperty && sourceColumn) { - // For list properties, remove the source value and add the target value - await this.updateListPropertyOnDrop( - path, - groupByPropertyId, - sourceColumn, - newGroupValue - ); - } else { - // For non-list properties, simply replace the value - await this.updateTaskFrontmatterProperty( - path, - groupByPropertyId, - newGroupValue - ); - } - } - - // Update swimlane property if applicable (only if changing swimlanes) const isSameSwimlane = sourceSwimlane === newSwimLaneValue; - if (newSwimLaneValue !== null && this.swimLanePropertyId && !isSameSwimlane) { - if (isSwimlaneListProperty && sourceSwimlane) { - // For list swimlane properties, remove source and add target - await this.updateListPropertyOnDrop( - path, - this.swimLanePropertyId, - sourceSwimlane, - newSwimLaneValue - ); - } else { - // For non-list swimlane properties, simply replace the value - await this.updateTaskFrontmatterProperty( - path, - this.swimLanePropertyId, - newSwimLaneValue - ); - } - } - } - // Compute and write sort_order if we have a drop position - // and the view's sort config includes sort_order - const hasSortOrder = isSortOrderInSortConfig(this.dataAdapter, this.plugin.settings.fieldMapping.sortOrder); - if (dropTarget && hasSortOrder) { - const cleanGroupByForSort = stripPropertyPrefix(groupByPropertyId); - const cleanSwimLaneForSort = this.swimLanePropertyId - ? stripPropertyPrefix(this.swimLanePropertyId) : null; - for (const path of pathsToUpdate) { - const newSortOrder = await computeSortOrder( + this.debugLog("HANDLE-DROP-TASK", { + taskFile: path.split("/").pop(), + sourceColumn, + newGroupValue, + isSameColumn, + isGroupByListProperty, + sourceSwimlane, + newSwimLaneValue, + }); + + const needsGroupUpdate = !isSameColumn; + const needsSwimlaneUpdate = newSwimLaneValue !== null && !!this.swimLanePropertyId && !isSameSwimlane; + + // Compute sort_order first (read-only — no file writes yet) + let newSortOrder: string | null = null; + if (hasSortOrder) { + if (dropTarget) { + this.debugLog("COMPUTE-SORT-ORDER-CALL", { + taskFile: path.split("/").pop(), + targetFile: dropTarget.taskPath.split("/").pop(), + above: dropTarget.above, + groupKey: newGroupValue, + cleanGroupBy: cleanGroupByForSort, + cleanSwimLane: cleanSwimLaneForSort, + }); + + newSortOrder = await computeSortOrder( dropTarget.taskPath, dropTarget.above, newGroupValue, @@ -2070,19 +2269,85 @@ export class KanbanView extends BasesViewBase { path, this.plugin, this.taskInfoCache, - newSwimLaneValue, - cleanSwimLaneForSort + (msg, data) => this.debugLog(msg, data) ); - if (newSortOrder !== null) { - const sortOrderField = this.plugin.settings.fieldMapping.sortOrder; - const file = this.plugin.app.vault.getAbstractFileByPath(path); - if (file && file instanceof TFile) { - await this.plugin.app.fileManager.processFrontMatter(file, (frontmatter) => { - frontmatter[sortOrderField] = newSortOrder; - }); + } else { + // No specific drop target (cross-column drop without card position). + // Assign a rank near the end so the task appears at the bottom of + // the target column. + newSortOrder = generateEndRank(); + this.debugLog("SORT-ORDER-CROSS-COLUMN-APPEND", { + taskFile: path.split("/").pop(), + groupKey: newGroupValue, + newSortOrder, + }); + } + + this.debugLog("SORT-ORDER-RESULT", { + taskFile: path.split("/").pop(), + newSortOrder, + isNull: newSortOrder === null, + }); + } + + // Skip file write if nothing to change + const needsWrite = needsGroupUpdate || needsSwimlaneUpdate || newSortOrder !== null; + if (!needsWrite) continue; + + const file = this.plugin.app.vault.getAbstractFileByPath(path); + if (!file || !(file instanceof TFile)) continue; + + // Single atomic write: groupBy + swimlane + sort_order + await this.plugin.app.fileManager.processFrontMatter(file, (fm) => { + // Update groupBy property if changing columns + if (needsGroupUpdate) { + const frontmatterKey = groupByPropertyId.replace(/^(note\.|file\.|task\.)/, ""); + if (isGroupByListProperty && sourceColumn) { + // List property: remove source value, add target value + let currentValue = fm[frontmatterKey]; + if (!Array.isArray(currentValue)) { + currentValue = currentValue ? [currentValue] : []; + } + const newValue = currentValue.filter((v: string) => v !== sourceColumn); + if (!newValue.includes(newGroupValue) && newGroupValue !== "None") { + newValue.push(newGroupValue); + } + fm[frontmatterKey] = newValue.length > 0 ? newValue : []; + } else { + fm[frontmatterKey] = newGroupValue; } } - } + + // Update swimlane property if changing swimlanes + if (needsSwimlaneUpdate) { + const swimKey = this.swimLanePropertyId!.replace(/^(note\.|file\.|task\.)/, ""); + if (isSwimlaneListProperty && sourceSwimlane) { + let currentValue = fm[swimKey]; + if (!Array.isArray(currentValue)) { + currentValue = currentValue ? [currentValue] : []; + } + const newValue = currentValue.filter((v: string) => v !== sourceSwimlane); + if (!newValue.includes(newSwimLaneValue!) && newSwimLaneValue !== "None") { + newValue.push(newSwimLaneValue!); + } + fm[swimKey] = newValue.length > 0 ? newValue : []; + } else { + fm[swimKey] = newSwimLaneValue; + } + } + + // Write sort_order + if (newSortOrder !== null) { + fm[sortOrderField] = newSortOrder; + } + }); + + this.debugLog("ATOMIC-WRITE-DONE", { + taskFile: path.split("/").pop(), + needsGroupUpdate, + needsSwimlaneUpdate, + hasSortOrder: newSortOrder !== null, + }); } // Clear selection after batch move @@ -2091,10 +2356,15 @@ export class KanbanView extends BasesViewBase { this.plugin.taskSelectionService?.exitSelectionMode(); } - // Refresh to show updated position - this.debouncedRefresh(); + this.debugLog("HANDLE-DROP-COMPLETE", { pathsUpdated: pathsToUpdate.map(p => p.split("/").pop()) }); + }); // end dropQueue.enqueue } catch (error) { console.error("[TaskNotes][KanbanView] Error updating task:", error); + } finally { + this.activeDropCount--; + if (this.activeDropCount === 0) { + this.schedulePostDropRender(); + } } } @@ -2102,40 +2372,6 @@ export class KanbanView extends BasesViewBase { * Update a list property when dragging between columns. * Removes the source column's value and adds the target column's value. */ - private async updateListPropertyOnDrop( - taskPath: string, - basesPropertyId: string, - sourceValue: string, - targetValue: string - ): Promise { - // If dropping on the same column, do nothing - if (sourceValue === targetValue) return; - - const file = this.plugin.app.vault.getAbstractFileByPath(taskPath); - if (!file || !(file instanceof TFile)) { - throw new Error(`Cannot find task file: ${taskPath}`); - } - - const frontmatterKey = basesPropertyId.replace(/^(note\.|file\.|task\.)/, ""); - - await this.plugin.app.fileManager.processFrontMatter(file, (frontmatter) => { - let currentValue = frontmatter[frontmatterKey]; - - // Ensure we're working with an array - if (!Array.isArray(currentValue)) { - currentValue = currentValue ? [currentValue] : []; - } - - // Create new array: remove source value, add target value (if not already present) - const newValue = currentValue.filter((v: string) => v !== sourceValue); - if (!newValue.includes(targetValue) && targetValue !== "None") { - newValue.push(targetValue); - } - - // Update the frontmatter - frontmatter[frontmatterKey] = newValue.length > 0 ? newValue : []; - }); - } /** * Update a frontmatter property for any property (built-in or user-defined) @@ -2211,6 +2447,33 @@ export class KanbanView extends BasesViewBase { }, 150); } + private static readonly POST_DROP_RENDER_DELAY = 500; // ms + + private debugLog(msg: string, data?: Record): void { + const ts = new Date().toISOString().slice(11, 23); + if (data) { + console.log(`[TN-DBG ${ts}] ${msg}`, JSON.stringify(data)); + } else { + console.log(`[TN-DBG ${ts}] ${msg}`); + } + } + + private schedulePostDropRender(): void { + this.debugLog("SCHEDULE-POST-DROP-RENDER", { delay: KanbanView.POST_DROP_RENDER_DELAY }); + this.suppressRenderUntil = Date.now() + KanbanView.POST_DROP_RENDER_DELAY; + this.pendingRender = false; + + if (this.postDropTimer) clearTimeout(this.postDropTimer); + + const win = this.containerEl.ownerDocument.defaultView || window; + this.postDropTimer = win.setTimeout(() => { + this.debugLog("POST-DROP-TIMER-FIRED: rendering now"); + this.postDropTimer = null; + this.suppressRenderUntil = 0; + this.debouncedRefresh(); + }, KanbanView.POST_DROP_RENDER_DELAY); + } + private renderEmptyState(): void { if (!this.boardEl) return; // Use containerEl.ownerDocument for pop-out window support @@ -2794,6 +3057,12 @@ export class KanbanView extends BasesViewBase { * Override from Component base class. */ onunload(): void { + if (this.postDropTimer) { + clearTimeout(this.postDropTimer); + this.postDropTimer = null; + } + this.suppressRenderUntil = 0; + // Component.register() calls will be automatically cleaned up // We just need to clean up view-specific state this.unregisterBoardListeners(); diff --git a/src/bases/TaskListView.ts b/src/bases/TaskListView.ts index 673cdcda..de4a4c48 100644 --- a/src/bases/TaskListView.ts +++ b/src/bases/TaskListView.ts @@ -17,6 +17,7 @@ import { stripPropertyPrefix, isSortOrderInSortConfig, computeSortOrder, + DropOperationQueue, } from "./sortOrderUtils"; export class TaskListView extends BasesViewBase { @@ -49,6 +50,7 @@ export class TaskListView extends BasesViewBase { private dragOverRafId: number = 0; // rAF handle for throttled dragover private dragContainer: HTMLElement | null = null; // Container holding siblings during drag private currentInsertionIndex: number = -1; // Current gap/slot position + private dropQueue = new DropOperationQueue(); /** * Threshold for enabling virtual scrolling in task list view. @@ -438,29 +440,46 @@ export class TaskListView extends BasesViewBase { targetGroupKey: string | null, sourceGroupKey: string | null ): Promise { - const groupByPropertyId = this.getGroupByPropertyId(); - const cleanGroupBy = groupByPropertyId ? stripPropertyPrefix(groupByPropertyId) : null; + await this.dropQueue.enqueue(draggedPath, async () => { + const groupByPropertyId = this.getGroupByPropertyId(); + const cleanGroupBy = groupByPropertyId ? stripPropertyPrefix(groupByPropertyId) : null; - // If the task was dragged to a different group, update the group property - if (groupByPropertyId && targetGroupKey !== null && targetGroupKey !== sourceGroupKey) { - await this.updateTaskGroupProperty(draggedPath, groupByPropertyId, targetGroupKey); - } + const needsGroupUpdate = groupByPropertyId && targetGroupKey !== null && targetGroupKey !== sourceGroupKey; - // Compute and write sort_order for positioning within the target group - const newSortOrder = await computeSortOrder( - targetPath, above, targetGroupKey, cleanGroupBy, draggedPath, - this.plugin, this.taskInfoCache - ); - if (newSortOrder !== null) { - const sortOrderField = this.plugin.settings.fieldMapping.sortOrder; - const file = this.plugin.app.vault.getAbstractFileByPath(draggedPath); - if (file && file instanceof TFile) { - await this.plugin.app.fileManager.processFrontMatter(file, (fm) => { - fm[sortOrderField] = newSortOrder; - }); + // Compute sort_order first (read-only — no file writes yet) + const newSortOrder = await computeSortOrder( + targetPath, above, targetGroupKey, cleanGroupBy, draggedPath, + this.plugin, this.taskInfoCache + ); + + // Determine if we need to write anything + const needsWrite = needsGroupUpdate || newSortOrder !== null; + if (!needsWrite) { + this.debouncedRefresh(); + return; } - } - this.debouncedRefresh(); + + const file = this.plugin.app.vault.getAbstractFileByPath(draggedPath); + if (!file || !(file instanceof TFile)) { + this.debouncedRefresh(); + return; + } + + const sortOrderField = this.plugin.settings.fieldMapping.sortOrder; + + // Single atomic write: group property + sort_order + await this.plugin.app.fileManager.processFrontMatter(file, (fm) => { + if (needsGroupUpdate) { + const frontmatterKey = groupByPropertyId!.replace(/^(note\.|file\.|task\.)/, ""); + fm[frontmatterKey] = targetGroupKey; + } + if (newSortOrder !== null) { + fm[sortOrderField] = newSortOrder; + } + }); + + this.debouncedRefresh(); + }); } /** @@ -468,32 +487,6 @@ export class TaskListView extends BasesViewBase { * Uses TaskService.updateProperty when possible (for business logic like * completedDate, auto-archive, webhooks), falls back to direct frontmatter write. */ - private async updateTaskGroupProperty( - taskPath: string, - basesPropertyId: string, - value: any - ): Promise { - const file = this.plugin.app.vault.getAbstractFileByPath(taskPath); - if (!file || !(file instanceof TFile)) return; - - // Strip Bases prefix to get the frontmatter key - const frontmatterKey = basesPropertyId.replace(/^(note\.|file\.|task\.)/, ""); - - const task = await this.plugin.cacheManager.getTaskInfo(taskPath); - const taskProperty = this.plugin.fieldMapper.lookupMappingKey(frontmatterKey); - - if (task && taskProperty) { - await this.plugin.taskService.updateProperty( - task, - taskProperty as keyof TaskInfo, - value - ); - } else { - await this.plugin.app.fileManager.processFrontMatter(file, (frontmatter) => { - frontmatter[frontmatterKey] = value; - }); - } - } /** * Compute Bases formulas for TaskNotes items. diff --git a/src/bases/sortOrderUtils.ts b/src/bases/sortOrderUtils.ts index 82eea3b7..2093eed8 100644 --- a/src/bases/sortOrderUtils.ts +++ b/src/bases/sortOrderUtils.ts @@ -57,6 +57,92 @@ function tryParseLexoRank(value: string): LexoRank | null { } } +/** + * Increment a base-36 digit (0-9, a-z). Returns 'z' at ceiling. + */ +function nextBase36Char(ch: string): string { + if (ch >= "0" && ch <= "8") return String.fromCharCode(ch.charCodeAt(0) + 1); + if (ch === "9") return "a"; + if (ch >= "a" && ch <= "y") return String.fromCharCode(ch.charCodeAt(0) + 1); + return "z"; +} + +/** + * Decrement a base-36 digit (0-9, a-z). Returns '0' at floor. + */ +function prevBase36Char(ch: string): string { + if (ch >= "1" && ch <= "9") return String.fromCharCode(ch.charCodeAt(0) - 1); + if (ch === "a") return "9"; + if (ch >= "b" && ch <= "z") return String.fromCharCode(ch.charCodeAt(0) - 1); + return "0"; +} + +/** + * Generate a rank that sorts after `rank` in plain string comparison. + * + * For values whose integer part doesn't start with 'z', constructs an + * upper bound (same digit count) and uses `between()` for the midpoint. + * + * For values starting with 'z' (near the ceiling of the range), or if + * `between()` overflows, falls back to extending the decimal part with + * 'i' (midpoint of base-36). This is always safe because appending to + * the decimal makes the string lexicographically greater without any + * arithmetic that could wrap around. + */ +function safeGenNext(rank: LexoRank): LexoRank { + const str = rank.toString(); + const pipeIdx = str.indexOf("|"); + const colonIdx = str.indexOf(":"); + const bucket = str.substring(0, pipeIdx); + const value = str.substring(pipeIdx + 1, colonIdx); + const decimal = str.substring(colonIdx + 1); + const firstChar = value.charAt(0); + + if (firstChar !== "z") { + const upperStr = bucket + "|" + nextBase36Char(firstChar) + value.slice(1) + ":"; + const result = rank.between(LexoRank.parse(upperStr)); + // Sanity check: result must actually be greater than rank. + // LexoRank.between() can overflow for near-ceiling values. + if (result.toString() > str) return result; + } + + // Near ceiling or between() overflow: extend via the decimal part. + // e.g. "0|z14hzz:" → "0|z14hzz:i" (always > original) + return LexoRank.parse(bucket + "|" + value + ":" + decimal + "i"); +} + +/** + * Generate a rank that sorts before `rank` in plain string comparison. + * Mirror of `safeGenNext` — decrements the first character to build a + * lower bound and uses `between()`. + */ +function safeGenPrev(rank: LexoRank): LexoRank { + const str = rank.toString(); + const pipeIdx = str.indexOf("|"); + const colonIdx = str.indexOf(":"); + const bucket = str.substring(0, pipeIdx); + const value = str.substring(pipeIdx + 1, colonIdx); + const firstChar = value.charAt(0); + + let lowerStr: string; + if (firstChar === "0") { + lowerStr = bucket + "|0:"; + } else { + lowerStr = bucket + "|" + prevBase36Char(firstChar) + value.slice(1) + ":"; + } + return LexoRank.parse(lowerStr).between(rank); +} + +/** + * Generate a LexoRank string near the end of the ranking space. + * Used for cross-column drops where no specific position was targeted — + * the task should appear at the bottom of the target column. + */ +export function generateEndRank(): string { + const endRank = LexoRank.parse("0|zzzzzz:"); + return safeGenPrev(endRank).toString(); +} + /** * Ensure a task has a valid LexoRank. If it doesn't, assign one and persist it * to the task's frontmatter. @@ -120,17 +206,31 @@ async function rankBetween( sortOrderField: string ): Promise { const aboveRank = await ensureRank(aboveTask, aboveFallback, plugin, sortOrderField); + const belowFallback = safeGenNext(safeGenNext(aboveRank)); const belowRank = await ensureRank( - belowTask, aboveRank.genNext().genNext(), plugin, sortOrderField + belowTask, belowFallback, plugin, sortOrderField ); try { - return aboveRank.between(belowRank).toString(); + const result = aboveRank.between(belowRank).toString(); + const aboveStr = aboveRank.toString(); + const belowStr = belowRank.toString(); + // Sanity check: result must be strictly between the two ranks. + // LexoRank.between() can overflow for near-ceiling values. + if (result > aboveStr && result < belowStr) { + return result; + } + console.warn( + `[TaskNotes] LexoRank.between() produced out-of-range result: ` + + `${result} (expected between ${aboveStr} and ${belowStr}), ` + + `falling back to safeGenNext()` + ); + return safeGenNext(aboveRank).toString(); } catch (e) { console.warn( - "[TaskNotes] LexoRank.between() failed, falling back to genNext():", + "[TaskNotes] LexoRank.between() failed, falling back to safeGenNext():", e ); - return aboveRank.genNext().toString(); + return safeGenNext(aboveRank).toString(); } } @@ -229,7 +329,7 @@ export function getGroupTasks( * - Drop above first task → `firstRank.genPrev()` * - Drop below last task → `lastRank.genNext()` * - Between two tasks → `rankBetween()` (with try/catch protection) - * - Unranked/legacy neighbors get a rank assigned on the fly via `ensureRank()` + * - Tasks without a valid LexoRank sort_order are excluded from neighbor lookup * * @param targetTaskPath - Path of the task being dropped on. * @param above - Whether to drop above (true) or below (false) the target. @@ -238,8 +338,7 @@ export function getGroupTasks( * @param draggedPath - Path of the task being dragged. * @param plugin - Plugin instance. * @param taskInfoCache - Optional cache for TaskInfo reuse. - * @param swimlaneKey - Swimlane value to scope to, or `null`/`undefined`. - * @param swimlaneProperty - Cleaned frontmatter property name for swimlanes, or `null`/`undefined`. + * @param debugLog - Optional debug logging callback. * @returns The computed sort_order string, or `null` if computation fails. */ export async function computeSortOrder( @@ -250,23 +349,80 @@ export async function computeSortOrder( draggedPath: string, plugin: TaskNotesPlugin, taskInfoCache?: Map, - swimlaneKey?: string | null, - swimlaneProperty?: string | null + debugLog?: (msg: string, data?: Record) => void ): Promise { const sortOrderField = plugin.settings.fieldMapping.sortOrder; + const _log = debugLog || (() => {}); - // Get group tasks (with swimlane filtering), excluding the task being dragged + // Always scan the full vault for ALL tasks in this column — not just + // the tasks visible in the current view. This prevents invisible tasks + // (hidden by view filters or swimlane grouping) from being "leapfrogged" + // when the user reorders within a filtered view. + // + // Also exclude tasks with non-LexoRank sort_orders (e.g. legacy numeric + // timestamps). These can't participate in LexoRank.between() and would + // corrupt the neighbor lookup if they end up adjacent to the target. const columnTasks = getGroupTasks( groupKey, groupByProperty, plugin, taskInfoCache, - swimlaneKey, swimlaneProperty, sortOrderField - ).filter(t => t.path !== draggedPath); + undefined, undefined, sortOrderField + ).filter(t => { + if (t.path === draggedPath) return false; + // Exclude tasks without a sort_order or with a non-LexoRank sort_order. + // Tasks without a defined LexoRank can't meaningfully participate in + // neighbor lookup — using them causes ensureRank() to write fallback + // ranks to unrelated files and LexoRank.between() to produce garbage + // for boundary values. + if (t.sortOrder === undefined) return false; + if (!tryParseLexoRank(t.sortOrder)) return false; + return true; + }); + + _log("COMPUTE-SORT-ORDER", { + columnTaskCount: columnTasks.length, + columnTasks: columnTasks.map(t => ({ + file: t.path.split("/").pop(), + sortOrder: t.sortOrder, + })), + targetFile: targetTaskPath.split("/").pop(), + draggedFile: draggedPath.split("/").pop(), + above, + groupKey, + groupByProperty, + }); if (!columnTasks || columnTasks.length === 0) { + _log("COMPUTE-SORT-ORDER: empty column, returning middle"); return LexoRank.middle().toString(); } const targetIndex = columnTasks.findIndex(t => t.path === targetTaskPath); - if (targetIndex === -1) return null; + + + + if (targetIndex === -1) { + _log("COMPUTE-SORT-ORDER: target not found — appending to end of column", { + targetFile: targetTaskPath.split("/").pop(), + allPaths: columnTasks.map(t => t.path.split("/").pop()), + }); + // Target not in column (e.g. cross-column drop with stale target, or empty target). + // Append after the last task in the column. + const lastTask = columnTasks[columnTasks.length - 1]; + const lastRank = await ensureRank(lastTask, LexoRank.middle(), plugin, sortOrderField); + const result = safeGenNext(lastRank).toString(); + _log("COMPUTE-SORT-ORDER: appended after last", { lastFile: lastTask.path.split("/").pop(), lastRank: lastRank.toString(), result }); + return result; + } + + _log("COMPUTE-SORT-ORDER: target found", { + targetIndex, + totalTasks: columnTasks.length, + position: above + ? (targetIndex === 0 ? "ABOVE-FIRST" : "BETWEEN") + : (targetIndex === columnTasks.length - 1 ? "BELOW-LAST" : "BETWEEN"), + neighborAbove: targetIndex > 0 ? { file: columnTasks[targetIndex - 1].path.split("/").pop(), so: columnTasks[targetIndex - 1].sortOrder } : null, + target: { file: columnTasks[targetIndex].path.split("/").pop(), so: columnTasks[targetIndex].sortOrder }, + neighborBelow: targetIndex < columnTasks.length - 1 ? { file: columnTasks[targetIndex + 1].path.split("/").pop(), so: columnTasks[targetIndex + 1].sortOrder } : null, + }); if (above) { if (targetIndex === 0) { @@ -274,31 +430,66 @@ export async function computeSortOrder( const firstRank = await ensureRank( columnTasks[0], LexoRank.middle(), plugin, sortOrderField ); - return firstRank.genPrev().toString(); + const result = safeGenPrev(firstRank).toString(); + _log("COMPUTE-SORT-ORDER: above first → safeGenPrev", { firstRank: firstRank.toString(), result }); + return result; } // Between two tasks - return rankBetween( + const result = await rankBetween( columnTasks[targetIndex - 1], columnTasks[targetIndex], LexoRank.middle(), plugin, sortOrderField ); + _log("COMPUTE-SORT-ORDER: between (above)", { result }); + return result; } else { if (targetIndex === columnTasks.length - 1) { // Drop below last task const lastRank = await ensureRank( columnTasks[columnTasks.length - 1], LexoRank.middle(), plugin, sortOrderField ); - return lastRank.genNext().toString(); + const result = safeGenNext(lastRank).toString(); + _log("COMPUTE-SORT-ORDER: below last → safeGenNext", { lastRank: lastRank.toString(), result }); + return result; } // Between two tasks - return rankBetween( + const result = await rankBetween( columnTasks[targetIndex], columnTasks[targetIndex + 1], LexoRank.middle(), plugin, sortOrderField ); + _log("COMPUTE-SORT-ORDER: between (below)", { result }); + return result; + } +} + +/** + * Per-task promise queue that serializes async drop operations on the same file. + * + * When a user rapidly drags the same task twice (e.g. column A → B → A), the + * second `handleTaskDrop` must wait for the first to fully complete before + * running — otherwise their interleaved `processFrontMatter` writes cause the + * task to end up at the wrong position. + * + * Different task paths are processed in parallel (they write to different files). + */ +export class DropOperationQueue { + private queues = new Map>(); + + async enqueue(taskPath: string, operation: () => Promise): Promise { + const prev = this.queues.get(taskPath) ?? Promise.resolve(); + const next = prev.then(operation, operation); + this.queues.set(taskPath, next); + try { + await next; + } finally { + if (this.queues.get(taskPath) === next) { + this.queues.delete(taskPath); + } + } } }