From 94fdadbc3486331e1b67dabb9bb329a00607b2ac Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 1 Sep 2020 07:19:33 -0700 Subject: [PATCH] Support string in IHoverOptions.text Fixes #105841 --- .../workbench/services/hover/browser/hover.ts | 14 ++++++++++---- .../services/hover/browser/hoverWidget.ts | 19 +++++++++++++++++-- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/services/hover/browser/hover.ts b/src/vs/workbench/services/hover/browser/hover.ts index f696ebd64e7..12c573dd7e4 100644 --- a/src/vs/workbench/services/hover/browser/hover.ts +++ b/src/vs/workbench/services/hover/browser/hover.ts @@ -39,9 +39,10 @@ export interface IHoverService { export interface IHoverOptions { /** - * The text to display in the primary section of the hover. + * The text to display in the primary section of the hover. The type of text determines the + * default `hideOnHover` behavior. */ - text: IMarkdownString; + text: IMarkdownString | string; /** * The target for the hover. This determines the position of the hover and it will only be @@ -69,8 +70,13 @@ export interface IHoverOptions { /** * Whether to hide the hover when the mouse leaves the `target` and enters the actual hover. - * This is false by default and note that it will be ignored if any `actions` are provided such - * that they are accessible. + * This is false by default when text is an `IMarkdownString` and true when `text` is a + * `string`. Note that this will be ignored if any `actions` are provided as hovering is + * required to make them accessible. + * + * In general hiding on hover is desired for: + * - Regular text where selection is not important + * - Markdown that contains no links where selection is not important */ hideOnHover?: boolean; } diff --git a/src/vs/workbench/services/hover/browser/hoverWidget.ts b/src/vs/workbench/services/hover/browser/hoverWidget.ts index c800d270500..5ad588d6017 100644 --- a/src/vs/workbench/services/hover/browser/hoverWidget.ts +++ b/src/vs/workbench/services/hover/browser/hoverWidget.ts @@ -17,6 +17,7 @@ import { Widget } from 'vs/base/browser/ui/widget'; import { AnchorPosition } from 'vs/base/browser/ui/contextview/contextview'; import { IOpenerService } from 'vs/platform/opener/common/opener'; import { IWorkbenchLayoutService } from 'vs/workbench/services/layout/browser/layoutService'; +import { MarkdownString } from 'vs/base/common/htmlContent'; const $ = dom.$; @@ -78,7 +79,8 @@ export class HoverWidget extends Widget { const rowElement = $('div.hover-row.markdown-hover'); const contentsElement = $('div.hover-contents'); - const markdownElement = renderMarkdown(options.text, { + const markdown = typeof options.text === 'string' ? new MarkdownString(options.text) : options.text; + const markdownElement = renderMarkdown(markdown, { actionHandler: { callback: (content) => this._linkHandler(content), disposeables: this._messageListeners @@ -118,7 +120,20 @@ export class HoverWidget extends Widget { } const mouseTrackerTargets = [...this._target.targetElements]; - if (!options.hideOnHover || (options.actions && options.actions.length > 0)) { + let hideOnHover: boolean; + if (options.hideOnHover === undefined) { + if (options.actions && options.actions.length > 0) { + // If there are actions, require hover so they can be accessed + hideOnHover = false; + } else { + // Defaults to true when string, false when markdown as it may contain links + hideOnHover = typeof options.text === 'string'; + } + } else { + // It's set explicitly + hideOnHover = options.hideOnHover; + } + if (!hideOnHover) { mouseTrackerTargets.push(this._hover.containerDomNode); } this._mouseTracker = new CompositeMouseTracker(mouseTrackerTargets);