Re-instate swipeToNavigate for Mac (fix #204812) #276360
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new swipeToNavigate setting that enables navigation between open files using three-finger horizontal swipe gestures on macOS. The feature allows users to navigate to previously/next recently used editors by swiping left/right.
- Adds
workbench.editor.swipeToNavigateconfiguration option (macOS-only, non-web) - Implements swipe gesture handling in the Electron main process
- Maps swipe left/right to corresponding editor navigation commands
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vs/workbench/common/editor.ts | Adds swipeToNavigate property to editor configuration interface |
| src/vs/workbench/browser/workbench.contribution.ts | Registers the new configuration setting with description and platform constraints |
| src/vs/workbench/browser/parts/editor/editor.ts | Adds default value and validation for swipeToNavigate option |
| src/vs/platform/windows/electron-main/windowImpl.ts | Implements swipe event listener registration and handling logic |
| // Swipe command support (macOS) | ||
| if (isMacintosh) { | ||
| const config = this.configurationService.getValue<IWorkbenchEditorConfiguration>(); | ||
| if (config?.workbench?.editor?.swipeToNavigate) { | ||
| this.registerSwipeListener(); | ||
| } else { | ||
| this._win.removeAllListeners('swipe'); | ||
| } | ||
| } |
There was a problem hiding this comment.
The configuration check runs on every configuration update, not just when swipeToNavigate changes. This should use affectsConfiguration to check if the specific setting changed (like the menu bar and proxy settings below do). Without this check, registerSwipeListener() is called repeatedly whenever any configuration changes, which leads to multiple duplicate event listeners being registered for the 'swipe' event.
| private registerSwipeListener() { | ||
| this._win.on('swipe', (event: Electron.Event, cmd: string) => { | ||
| if (!this.isReady) { | ||
| return; // window must be ready | ||
| } | ||
|
|
||
| if (cmd === 'left') { | ||
| this.send('vscode:runAction', { id: 'workbench.action.openPreviousRecentlyUsedEditor', from: 'mouse' }); | ||
| } else if (cmd === 'right') { | ||
| this.send('vscode:runAction', { id: 'workbench.action.openNextRecentlyUsedEditor', from: 'mouse' }); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
The swipe event listener is not properly managed for disposal. Each call to registerSwipeListener() adds a new listener without removing the previous one. The listener should be registered using this._register() with Event.fromNodeEventEmitter() (following the pattern used for other window events in setWin() method) to ensure proper cleanup when the window is disposed or the configuration changes.
No description provided.