Skip to content

explorer-nav-dragover-fix: v1.2 edge-scroll and transient folder collapse#4294

Open
tonythethompson wants to merge 3 commits into
ramensoftware:mainfrom
tonythethompson:explorer-nav-dragover-fix-v1.2
Open

explorer-nav-dragover-fix: v1.2 edge-scroll and transient folder collapse#4294
tonythethompson wants to merge 3 commits into
ramensoftware:mainfrom
tonythethompson:explorer-nav-dragover-fix-v1.2

Conversation

@tonythethompson
Copy link
Copy Markdown
Contributor

Summary

Follow-up to the merged Explorer Nav DragOver Fix mod (#4192). Bumps the mod to v1.2.4 with:

  • Accelerated edge-scroll during drag — scroll speed scales with cursor distance from the top/bottom of the nav pane (configurable band and max lines).
  • Transient folder collapse — folders that were collapsed before the drag and auto-expanded only because of drag-hover are collapsed again when the drag ends (staggered animation optional). A successful drop keeps the branch that contained the last nav-pane drop target expanded.
  • Reliable collapse across branch changes — separates drag session id (one per outer drag) from viewport generation (bumped on nav wheel during drag). Fixes folders left expanded after scrolling between drives or sibling branches during the same drag.

New settings

Setting Default Purpose
accelerateEdgeScrollDuringDrag true Faster edge-scroll vs native one-line crawl
edgeScrollBandPx 48 Pixel band for acceleration (8–128)
edgeScrollMaxLines 6 Max lines per edge-scroll tick (1–24)
collapseTransientExpansions true Fold drag-opened folders when drag ends
transientCollapseStaggerMs 140 Delay between collapse steps; 0 = instant (0–800)

Existing anti-jump and wheel-during-drag settings are unchanged.

Implementation notes

  • Tracks transient expansions with itemLParam and re-resolves items after staggered TVM_EXPAND collapse if handles go stale.
  • Staggered collapse uses WM_TIMER, WM_SETREDRAW, and a reconcile pass for missed folders.
  • Edge-scroll adds extra line steps on top of Explorer’s native edge scroll (with bottom/top outside-slack for clipped cursors).
  • Preserve-on-drop uses ancestor check: keep expansion nodes that contain the drop target.

Testing

  • Windows 11, File Explorer navigation pane.
  • Drag across This PC → Drive A → Drive B with wheel scroll between drives; cancel or drop outside branch — both transient expansions collapse.
  • Drag between sibling folders under the same parent without scrolling — both collapse on cancel.
  • Successful drop into a nested folder — ancestors on the drop path stay expanded; unrelated drag-opened branches collapse.
  • Edge-scroll at top/bottom of nav pane feels faster than native; settings off restore native behavior.
  • transientCollapseStaggerMs at 0 and at 180–250 — instant vs calmer stepped collapse.

Changelog

1.2.4

  • Fix transient collapse skipped after nav wheel scroll (drag session id vs generation).
  • Fix preserve-on-drop ancestor check for successful drops.

1.2.0–1.2.3 (included in this PR)

  • Edge-scroll acceleration during drag.
  • Transient expansion tracking, staggered collapse, reconcile pass, lParam re-resolution.

Mod authorship

  • The submitter, with AI assistance
  • The submitter, without AI assistance
  • Someone else (please credit them in the PR)

…lapse

Adds accelerated nav edge-scroll during drag, optional staggered collapse of folders opened only by drag-hover, and drag-session tracking so wheel scroll between branch families (e.g. drives) no longer orphans collapse. Successful drops preserve the branch containing the last nav drop target.

Co-authored-by: Cursor <cursoragent@cursor.com>
@m417z
Copy link
Copy Markdown
Member

m417z commented Jun 4, 2026

Submission review

Note: This review was done by Claude, and then refined manually. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

The main note is about the mod's functionality. I didn't inspect the code's logic manually, please check the review and and address it, or let me know if it's not correct.


One correctness issue with the new "preserve on drop" feature:

Preserve-on-drop collapses the drop target's ancestors instead of preserving them (IsSameOrDescendant arguments reversed). In FlushDragTreeState:

if (successfulDrop && entry.hwndTree == preserveTree && preserveItem &&
    IsSameOrDescendant(entry.hwndTree, entry.hItem, preserveItem)) {
    continue;  // keep this expansion
}

IsSameOrDescendant(tree, hItem, possibleAncestor) walks up from hItem and returns true when hItem is possibleAncestor or a descendant of it. So this skips collapse only for transient expansions that are the drop target or sit below it — i.e. it preserves descendants of the drop target and collapses its ancestors.

But the feature (and your PR notes — "ancestors on the drop path stay expanded") needs the opposite: keep the expansions that contain the drop target. Concretely, drag a file, hover C: (auto-expands), move into C:\Projects (auto-expands), drop on Projects. Both C: and Projects are transient expansions; the drop target is Projects. The check preserves Projects but collapses C: — collapsing C: hides Projects, so right after a successful nested drop the whole path folds up and you lose sight of where you dropped. (It only looks correct when the ancestor was already expanded before the drag, so it wasn't a transient expansion in the first place — which may be why testing missed it.)

Fix is to swap the two item arguments so you preserve an expansion when it is an ancestor-or-self of the drop target (drop target is same-or-descendant of the expansion):

    IsSameOrDescendant(entry.hwndTree, preserveItem, entry.hItem)) {
Optional improvements

Minor polish — none of this affects users, so it's your call.

  • The staggered-collapse machinery is very heavy for a cosmetic cleanup. The transient-collapse feature pulls in ~700 lines: per-window stagger sessions, a depth-scaled timer schedule (ComputeInitialCollapseTimerMs lead-in + ComputeNextCollapseTimerMs depth bonus), a 4-pass ReconcileTransientCollapses, lParam re-resolution via full-tree DFS, and two parallel code paths (instant vs staggered). For "fold the drag-opened folders back when the drag ends," a single collapse pass (deepest-first, which you already do) would deliver almost all of the benefit at a fraction of the surface area. Given the AI-assisted authorship, it's worth asking whether the animated stagger earns its complexity — collapsing instantly is simpler to reason about and to maintain. Not blocking, just a maintainability call.

  • protectedMaxDepth struct default disagrees with the settings/README default. The Settings struct initializes std::atomic<int> protectedMaxDepth{2}; while the settings block and README both document the default as 3. LoadSettings() always overwrites it from the setting, so the {2} is never actually used — but aligning it to {3} avoids confusion for the next reader.

  • Wh_ModUninit performs the collapse animation during teardown via unbounded SendMessageW. DrainStaggeredCollapseForWindowCollapseTargetsNow/ReconcileTransientCollapses issue SendInternal (plain SendMessageW, no timeout) to each tree from the uninit thread. Everywhere else this mod is careful to bound teardown (SMTO_ABORTIFHUNG, the 100 ms drain wait, leaking rather than blocking), so doing potentially-blocking cross-thread sends here is a bit inconsistent — and doing a visible fold-up as the mod is being disabled is questionable UX. Consider just killing the timers and clearing g_staggeredCollapseSessions on unload rather than flushing the collapses.

Functionality notes

Non-critical observations about the feature behavior itself.

  • A pending staggered collapse isn't cancelled when a new drag begins, so it can fire mid-drag. g_staggeredCollapseSessions is keyed by HWND and is not touched on the 0 -> 1 drag edge in DoDragDrop_Hook (only PurgeStaleDragTreeStateNotInSession runs, which clears g_transientExpansions/g_lastNavDropTarget but not the timer sessions). If the user finishes a drag and immediately starts another before the stagger timer drains, the leftover timer keeps collapsing the previously-recorded folders — which can collapse a folder the user is actively hovering in the new drag. Draining/cancelling pending staggered collapses on the new drag's 0 -> 1 edge would close this.

Swap preserve-on-drop IsSameOrDescendant args so ancestor expansions on the drop path stay open. Cancel pending stagger timers on new drag and on mod unload instead of draining collapses during teardown. Align protectedMaxDepth struct default with settings (3). v1.2.5.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tonythethompson
Copy link
Copy Markdown
Contributor Author

tonythethompson commented Jun 4, 2026

Thanks for the review — addressed in 787e6de (v1.2.5) and 1218aa3 (v1.2.6):

Blocking: Swapped preserve-on-drop to IsSameOrDescendant(tree, preserveItem, entry.hItem) so we walk up from the drop target and keep transient expansions that are ancestors of (or equal to) the drop target. You were right that the previous argument order preserved descendants and collapsed ancestors.

Optional improvements (from review):

  • protectedMaxDepth struct default aligned to 3
  • Stagger removed entirely (1218aa3) — took your maintainability point; dropped transientCollapseStaggerMs, timers, and stagger sessions. Drag-end cleanup is instant deepest-first collapse plus the reconcile pass only (~200 lines removed). No collapse work on mod unload anymore since there are no pending timers.

Remove staggered timer collapse, transientCollapseStaggerMs setting, and ~200 lines of timer session machinery. Drag-end cleanup is a single deepest-first pass plus reconcile. v1.2.6.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants