Repository navigation
Preserve Explorer rename input when the window loses focus - #338126
Conversation
|
@microsoft-github-policy-service agree |
ebdb2dc to
04d0907
Compare
04d0907 to
6cdf0cb
Compare
|
I just revamped a large portion of the rename input, can you address the merge conflicts please |
Keep the upstream Explorer inline-edit refactor and move the page-blur focus check to the shared InlineEditInputBox. Validation: existing regression fails without the migrated fix; both Explorer focus tests and seven InlineEditInputBox tests pass on Wayland with Electron 43.7.5. ESLint and diff against main checks pass. TypeScript reports the same 583 diagnostics as unmodified main with the available local dependencies.
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: Benjamin Christopher Simmonds (@benibenj)Matched files:
|
|
Logan Ramos (@lramos15) Sorry for the delay! I've merged the latest main and resolved the conflicts, keeping the new InlineEditInputBox implementation and moving the focus fix there. The Explorer regression tests and existing InlineEditInputBox tests pass. |
Logan Ramos (lramos15)
left a comment
There was a problem hiding this comment.
Thanks for the fix, and for moving it into the new InlineEditInputBox after the rework! The input-source switching case looks right, and the Electron focus helper is a nice way to test it.
One regression to address before merging: Chromium doesn't dispatch blur for focus changes made while the window is unfocused. Before this change that didn't matter, because losing window focus always finished editing. Now the blur from the window losing focus hits the return branch, so nothing is waiting any longer. If focus then moves inside VS Code while it's in the background, the inline edit stays open with focus elsewhere and never finishes. For example: an extension calls showTextDocument, the debugger focuses the editor when a breakpoint hits, or code -r file runs. Because this class is shared, that applies to Explorer, terminal tabs, Ports and Timeline. In Explorer the item stays in its editing state until the user clicks back into the box.
I've left two suggestions inline:
- In the
activeElementbranch, wait for the window to regain focus and check again, instead of returning. - A regression test for that scenario, using your focus helper.
Validated locally on macOS:
- The new test fails on the current head (
connected: true, finished: []). - With both suggestions applied, all 11 tests in
explorerRename.test.tsandinlineEditInputBox.test.tspass consistently (3 runs), including your two tests. npm run typecheck-clientand ESLint on the changed files are clean.
Smaller, non-blocking:
- Since the change now lives in the shared
InlineEditInputBox, these tests might fit better as anelectron-browsertest next tosrc/vs/workbench/test/browser/parts/views/inlineEditInputBox.test.tsthat constructsInlineEditInputBoxdirectly, rather than going throughFilesRendererwith Explorer stubs. - It's worth a sentence in the class doc comment saying that the window losing focus (e.g. Alt+Tab or an OS input-source switcher) keeps editing, since that's a deliberate change from the previous behavior.
cfcd228 to
1d0b089
Compare
Co-authored-by: Logan Ramos <lramos15@gmail.com>
…ename.test.ts Co-authored-by: Logan Ramos <lramos15@gmail.com>
|
Logan Ramos (@lramos15) Thanks for the review! I've applied both suggestions and resolved the threads. All 11 tests, type checking, and ESLint passed locally with these changes. |
|
Logan Ramos (@lramos15) I forgot to address the two non-blocking suggestions in my previous update. Both are now done: the Electron tests construct InlineEditInputBox directly, and the class documentation explains the window-focus behavior. All 11 checks and ESLint pass locally. |
Fixes #338125
Switching input sources through GNOME Shell's switcher on Wayland can close the Explorer rename input and accept a valid pending filename. The blur handler checks
document.hasFocus()first, so losing page focus finishes editing even when the input remains the document's active element.Check the input's own
ownerDocument.activeElementbefore deciding to finish editing. This preserves the input, pending text and selection when the page loses focus. Moving focus to another element in the document still finishes editing, and the existing context-menu handling is retained.This intentionally also preserves rename across window switches such as Alt+Tab, provided the input remains its document's active element.
Adds two Electron tests using the real
FilesRenderer: preserving rename on page blur, and finishing it when focus moves to another input. A test-only IPC helper focuses a secondWebContentsViewto produce page blur without changing the original document's active element.Validation (rechecked after rebasing onto
c94d3d7ed24, with Electron 43.7.3):git diff --checkpassed.To reproduce: select a file in Explorer, press F2, edit the name without confirming, then switch input sources using the configured shortcut (for example, Super+Space). The input should remain open. The GNOME switcher scenario was verified manually; the automated tests cover page-focus handling.
Run the tests after compiling:
Demo
Before the fix — installed VS Code, Wayland:
Screencast.from.2026-09-30.21-44-27.mp4
After the fix — development build with this PR, Wayland:
Screencast.from.2026-09-30.21-46-20.mp4