Skip to content

Commit 6074cd2

Browse files
committed
Bugfix: don't re-anchor focus inside selectVolumeByIndex/navigateToPath
Reverts the focus-restoration calls added to those two methods in `d9212b83`. They were intended to fix MCP/test paths where DOM focus drifted, but post-Group-A the real fix turned out to be the null-vs-undefined sweep in `f2019aff` — those tests now pass without the focus polish. Worse, the late-firing `.finally(() => containerElement?.focus())` inside `navigateToPath` raced with subsequent keystrokes in `mtp.spec.ts:414` ("deletes multiple selected files on MTP"): one of the Space presses in the multi-select sequence got dropped, the autoConfirmed delete then targeted only the first file, and the second backing file survived. `moveCursor` keeps its `await paneRef.setCursorIndex(...)` (was fire-and-forget) — that's a real correctness fix for the MCP `move_cursor` round-trip, independent of focus. If MCP/test paths genuinely need focus restoration after async nav, the right place is the `mcp-nav-to-path` event handler in `mcp-listeners.ts`, which already awaits the promise — re-anchor focus there, not via fire-and-forget `.finally()` here.
1 parent 6a90560 commit 6074cd2

1 file changed

Lines changed: 10 additions & 15 deletions

File tree

apps/desktop/src/lib/file-explorer/pane/DualPaneExplorer.svelte

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1920,10 +1920,9 @@
19201920
handleVolumeChange(pane, volume.id, volume.path, volume.path)
19211921
}
19221922
1923-
// MCP-driven volume change: re-anchor DOM focus on the explorer container so
1924-
// subsequent keyboard input (arrow keys, F-keys) lands in the right pane's
1925-
// dispatcher chain. `handleVolumeChange` updates state but doesn't touch focus.
1926-
containerElement?.focus()
1923+
// Note: previously we re-anchored DOM focus on the container here, but it was
1924+
// the proximate cause of mtp.spec.ts:414 dropping a Space press during the
1925+
// multi-select-then-delete sequence. Removed pending a less invasive fix.
19271926
return true
19281927
}
19291928
@@ -1990,17 +1989,13 @@
19901989
19911990
const paneRef = getPaneRef(pane)
19921991
if (!paneRef) return 'Pane not available'
1993-
const result = paneRef.navigateToPath(path)
1994-
// After the listing settles, re-anchor DOM focus on the explorer container.
1995-
// MCP-driven nav doesn't go through `handleFocus`, so without this the inner
1996-
// FilePane re-render can leave `document.activeElement` as `<body>`, which
1997-
// breaks subsequent keyboard input that relies on bubbling through the pane.
1998-
if (result instanceof Promise) {
1999-
void result.finally(() => containerElement?.focus())
2000-
} else {
2001-
containerElement?.focus()
2002-
}
2003-
return result
1992+
// Note: previously we re-anchored DOM focus on the container after the listing
1993+
// settled, but the late-firing `.finally()` callback raced with subsequent test
1994+
// keystrokes (mtp.spec.ts line 414 — `deletes multiple selected files on MTP`)
1995+
// and dropped one of the Space presses on the floor. If MCP/test paths actually
1996+
// need focus restoration here, rewire the `mcp-nav-to-path` listener to restore
1997+
// focus AFTER awaiting the promise, not via fire-and-forget `.finally()`.
1998+
return paneRef.navigateToPath(path)
20041999
}
20052000
20062001
/**

0 commit comments

Comments
 (0)