From 4dfdf2adf080f98e8efc73799d5852bb13e2e16b Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 28 Mar 2023 15:04:27 -0700 Subject: [PATCH] Improve hover behavior when not enough space This changes the hover's position fallback logic from: If not enough room on right Position left If not enough room on left Position right To: If not enough room on right If enough room on left Position left Else Position below If not enough room on left If enough room on right Position right Else Position below A new option showHoverHint is also added as part of this which will show a hint to the user to hold alt/option in order to mouse over the hover (only when the evaluated hover is true). This is an explicit setting such that it doesn't show up on hovers such as the activity bar which looks awkward. Fixes #176704 --- src/vs/base/browser/ui/hover/hover.css | 5 ++ .../workbench/services/hover/browser/hover.ts | 8 ++++ .../services/hover/browser/hoverWidget.ts | 48 ++++++++++++++++--- .../quickinput/browser/quickInputService.ts | 2 +- 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/src/vs/base/browser/ui/hover/hover.css b/src/vs/base/browser/ui/hover/hover.css index 5cd824a3d76..f3058d6c107 100644 --- a/src/vs/base/browser/ui/hover/hover.css +++ b/src/vs/base/browser/ui/hover/hover.css @@ -114,6 +114,11 @@ line-height: 22px; } +.monaco-hover .hover-row.status-bar .info { + font-style: italic; + padding: 0px 8px; +} + .monaco-hover .hover-row.status-bar .actions { display: flex; padding: 0px 8px; diff --git a/src/vs/workbench/services/hover/browser/hover.ts b/src/vs/workbench/services/hover/browser/hover.ts index 1f1de94425a..1c850be8540 100644 --- a/src/vs/workbench/services/hover/browser/hover.ts +++ b/src/vs/workbench/services/hover/browser/hover.ts @@ -93,6 +93,14 @@ export interface IHoverOptions { */ hideOnHover?: boolean; + /** + * When {@link hideOnHover} is explicitly true or undefined and its auto value is detected to + * hide, show a hint at the bottom of the hover explaining how to mouse over the widget. This + * should be used in the cases where despite the hover having no interactive content, it's + * likely the user may want to interact with it somehow. + */ + showHoverHint?: boolean; + /** * Whether to hide the hover when a key is pressed. */ diff --git a/src/vs/workbench/services/hover/browser/hoverWidget.ts b/src/vs/workbench/services/hover/browser/hoverWidget.ts index b227a4ac141..0dc0d9f4830 100644 --- a/src/vs/workbench/services/hover/browser/hoverWidget.ts +++ b/src/vs/workbench/services/hover/browser/hoverWidget.ts @@ -18,6 +18,8 @@ import { IOpenerService } from 'vs/platform/opener/common/opener'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { MarkdownRenderer, openLinkFromMarkdown } from 'vs/editor/contrib/markdownRenderer/browser/markdownRenderer'; import { isMarkdownString } from 'vs/base/common/htmlContent'; +import { localize } from 'vs/nls'; +import { isMacintosh } from 'vs/base/common/platform'; const $ = dom.$; type TargetRect = { @@ -191,19 +193,31 @@ export class HoverWidget extends Widget { } this._hoverContainer.appendChild(this._hover.containerDomNode); + // Determine whether to hide on hover let hideOnHover: boolean; if (options.actions && options.actions.length > 0) { // If there are actions, require hover so they can be accessed hideOnHover = false; } else { if (options.hideOnHover === undefined) { - // Defaults to true when string, false when markdown as it may contain links - hideOnHover = typeof options.content === 'string'; + // When unset, will default to true when it's a string or when it's markdown that + // appears to have a link using a naive check for '](' + hideOnHover = typeof options.content === 'string' || isMarkdownString(options.content) && !options.content.value.includes(']('); } else { // It's set explicitly hideOnHover = options.hideOnHover; } } + + // Show the hover hint if needed + if (hideOnHover && options.showHoverHint) { + const statusBarElement = $('div.hover-row.status-bar'); + const infoElement = $('div.info'); + infoElement.textContent = localize('hoverhint', 'Hold {0} key to mouse over', isMacintosh ? 'Option' : 'Alt'); + statusBarElement.appendChild(infoElement); + this._hover.containerDomNode.appendChild(statusBarElement); + } + const mouseTrackerTargets = [...this._target.targetElements]; if (!hideOnHover) { mouseTrackerTargets.push(this._hoverContainer); @@ -453,14 +467,36 @@ export class HoverWidget extends Widget { // Position hover on right to target if (this._hoverPosition === HoverPosition.RIGHT) { + const roomOnRight = document.documentElement.clientWidth - target.right; // Hover on the right is going beyond window. - if (target.right + this._hover.containerDomNode.clientWidth >= document.documentElement.clientWidth) { - this._hoverPosition = HoverPosition.LEFT; + if (roomOnRight < this._hover.containerDomNode.clientWidth) { + const roomOnLeft = target.left; + // There's enough room on the left, flip the hover position + if (roomOnLeft >= this._hover.containerDomNode.clientWidth) { + this._hoverPosition = HoverPosition.LEFT; + } + // Hover on the left would go beyond window too + else { + this._hoverPosition = HoverPosition.BELOW; + } } } - // Position hover on left to target - if (this._hoverPosition === HoverPosition.LEFT) { + else if (this._hoverPosition === HoverPosition.LEFT) { + + const roomOnLeft = target.left; + // Hover on the left is going beyond window. + if (roomOnLeft < this._hover.containerDomNode.clientWidth) { + const roomOnRight = document.documentElement.clientWidth - target.right; + // There's enough room on the right, flip the hover position + if (roomOnRight >= this._hover.containerDomNode.clientWidth) { + this._hoverPosition = HoverPosition.RIGHT; + } + // Hover on the right would go beyond window too + else { + this._hoverPosition = HoverPosition.BELOW; + } + } // Hover on the left is going beyond window. if (target.left - this._hover.containerDomNode.clientWidth <= document.documentElement.clientLeft) { this._hoverPosition = HoverPosition.RIGHT; diff --git a/src/vs/workbench/services/quickinput/browser/quickInputService.ts b/src/vs/workbench/services/quickinput/browser/quickInputService.ts index 77498318db2..4e511787aa4 100644 --- a/src/vs/workbench/services/quickinput/browser/quickInputService.ts +++ b/src/vs/workbench/services/quickinput/browser/quickInputService.ts @@ -72,7 +72,7 @@ class QuickInputHoverDelegate implements IHoverDelegate { showHover(options: IHoverDelegateOptions, focus?: boolean): IHoverWidget | undefined { return this.hoverService.showHover({ ...options, - hideOnHover: false, + showHoverHint: true, hideOnKeyDown: false, skipFadeInAnimation: true, }, focus);