Skip to content

Don't dismiss hover by mouse movement if it is triggered from the keyboard #164446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alexdima merged 1 commit into from
Oct 24, 2022
Merged

Don't dismiss hover by mouse movement if it is triggered from the keyboard #164446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/vs/editor/contrib/colorPicker/browser/colorContributions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { IEditorContribution } from 'vs/editor/common/editorCommon';
import { ColorDecorationInjectedTextMarker } from 'vs/editor/contrib/colorPicker/browser/colorDetector';
import { ColorHoverParticipant } from 'vs/editor/contrib/colorPicker/browser/colorHoverParticipant';
import { ModesHoverController } from 'vs/editor/contrib/hover/browser/hover';
import { HoverStartMode } from 'vs/editor/contrib/hover/browser/hoverOperation';
import { HoverStartMode, HoverStartSource } from 'vs/editor/contrib/hover/browser/hoverOperation';
import { HoverParticipantRegistry } from 'vs/editor/contrib/hover/browser/hoverTypes';

export class ColorContribution extends Disposable implements IEditorContribution {
Expand Down Expand Up @@ -55,7 +55,7 @@ export class ColorContribution extends Disposable implements IEditorContribution
}
if (!hoverController.isColorPickerVisible()) {
const range = new Range(target.range.startLineNumber, target.range.startColumn + 1, target.range.endLineNumber, target.range.endColumn + 1);
hoverController.showContentHover(range, HoverStartMode.Immediate, false);
hoverController.showContentHover(range, HoverStartMode.Immediate, HoverStartSource.Mouse, false);
}
}
}
Expand Down
37 changes: 26 additions & 11 deletions src/vs/editor/contrib/hover/browser/contentHover.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { Range } from 'vs/editor/common/core/range';
import { IModelDecoration, PositionAffinity } from 'vs/editor/common/model';
import { ModelDecorationOptions } from 'vs/editor/common/model/textModel';
import { TokenizationRegistry } from 'vs/editor/common/languages';
import { HoverOperation, HoverStartMode, IHoverComputer } from 'vs/editor/contrib/hover/browser/hoverOperation';
import { HoverOperation, HoverStartMode, HoverStartSource, IHoverComputer } from 'vs/editor/contrib/hover/browser/hoverOperation';
import { HoverAnchor, HoverAnchorType, HoverParticipantRegistry, HoverRangeAnchor, IEditorHoverColorPickerWidget, IEditorHoverAction, IEditorHoverParticipant, IEditorHoverRenderContext, IEditorHoverStatusBar, IHoverPart } from 'vs/editor/contrib/hover/browser/hoverTypes';
import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
Expand Down Expand Up @@ -104,25 +104,25 @@ export class ContentHoverController extends Disposable {
}

if (anchorCandidates.length === 0) {
return this._startShowingOrUpdateHover(null, HoverStartMode.Delayed, false, mouseEvent);
return this._startShowingOrUpdateHover(null, HoverStartMode.Delayed, HoverStartSource.Mouse, false, mouseEvent);
}

anchorCandidates.sort((a, b) => b.priority - a.priority);
return this._startShowingOrUpdateHover(anchorCandidates[0], HoverStartMode.Delayed, false, mouseEvent);
return this._startShowingOrUpdateHover(anchorCandidates[0], HoverStartMode.Delayed, HoverStartSource.Mouse, false, mouseEvent);
}

public startShowingAtRange(range: Range, mode: HoverStartMode, focus: boolean): void {
this._startShowingOrUpdateHover(new HoverRangeAnchor(0, range, undefined, undefined), mode, focus, null);
public startShowingAtRange(range: Range, mode: HoverStartMode, source: HoverStartSource, focus: boolean): void {
this._startShowingOrUpdateHover(new HoverRangeAnchor(0, range, undefined, undefined), mode, source, focus, null);
}

/**
* Returns true if the hover shows now or will show.
*/
private _startShowingOrUpdateHover(anchor: HoverAnchor | null, mode: HoverStartMode, focus: boolean, mouseEvent: IEditorMouseEvent | null): boolean {
private _startShowingOrUpdateHover(anchor: HoverAnchor | null, mode: HoverStartMode, source: HoverStartSource, focus: boolean, mouseEvent: IEditorMouseEvent | null): boolean {
if (!this._widget.position || !this._currentResult) {
// The hover is not visible
if (anchor) {
this._startHoverOperationIfNecessary(anchor, mode, focus, false);
this._startHoverOperationIfNecessary(anchor, mode, source, focus, false);
return true;
}
return false;
Expand All @@ -135,7 +135,7 @@ export class ContentHoverController extends Disposable {
// The mouse is getting closer to the hover, so we will keep the hover untouched
// But we will kick off a hover update at the new anchor, insisting on keeping the hover visible.
if (anchor) {
this._startHoverOperationIfNecessary(anchor, mode, focus, true);
this._startHoverOperationIfNecessary(anchor, mode, source, focus, true);
}
return true;
}
Expand All @@ -153,18 +153,18 @@ export class ContentHoverController extends Disposable {
if (!anchor.canAdoptVisibleHover(this._currentResult.anchor, this._widget.position)) {
// The new anchor is not compatible with the previous anchor
this._setCurrentResult(null);
this._startHoverOperationIfNecessary(anchor, mode, focus, false);
this._startHoverOperationIfNecessary(anchor, mode, source, focus, false);
return true;
}

// We aren't getting any closer to the hover, so we will filter existing results
// and keep those which also apply to the new anchor.
this._setCurrentResult(this._currentResult.filter(anchor));
this._startHoverOperationIfNecessary(anchor, mode, focus, false);
this._startHoverOperationIfNecessary(anchor, mode, source, focus, false);
return true;
}

private _startHoverOperationIfNecessary(anchor: HoverAnchor, mode: HoverStartMode, focus: boolean, insistOnKeepingHoverVisible: boolean): void {
private _startHoverOperationIfNecessary(anchor: HoverAnchor, mode: HoverStartMode, source: HoverStartSource, focus: boolean, insistOnKeepingHoverVisible: boolean): void {
if (this._computer.anchor && this._computer.anchor.equals(anchor)) {
// We have to start a hover operation at the exact same anchor as before, so no work is needed
return;
Expand All @@ -173,6 +173,7 @@ export class ContentHoverController extends Disposable {
this._hoverOperation.cancel();
this._computer.anchor = anchor;
this._computer.shouldFocus = focus;
this._computer.source = source;
this._computer.insistOnKeepingHoverVisible = insistOnKeepingHoverVisible;
this._hoverOperation.start(mode);
}
Expand Down Expand Up @@ -203,6 +204,10 @@ export class ContentHoverController extends Disposable {
return this._widget.isColorPickerVisible;
}

public isVisibleFromKeyboard(): boolean {
return this._widget.isVisibleFromKeyboard;
}

public containsNode(node: Node): boolean {
return this._widget.getDomNode().contains(node);
}
Expand Down Expand Up @@ -286,6 +291,7 @@ export class ContentHoverController extends Disposable {
showAtSecondaryPosition,
this._editor.getOption(EditorOption.hover).above,
this._computer.shouldFocus,
this._computer.source,
isBeforeContent,
anchor.initialMousePosX,
anchor.initialMousePosY,
Expand Down Expand Up @@ -379,6 +385,7 @@ class ContentHoverVisibleData {
public readonly showAtSecondaryPosition: Position,
public readonly preferAbove: boolean,
public readonly stoleFocus: boolean,
public readonly source: HoverStartSource,
public readonly isBeforeContent: boolean,
public initialMousePosX: number | undefined,
public initialMousePosY: number | undefined,
Expand Down Expand Up @@ -408,6 +415,10 @@ export class ContentHoverWidget extends Disposable implements IContentWidget {
return Boolean(this._visibleData?.colorPicker);
}

public get isVisibleFromKeyboard(): boolean {
return (this._visibleData?.source === HoverStartSource.Keyboard);
}

constructor(
private readonly _editor: ICodeEditor,
@IContextKeyService private readonly _contextKeyService: IContextKeyService,
Expand Down Expand Up @@ -613,6 +624,10 @@ class ContentHoverComputer implements IHoverComputer<IHoverPart> {
public get shouldFocus(): boolean { return this._shouldFocus; }
public set shouldFocus(value: boolean) { this._shouldFocus = value; }

private _source: HoverStartSource = HoverStartSource.Mouse;
public get source(): HoverStartSource { return this._source; }
public set source(value: HoverStartSource) { this._source = value; }

private _insistOnKeepingHoverVisible: boolean = false;
public get insistOnKeepingHoverVisible(): boolean { return this._insistOnKeepingHoverVisible; }
public set insistOnKeepingHoverVisible(value: boolean) { this._insistOnKeepingHoverVisible = value; }
Expand Down
17 changes: 12 additions & 5 deletions src/vs/editor/contrib/hover/browser/hover.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { IEditorContribution, IScrollEvent } from 'vs/editor/common/editorCommon
import { EditorContextKeys } from 'vs/editor/common/editorContextKeys';
import { ILanguageService } from 'vs/editor/common/languages/language';
import { GotoDefinitionAtPositionEditorContribution } from 'vs/editor/contrib/gotoSymbol/browser/link/goToDefinitionAtPosition';
import { HoverStartMode } from 'vs/editor/contrib/hover/browser/hoverOperation';
import { HoverStartMode, HoverStartSource } from 'vs/editor/contrib/hover/browser/hoverOperation';
import { ContentHoverWidget, ContentHoverController } from 'vs/editor/contrib/hover/browser/contentHover';
import { MarginHoverWidget } from 'vs/editor/contrib/hover/browser/marginHover';
import * as nls from 'vs/nls';
Expand Down Expand Up @@ -172,6 +172,13 @@ export class ModesHoverController implements IEditorContribution {
}

const contentWidget = this._getOrCreateContentWidget();

if (this._isHoverSticky && contentWidget.isVisibleFromKeyboard()) {
// Sticky mode is on and the hover has been shown via keyboard
// so moving the mouse has no effect
return;
}

if (contentWidget.maybeShowAt(mouseEvent)) {
this._glyphWidget?.hide();
return;
Expand Down Expand Up @@ -217,8 +224,8 @@ export class ModesHoverController implements IEditorContribution {
return this._contentWidget?.isColorPickerVisible() || false;
}

public showContentHover(range: Range, mode: HoverStartMode, focus: boolean): void {
this._getOrCreateContentWidget().startShowingAtRange(range, mode, focus);
public showContentHover(range: Range, mode: HoverStartMode, source: HoverStartSource, focus: boolean): void {
this._getOrCreateContentWidget().startShowingAtRange(range, mode, source, focus);
}

public dispose(): void {
Expand Down Expand Up @@ -263,7 +270,7 @@ class ShowHoverAction extends EditorAction {
const position = editor.getPosition();
const range = new Range(position.lineNumber, position.column, position.lineNumber, position.column);
const focus = editor.getOption(EditorOption.accessibilitySupport) === AccessibilitySupport.Enabled;
controller.showContentHover(range, HoverStartMode.Immediate, focus);
controller.showContentHover(range, HoverStartMode.Immediate, HoverStartSource.Keyboard, focus);
}
}

Expand Down Expand Up @@ -302,7 +309,7 @@ class ShowDefinitionPreviewHoverAction extends EditorAction {
}
const promise = goto.startFindDefinitionFromCursor(position);
promise.then(() => {
controller.showContentHover(range, HoverStartMode.Immediate, true);
controller.showContentHover(range, HoverStartMode.Immediate, HoverStartSource.Keyboard, true);
});
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/vs/editor/contrib/hover/browser/hoverOperation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ export const enum HoverStartMode {
Immediate = 1
}

export const enum HoverStartSource {
Mouse = 0,
Keyboard = 1
}

export class HoverResult<T> {
constructor(
public readonly value: T[],
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/debug/browser/debugEditorContribution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { Disposable, dispose, IDisposable } from 'vs/base/common/lifecycle';
import { EditOperation } from 'vs/editor/common/core/editOperation';
import { basename } from 'vs/base/common/path';
import { ModesHoverController } from 'vs/editor/contrib/hover/browser/hover';
import { HoverStartMode } from 'vs/editor/contrib/hover/browser/hoverOperation';
import { HoverStartMode, HoverStartSource } from 'vs/editor/contrib/hover/browser/hoverOperation';
import { IHostService } from 'vs/workbench/services/host/browser/host';
import { Event } from 'vs/base/common/event';
import { IUriIdentityService } from 'vs/platform/uriIdentity/common/uriIdentity';
Expand Down Expand Up @@ -349,7 +349,7 @@ export class DebugEditorContribution implements IDebugEditorContribution {
// If the debug hover was visible immediately show the editor hover for the alt transition to be smooth
const hoverController = this.editor.getContribution<ModesHoverController>(ModesHoverController.ID);
const range = new Range(this.hoverPosition.lineNumber, this.hoverPosition.column, this.hoverPosition.lineNumber, this.hoverPosition.column);
hoverController?.showContentHover(range, HoverStartMode.Immediate, false);
hoverController?.showContentHover(range, HoverStartMode.Immediate, HoverStartSource.Mouse, false);
}

const onKeyUp = new DomEmitter(document, 'keyup');
Expand Down