From 5f3c3ef5751b67a693722a94c958b5e6d30338a4 Mon Sep 17 00:00:00 2001 From: BeniBenj Date: Fri, 10 Jan 2025 14:28:51 +0100 Subject: [PATCH] fix gutter menu mouse interaction --- .../controller/inlineCompletionsController.ts | 18 ++++++++-- .../browser/view/inlineCompletionsView.ts | 5 +-- .../view/inlineEdits/gutterIndicatorView.ts | 36 +++++++++++++------ .../browser/view/inlineEdits/view.ts | 4 ++- .../view/inlineEdits/viewAndDiffProducer.ts | 5 +-- 5 files changed, 50 insertions(+), 18 deletions(-) diff --git a/src/vs/editor/contrib/inlineCompletions/browser/controller/inlineCompletionsController.ts b/src/vs/editor/contrib/inlineCompletions/browser/controller/inlineCompletionsController.ts index b2b4b8ca6c696..a0fbec7292037 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/controller/inlineCompletionsController.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/controller/inlineCompletionsController.ts @@ -8,7 +8,7 @@ import { timeout } from '../../../../../base/common/async.js'; import { cancelOnDispose } from '../../../../../base/common/cancellation.js'; import { createHotClass } from '../../../../../base/common/hotReloadHelpers.js'; import { Disposable, toDisposable } from '../../../../../base/common/lifecycle.js'; -import { ITransaction, autorun, derived, derivedDisposable, derivedObservableWithCache, observableFromEvent, observableSignal, runOnChange, runOnChangeWithStore, transaction, waitForState } from '../../../../../base/common/observable.js'; +import { ITransaction, autorun, derived, derivedDisposable, derivedObservableWithCache, observableFromEvent, observableSignal, observableValue, runOnChange, runOnChangeWithStore, transaction, waitForState } from '../../../../../base/common/observable.js'; import { isUndefined } from '../../../../../base/common/types.js'; import { localize } from '../../../../../nls.js'; import { IAccessibilityService } from '../../../../../platform/accessibility/common/accessibility.js'; @@ -82,6 +82,13 @@ export class InlineCompletionsController extends Disposable { { min: 50, max: 50 } ); + private readonly _focusIsInMenu = observableValue(this, false); + private readonly _focusIsInEditorOrMenu = derived(this, reader => { + const editorHasFocus = this._editorObs.isFocused.read(reader); + const menuHasFocus = this._focusIsInMenu.read(reader); + return editorHasFocus || menuHasFocus; + }); + private readonly _cursorIsInIndentation = derived(this, reader => { const cursorPos = this._editorObs.cursorPosition.read(reader); if (cursorPos === null) { return false; } @@ -114,7 +121,7 @@ export class InlineCompletionsController extends Disposable { private readonly _hideInlineEditOnSelectionChange = this._editorObs.getOption(EditorOption.inlineSuggest).map(val => true); - protected readonly _view = this._register(new InlineCompletionsView(this.editor, this.model, this._instantiationService)); + protected readonly _view = this._register(new InlineCompletionsView(this.editor, this.model, this._focusIsInMenu, this._instantiationService)); constructor( public readonly editor: ICodeEditor, @@ -190,7 +197,12 @@ export class InlineCompletionsController extends Disposable { } })); - this._register(this.editor.onDidBlurEditorWidget(() => { + this._register(autorun(reader => { + const isFocused = this._focusIsInEditorOrMenu.read(reader); + if (isFocused) { + return; + } + // This is a hidden setting very useful for debugging if (this._contextKeyService.getContextKeyValue('accessibleViewIsShown') || this._configurationService.getValue('editor.inlineSuggest.keepOnBlur') diff --git a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineCompletionsView.ts b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineCompletionsView.ts index fa303f3fc6e35..faa3c671e122e 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineCompletionsView.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineCompletionsView.ts @@ -6,7 +6,7 @@ import { createStyleSheetFromObservable } from '../../../../../base/browser/domObservable.js'; import { readHotReloadableExport } from '../../../../../base/common/hotReloadHelpers.js'; import { Disposable } from '../../../../../base/common/lifecycle.js'; -import { derived, mapObservableArrayCached, derivedDisposable, constObservable, derivedObservableWithCache, IObservable } from '../../../../../base/common/observable.js'; +import { derived, mapObservableArrayCached, derivedDisposable, constObservable, derivedObservableWithCache, IObservable, ISettableObservable } from '../../../../../base/common/observable.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { ICodeEditor } from '../../../../browser/editorBrowser.js'; import { observableCodeEditor } from '../../../../browser/observableCodeEditor.js'; @@ -38,7 +38,7 @@ export class InlineCompletionsView extends Disposable { if (!this._everHadInlineEdit.read(reader)) { return undefined; } - return this._instantiationService.createInstance(InlineEditsViewAndDiffProducer.hot.read(reader), this._editor, this._inlineEdit, this._model); + return this._instantiationService.createInstance(InlineEditsViewAndDiffProducer.hot.read(reader), this._editor, this._inlineEdit, this._model, this._focusIsInMenu); }) .recomputeInitiallyAndOnChange(this._store); @@ -48,6 +48,7 @@ export class InlineCompletionsView extends Disposable { constructor( private readonly _editor: ICodeEditor, private readonly _model: IObservable, + private readonly _focusIsInMenu: ISettableObservable, @IInstantiationService private readonly _instantiationService: IInstantiationService, ) { super(); diff --git a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/gutterIndicatorView.ts b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/gutterIndicatorView.ts index e0a1ce126d0e2..01e997d4f6f53 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/gutterIndicatorView.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/gutterIndicatorView.ts @@ -5,8 +5,8 @@ import { renderIcon } from '../../../../../../base/browser/ui/iconLabel/iconLabels.js'; import { Codicon } from '../../../../../../base/common/codicons.js'; -import { Disposable } from '../../../../../../base/common/lifecycle.js'; -import { IObservable, autorun, constObservable, derived, observableFromEvent, observableValue } from '../../../../../../base/common/observable.js'; +import { Disposable, DisposableStore, toDisposable } from '../../../../../../base/common/lifecycle.js'; +import { IObservable, ISettableObservable, autorun, constObservable, derived, observableFromEvent, observableValue } from '../../../../../../base/common/observable.js'; import { IHoverService } from '../../../../../../platform/hover/browser/hover.js'; import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js'; import { buttonBackground, buttonForeground, buttonSecondaryBackground, buttonSecondaryForeground } from '../../../../../../platform/theme/common/colorRegistry.js'; @@ -23,6 +23,7 @@ import { InlineCompletionsModel } from '../../model/inlineCompletionsModel.js'; import { GutterIndicatorMenuContent } from './gutterIndicatorMenu.js'; import { mapOutFalsy, n, rectToProps } from './utils.js'; import { localize } from '../../../../../../nls.js'; +import { trackFocus } from '../../../../../../base/browser/dom.js'; export const inlineEditIndicatorPrimaryForeground = registerColor( 'inlineEdit.gutterIndicator.primaryForeground', buttonForeground, @@ -73,6 +74,7 @@ export class InlineEditsGutterIndicator extends Disposable { private readonly _originalRange: IObservable, private readonly _model: IObservable, private readonly _shouldShowHover: IObservable, + private readonly _focusIsInMenu: ISettableObservable, @IHoverService private readonly _hoverService: HoverService, @IInstantiationService private readonly _instantiationService: IInstantiationService, ) { @@ -158,12 +160,18 @@ export class InlineEditsGutterIndicator extends Disposable { if (this._layout.map(d => d && d.docked).read(reader)) { return { selectionOverride: 'accept' as const, - action: () => { this._model.get()?.accept(); } + action: () => { + this._editorObs.editor.focus(); + this._model.get()?.accept(); + } }; } else { return { selectionOverride: 'jump' as const, - action: () => { this._model.get()?.jump(); } + action: () => { + this._editorObs.editor.focus(); + this._model.get()?.jump(); + } }; } }); @@ -178,35 +186,43 @@ export class InlineEditsGutterIndicator extends Disposable { return; } - const content = this._instantiationService.createInstance( + const disposableStore = new DisposableStore(); + const content = disposableStore.add(this._instantiationService.createInstance( GutterIndicatorMenuContent, this._hoverSelectionOverride, (focusEditor) => { - h?.dispose(); if (focusEditor) { this._editorObs.editor.focus(); } + h?.dispose(); }, this._model.map((m, r) => m?.state.read(r)?.inlineCompletion?.inlineCompletion.source.inlineCompletions.commands), - ).toDisposableLiveElement(); + ).toDisposableLiveElement()); + + const focusTracker = disposableStore.add(trackFocus(content.element)); + disposableStore.add(focusTracker.onDidBlur(() => this._focusIsInMenu.set(false, undefined))); + disposableStore.add(focusTracker.onDidFocus(() => this._focusIsInMenu.set(true, undefined))); + disposableStore.add(toDisposable(() => this._focusIsInMenu.set(false, undefined))); + const h = this._hoverService.showHover({ target: this._iconRef.element, content: content.element, }) as HoverWidget | undefined; if (h) { this._hoverVisible = true; - h.onDispose(() => { - content.dispose(); + h.onDispose(() => { // TODO:@hediet fix leak + disposableStore.dispose(); this._hoverVisible = false; }); } else { - content.dispose(); + disposableStore.dispose(); } } private readonly _indicator = n.div({ class: 'inline-edits-view-gutter-indicator', onclick: () => this._onClickAction.get().action(), + tabIndex: 0, style: { position: 'absolute', overflow: 'visible', diff --git a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/view.ts b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/view.ts index 4282852af5316..8e01c35c5bcab 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/view.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/view.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Disposable } from '../../../../../../base/common/lifecycle.js'; -import { autorunWithStore, derived, IObservable, IReader, mapObservableArrayCached } from '../../../../../../base/common/observable.js'; +import { autorunWithStore, derived, IObservable, IReader, ISettableObservable, mapObservableArrayCached } from '../../../../../../base/common/observable.js'; import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js'; import { ICodeEditor } from '../../../../../browser/editorBrowser.js'; import { observableCodeEditor } from '../../../../../browser/observableCodeEditor.js'; @@ -37,6 +37,7 @@ export class InlineEditsView extends Disposable { private readonly _editor: ICodeEditor, private readonly _edit: IObservable, private readonly _model: IObservable, + private readonly _focusIsInMenu: ISettableObservable, @IInstantiationService private readonly _instantiationService: IInstantiationService, ) { super(); @@ -143,6 +144,7 @@ export class InlineEditsView extends Disposable { this._uiState.map(s => s && s.originalDisplayRange), this._model, this._sideBySide.isHovered, + this._focusIsInMenu, )); } else { store.add(new InlineEditsIndicator( diff --git a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/viewAndDiffProducer.ts b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/viewAndDiffProducer.ts index f56b4d2c9867b..a0e9d4d1268aa 100644 --- a/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/viewAndDiffProducer.ts +++ b/src/vs/editor/contrib/inlineCompletions/browser/view/inlineEdits/viewAndDiffProducer.ts @@ -8,7 +8,7 @@ import { CancellationToken } from '../../../../../../base/common/cancellation.js import { equalsIfDefined, itemEquals } from '../../../../../../base/common/equals.js'; import { createHotClass } from '../../../../../../base/common/hotReloadHelpers.js'; import { Disposable } from '../../../../../../base/common/lifecycle.js'; -import { derivedDisposable, ObservablePromise, derived, IObservable, derivedOpts } from '../../../../../../base/common/observable.js'; +import { derivedDisposable, ObservablePromise, derived, IObservable, derivedOpts, ISettableObservable } from '../../../../../../base/common/observable.js'; import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js'; import { ICodeEditor } from '../../../../../browser/editorBrowser.js'; import { IDiffProviderFactoryService } from '../../../../../browser/widget/diffEditor/diffProviderFactoryService.js'; @@ -99,13 +99,14 @@ export class InlineEditsViewAndDiffProducer extends Disposable { private readonly _editor: ICodeEditor, private readonly _edit: IObservable, private readonly _model: IObservable, + private readonly _focusIsInMenu: ISettableObservable, @IInstantiationService private readonly _instantiationService: IInstantiationService, @IDiffProviderFactoryService private readonly _diffProviderFactoryService: IDiffProviderFactoryService, @IModelService private readonly _modelService: IModelService ) { super(); - this._register(this._instantiationService.createInstance(InlineEditsView, this._editor, this._inlineEdit, this._model)); + this._register(this._instantiationService.createInstance(InlineEditsView, this._editor, this._inlineEdit, this._model, this._focusIsInMenu)); } }