modal - improve handling of Escape key and expand use of modal editors to more kinds

src/vs/workbench/browser/parts/editor/editorCommands.ts

@@ -1517,11 +1517,15 @@ function registerModalEditorCommands(): void {
 				f1: true,
 				icon: Codicon.close,
 				precondition: EditorPartModalContext,
-				keybinding: {
+				keybinding: [{
 					primary: KeyCode.Escape,
-					weight: KeybindingWeight.WorkbenchContrib + 10,
-					when: EditorPartModalContext
-				},
+					weight: KeybindingWeight.WorkbenchContrib + 10, // higher when no text editor is focused...
+					when: EditorContextKeys.focus.toNegated()
+				}, {
+					primary: KeyCode.Escape,
+					weight: KeybindingWeight.EditorContrib - 1, // ...lower to prevent accidental close when text editor is focused
+					when: EditorContextKeys.focus
+				}],
 				menu: {
 					id: MenuId.ModalEditorTitle,
 					group: 'navigation',

src/vs/workbench/browser/parts/editor/modalEditorPart.ts

@@ -7,7 +7,6 @@ import './media/modalEditorPart.css';
 import { $, addDisposableListener, append, EventHelper, EventType, hide, isHTMLElement, show } from '../../../../base/browser/dom.js';
 import { StandardKeyboardEvent } from '../../../../base/browser/keyboardEvent.js';
 import { Button } from '../../../../base/browser/ui/button/button.js';
-import { KeyCode } from '../../../../base/common/keyCodes.js';
 import { Emitter, Event } from '../../../../base/common/event.js';
 import { DisposableStore, MutableDisposable, toDisposable } from '../../../../base/common/lifecycle.js';
 import { MenuId } from '../../../../platform/actions/common/actions.js';
@@ -90,15 +89,8 @@ export class ModalEditorPart {
 		disposables.add(addDisposableListener(modalElement, EventType.KEY_DOWN, e => {
 			const event = new StandardKeyboardEvent(e);
 
-			// Close on Escape
-			if (event.equals(KeyCode.Escape)) {
-				EventHelper.stop(event, true);
-
-				editorPart.close();
-			}
-
 			// Prevent unsupported commands (not in sessions windows)
-			else if (!this.environmentService.isSessionsWindow) {
+			if (!this.environmentService.isSessionsWindow) {
 				const resolved = this.keybindingService.softDispatch(event, this.layoutService.mainContainer);
 				if (resolved.kind === ResultKind.KbFound && resolved.commandId) {
 					if (

src/vs/workbench/services/preferences/browser/preferencesService.ts

@@ -48,6 +48,7 @@ import { IURLService } from '../../../../platform/url/common/url.js';
 import { compareIgnoreCase } from '../../../../base/common/strings.js';
 import { IExtensionService } from '../../extensions/common/extensions.js';
 import { IProgressService, ProgressLocation } from '../../../../platform/progress/common/progress.js';
+import { IWorkbenchEnvironmentService } from '../../environment/common/environmentService.js';
 
 const emptyEditableSettingsContent = '{\n}';
 
@@ -90,7 +91,8 @@ export class PreferencesService extends Disposable implements IPreferencesServic
 		@ITextEditorService private readonly textEditorService: ITextEditorService,
 		@IURLService urlService: IURLService,
 		@IExtensionService private readonly extensionService: IExtensionService,
-		@IProgressService private readonly progressService: IProgressService
+		@IProgressService private readonly progressService: IProgressService,
+		@IWorkbenchEnvironmentService private readonly environmentService: IWorkbenchEnvironmentService,
 	) {
 		super();
 		// The default keybindings.json updates based on keyboard layouts, so here we make sure
@@ -273,7 +275,7 @@ export class PreferencesService extends Disposable implements IPreferencesServic
 			...options,
 			focusSearch: true
 		};
-		const group = this.getEditorGroupFromOptions(false, options);
+		const group = this.getEditorGroupFromOptions(options);
 		return this.editorService.openEditor(input, validateSettingsEditorOptions(options), group);
 	}
 
@@ -354,11 +356,11 @@ export class PreferencesService extends Disposable implements IPreferencesServic
 					this.editorService.openEditor({ resource: editableKeybindings, options }, sideEditorGroup.id)
 				]);
 			} else {
-				await this.editorService.openEditor({ resource: editableKeybindings, options }, options.groupId);
+				await this.editorService.openEditor({ resource: editableKeybindings, options }, this.getEditorGroupFromOptions(options));
 			}
 
 		} else {
-			const group = this.getEditorGroupFromOptions(false, options);
+			const group = this.getEditorGroupFromOptions(options);
 			const editor = (await this.editorService.openEditor(this.instantiationService.createInstance(KeybindingsEditorInput), { ...options }, group)) as IKeybindingsEditorPane;
 			if (options.query) {
 				editor.search(options.query);
@@ -371,8 +373,11 @@ export class PreferencesService extends Disposable implements IPreferencesServic
 		return this.editorService.openEditor({ resource: this.defaultKeybindingsResource, label: nls.localize('defaultKeybindings', "Default Keybindings") });
 	}
 
-	private getEditorGroupFromOptions(isTextual: boolean, options: { groupId?: number; openToSide?: boolean }): PreferredGroup {
-		if (!isTextual && this.configurationService.getValue<string>('workbench.editor.useModal') !== 'off') {
+	private getEditorGroupFromOptions(options: { groupId?: number; openToSide?: boolean }): PreferredGroup {
+		if (
+			this.configurationService.getValue<string>('workbench.editor.useModal') !== 'off' &&					// modal editors enabled in settings
+			!this.environmentService.enableSmokeTestDriver && !this.environmentService.extensionTestsLocationURI	// but not in smoke test or extension test environments to reduce flakiness
+		) {
 			return MODAL_GROUP;
 		}
 		if (options.openToSide) {
@@ -385,7 +390,7 @@ export class PreferencesService extends Disposable implements IPreferencesServic
 	}
 
 	private async openSettingsJson(resource: URI, options: IOpenSettingsOptions): Promise<IEditorPane | undefined> {
-		const group = this.getEditorGroupFromOptions(true, options);
+		const group = this.getEditorGroupFromOptions(options);
 		const editor = await this.doOpenSettingsJson(resource, options, group);
 		if (editor && options?.revealSetting) {
 			await this.revealSetting(options.revealSetting.key, !!options.revealSetting.edit, editor, resource);