From 44983e178097145924348c24af251f6a17769b0b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 Jun 2020 04:43:03 -0700 Subject: [PATCH 01/11] Partially working hover service Part of #97496 --- .../hover/browser/hover.contribution.ts | 43 +++++++ .../workbench/contrib/hover/browser/hover.ts | 63 ++++++++++ .../contrib/hover/browser/hoverService.ts | 46 +++++++ .../widgets => hover/browser}/hoverWidget.ts | 116 +++++------------- .../contrib/hover/browser/media/hover.css | 27 ++++ .../browser/links/terminalLinkManager.ts | 2 +- .../terminal/browser/media/widgets.css | 23 ---- .../widgets/environmentVariableInfoWidget.ts | 14 ++- .../browser/widgets/terminalHoverWidget.ts | 90 ++++++++------ .../terminal/browser/widgets/widgets.ts | 29 ----- src/vs/workbench/workbench.common.main.ts | 3 + 11 files changed, 277 insertions(+), 179 deletions(-) create mode 100644 src/vs/workbench/contrib/hover/browser/hover.contribution.ts create mode 100644 src/vs/workbench/contrib/hover/browser/hover.ts create mode 100644 src/vs/workbench/contrib/hover/browser/hoverService.ts rename src/vs/workbench/contrib/{terminal/browser/widgets => hover/browser}/hoverWidget.ts (56%) create mode 100644 src/vs/workbench/contrib/hover/browser/media/hover.css diff --git a/src/vs/workbench/contrib/hover/browser/hover.contribution.ts b/src/vs/workbench/contrib/hover/browser/hover.contribution.ts new file mode 100644 index 0000000000000..372c4a9be8b98 --- /dev/null +++ b/src/vs/workbench/contrib/hover/browser/hover.contribution.ts @@ -0,0 +1,43 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import 'vs/css!./media/hover'; +import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; +import { HoverService } from 'vs/workbench/contrib/hover/browser/hoverService'; +import { IHoverService } from 'vs/workbench/contrib/hover/browser/hover'; +import { registerThemingParticipant } from 'vs/platform/theme/common/themeService'; +import { editorHoverBackground, editorHoverBorder, textLinkForeground, editorHoverForeground, editorHoverStatusBarBackground, textCodeBlockBackground } from 'vs/platform/theme/common/colorRegistry'; + +registerSingleton(IHoverService, HoverService, true); + +registerThemingParticipant((theme, collector) => { + const hoverBackground = theme.getColor(editorHoverBackground); + if (hoverBackground) { + collector.addRule(`.monaco-workbench .workbench-hover { background-color: ${hoverBackground}; }`); + } + const hoverBorder = theme.getColor(editorHoverBorder); + if (hoverBorder) { + collector.addRule(`.monaco-workbench .workbench-hover { border: 1px solid ${hoverBorder}; }`); + collector.addRule(`.monaco-workbench .workbench-hover .hover-row:not(:first-child):not(:empty) { border-top: 1px solid ${hoverBorder.transparent(0.5)}; }`); + collector.addRule(`.monaco-workbench .workbench-hover hr { border-top: 1px solid ${hoverBorder.transparent(0.5)}; }`); + collector.addRule(`.monaco-workbench .workbench-hover hr { border-bottom: 0px solid ${hoverBorder.transparent(0.5)}; }`); + } + const link = theme.getColor(textLinkForeground); + if (link) { + collector.addRule(`.monaco-workbench .workbench-hover a { color: ${link}; }`); + } + const hoverForeground = theme.getColor(editorHoverForeground); + if (hoverForeground) { + collector.addRule(`.monaco-workbench .workbench-hover { color: ${hoverForeground}; }`); + } + const actionsBackground = theme.getColor(editorHoverStatusBarBackground); + if (actionsBackground) { + collector.addRule(`.monaco-workbench .workbench-hover .hover-row .actions { background-color: ${actionsBackground}; }`); + } + const codeBackground = theme.getColor(textCodeBlockBackground); + if (codeBackground) { + collector.addRule(`.monaco-workbench .workbench-hover code { background-color: ${codeBackground}; }`); + } +}); diff --git a/src/vs/workbench/contrib/hover/browser/hover.ts b/src/vs/workbench/contrib/hover/browser/hover.ts new file mode 100644 index 0000000000000..4561bf43390e8 --- /dev/null +++ b/src/vs/workbench/contrib/hover/browser/hover.ts @@ -0,0 +1,63 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; +import { IDisposable } from 'vs/base/common/lifecycle'; +import { IMarkdownString } from 'vs/base/common/htmlContent'; + +export const IHoverService = createDecorator('hoverService'); + +export interface IHoverService { + readonly _serviceBrand: undefined; + + showHover(options: IHoverOptions): void; + hideHover(): void; +} + +export interface IHoverOptions { + /** + * The text to display in the primary section of the hover. + */ + text: IMarkdownString; + + // TODO: Link handler not necessary? + linkHandler: (url: string) => void; + + /** + * A hover target holding 1+ elements that will hide the hover when the mouse leaves any of the + * elements or the hover. When this is not provided, only mouseout on the hover will hide the + * hover. + */ + target: IHoverTarget; + + /** + * A set of actions for the hover's "status bar". + */ + actions?: IHoverAction[]; +} + +export interface IHoverAction { + label: string; + iconClass?: string; + run: (target: HTMLElement) => void; + commandId: string; +} + +/** + * A target for a hover which can know about domain-specific locations. + */ +export interface IHoverTarget extends IDisposable { + readonly targetElements: readonly HTMLElement[]; + readonly anchor: IHoverAnchor; +} + +export interface IHoverAnchor { + x: number; + y: number; + /** + * Fallback Y value to try with opposite VerticalAlignment if the hover does not fit vertically. + */ + fallbackY: number; +} diff --git a/src/vs/workbench/contrib/hover/browser/hoverService.ts b/src/vs/workbench/contrib/hover/browser/hoverService.ts new file mode 100644 index 0000000000000..040ca144c7f77 --- /dev/null +++ b/src/vs/workbench/contrib/hover/browser/hoverService.ts @@ -0,0 +1,46 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { IHoverService, IHoverOptions } from 'vs/workbench/contrib/hover/browser/hover'; +import { IContextViewService } from 'vs/platform/contextview/browser/contextView'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { HoverWidget } from 'vs/workbench/contrib/hover/browser/hoverWidget'; +import { IContextViewProvider, AnchorPosition } from 'vs/base/browser/ui/contextview/contextview'; + +export class HoverService implements IHoverService { + declare readonly _serviceBrand: undefined; + + constructor( + @IInstantiationService private readonly _instantiationService: IInstantiationService, + @IContextViewService private readonly _contextViewService: IContextViewService + ) { + } + + showHover(options: IHoverOptions): void { + // TODO: Prevent hover when the same one is already visible + const hover = this._instantiationService.createInstance(HoverWidget, options.target, options.text, options.linkHandler, []); + const provider = this._contextViewService as IContextViewProvider; + provider.showContextView({ + render: container => { + hover.render(container); + return hover; + }, + anchorPosition: AnchorPosition.ABOVE, + getAnchor: () => { + console.log('anchor', options.target!.anchor); + if (options.target) { + } + return { + x: hover.x, + y: hover.y + }; + } + }); + } + + hideHover(): void { + this._contextViewService.hideContextView(); + } +} diff --git a/src/vs/workbench/contrib/terminal/browser/widgets/hoverWidget.ts b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts similarity index 56% rename from src/vs/workbench/contrib/terminal/browser/widgets/hoverWidget.ts rename to src/vs/workbench/contrib/hover/browser/hoverWidget.ts index 5a5e1d5e307ce..a67b60c5c7896 100644 --- a/src/vs/workbench/contrib/terminal/browser/widgets/hoverWidget.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts @@ -7,16 +7,15 @@ import { DisposableStore } from 'vs/base/common/lifecycle'; import { IMarkdownString } from 'vs/base/common/htmlContent'; import { renderMarkdown } from 'vs/base/browser/markdownRenderer'; import { Event, Emitter } from 'vs/base/common/event'; -import { registerThemingParticipant } from 'vs/platform/theme/common/themeService'; -import { editorHoverHighlight, editorHoverBackground, editorHoverBorder, textLinkForeground, editorHoverForeground, editorHoverStatusBarBackground, textCodeBlockBackground } from 'vs/platform/theme/common/colorRegistry'; import * as dom from 'vs/base/browser/dom'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; -import { IHoverTarget, HorizontalAnchorSide, VerticalAnchorSide } from 'vs/workbench/contrib/terminal/browser/widgets/widgets'; +import { IHoverTarget } from 'vs/workbench/contrib/hover/browser/hover'; import { KeyCode } from 'vs/base/common/keyCodes'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { EDITOR_FONT_DEFAULTS, IEditorOptions } from 'vs/editor/common/config/editorOptions'; import { HoverWidget as BaseHoverWidget, renderHoverAction } from 'vs/base/browser/ui/hover/hoverWidget'; import { Widget } from 'vs/base/browser/ui/widget'; +import { AnchorPosition } from 'vs/base/browser/ui/contextview/contextview'; const $ = dom.$; @@ -27,6 +26,9 @@ export class HoverWidget extends Widget { private readonly _hover: BaseHoverWidget; private _isDisposed: boolean = false; + private _anchor: AnchorPosition = AnchorPosition.ABOVE; + private _x: number = 0; + private _y: number = 0; get isDisposed(): boolean { return this._isDisposed; } get domNode(): HTMLElement { return this._hover.containerDomNode; } @@ -34,8 +36,11 @@ export class HoverWidget extends Widget { private readonly _onDispose = new Emitter(); get onDispose(): Event { return this._onDispose.event; } + get anchor(): AnchorPosition { return this._anchor; } + get x(): number { return this._x; } + get y(): number { return this._y; } + constructor( - private _container: HTMLElement, private _target: IHoverTarget, private _text: IMarkdownString, private _linkHandler: (url: string) => void, @@ -46,7 +51,8 @@ export class HoverWidget extends Widget { super(); this._hover = this._register(new BaseHoverWidget()); - this._hover.containerDomNode.classList.add('terminal-hover-widget', 'fadeIn', 'xterm-hover'); + // TODO: Move xterm-hover into terminal + this._hover.containerDomNode.classList.add('workbench-hover', 'fadeIn', 'xterm-hover'); // Don't allow mousedown out of the widget, otherwise preventDefault will call and text will // not be selected. @@ -72,7 +78,7 @@ export class HoverWidget extends Widget { }, codeBlockRenderCallback: () => { contentsElement.classList.add('code-hover-contents'); - this.layout(); + this.render(); } }); contentsElement.appendChild(markdownElement); @@ -94,53 +100,37 @@ export class HoverWidget extends Widget { this._mouseTracker = new CompositeMouseTracker([this._hover.containerDomNode, ..._target.targetElements]); this._register(this._mouseTracker.onMouseOut(() => this.dispose())); this._register(this._mouseTracker); - - this._container.appendChild(this._hover.containerDomNode); - - this.layout(); } - public layout(): void { - const anchor = this._target.anchor; + public render(container?: HTMLElement): void { + if (this._hover.containerDomNode.parentElement !== container) { + container?.appendChild(this._hover.containerDomNode); + } this._hover.containerDomNode.classList.remove('right-aligned'); this._hover.contentsDomNode.style.maxHeight = ''; - if (anchor.horizontalAnchorSide === HorizontalAnchorSide.Left) { - if (anchor.x + this._hover.containerDomNode.clientWidth > document.documentElement.clientWidth) { - // Shift the hover to the left when part of it would get cut off - const width = Math.round(this._hover.containerDomNode.clientWidth); - this._hover.containerDomNode.style.width = `${width - 1}px`; - this._hover.containerDomNode.style.maxWidth = ''; - const left = document.documentElement.clientWidth - width - 1; - this._hover.containerDomNode.style.left = `${left}px`; - // Right align if the right edge is closer to the anchor than the left edge - if (left + width / 2 < anchor.x) { - this._hover.containerDomNode.classList.add('right-aligned'); - } - } else { - this._hover.containerDomNode.style.width = ''; - this._hover.containerDomNode.style.maxWidth = `${document.documentElement.clientWidth - anchor.x - 1}px`; - this._hover.containerDomNode.style.left = `${anchor.x}px`; - } + + // Get horizontal alignment and position + const targetBounds = this._target.targetElements.map(e => e.getBoundingClientRect()); + const targetLeft = Math.min(...targetBounds.map(e => e.left)); + if (targetLeft + this._hover.containerDomNode.clientWidth >= document.documentElement.clientWidth) { + // TODO: Communicate horizontal alignment + this._x = document.documentElement.clientWidth; + this._hover.containerDomNode.classList.add('right-aligned'); } else { - this._hover.containerDomNode.style.right = `${anchor.x}px`; + this._x = targetLeft; } - // Use fallback y value if there is not enough vertical space - if (anchor.verticalAnchorSide === VerticalAnchorSide.Bottom) { - if (anchor.y + this._hover.containerDomNode.clientHeight > document.documentElement.clientHeight) { - this._hover.containerDomNode.style.top = `${anchor.fallbackY}px`; - this._hover.contentsDomNode.style.maxHeight = `${document.documentElement.clientHeight - anchor.fallbackY}px`; - } else { - this._hover.containerDomNode.style.bottom = `${anchor.y}px`; - this._hover.containerDomNode.style.maxHeight = ''; - } + + // Get vertical alignment and position + const targetTop = Math.min(...targetBounds.map(e => e.top)); + if (targetTop - this._hover.containerDomNode.clientHeight < 0) { + // TODO: Cap max height + this._anchor = AnchorPosition.BELOW; + this._y = Math.max(...targetBounds.map(e => e.bottom)) - 2; } else { - if (anchor.y + this._hover.containerDomNode.clientHeight > document.documentElement.clientHeight) { - this._hover.containerDomNode.style.bottom = `${anchor.fallbackY}px`; - } else { - this._hover.containerDomNode.style.top = `${anchor.y}px`; - } + this._y = targetTop; } + this._hover.onContentsChanged(); } @@ -209,41 +199,3 @@ class CompositeMouseTracker extends Widget { } } } - - -registerThemingParticipant((theme, collector) => { - let editorHoverHighlightColor = theme.getColor(editorHoverHighlight); - if (editorHoverHighlightColor) { - if (editorHoverHighlightColor.isOpaque()) { - editorHoverHighlightColor = editorHoverHighlightColor.transparent(0.5); - } - collector.addRule(`.integrated-terminal .hoverHighlight { background-color: ${editorHoverHighlightColor}; }`); - } - const hoverBackground = theme.getColor(editorHoverBackground); - if (hoverBackground) { - collector.addRule(`.integrated-terminal .monaco-hover { background-color: ${hoverBackground}; }`); - } - const hoverBorder = theme.getColor(editorHoverBorder); - if (hoverBorder) { - collector.addRule(`.integrated-terminal .monaco-hover { border: 1px solid ${hoverBorder}; }`); - collector.addRule(`.integrated-terminal .monaco-hover .hover-row:not(:first-child):not(:empty) { border-top: 1px solid ${hoverBorder.transparent(0.5)}; }`); - collector.addRule(`.integrated-terminal .monaco-hover hr { border-top: 1px solid ${hoverBorder.transparent(0.5)}; }`); - collector.addRule(`.integrated-terminal .monaco-hover hr { border-bottom: 0px solid ${hoverBorder.transparent(0.5)}; }`); - } - const link = theme.getColor(textLinkForeground); - if (link) { - collector.addRule(`.integrated-terminal .monaco-hover a { color: ${link}; }`); - } - const hoverForeground = theme.getColor(editorHoverForeground); - if (hoverForeground) { - collector.addRule(`.integrated-terminal .monaco-hover { color: ${hoverForeground}; }`); - } - const actionsBackground = theme.getColor(editorHoverStatusBarBackground); - if (actionsBackground) { - collector.addRule(`.integrated-terminal .monaco-hover .hover-row .actions { background-color: ${actionsBackground}; }`); - } - const codeBackground = theme.getColor(textCodeBlockBackground); - if (codeBackground) { - collector.addRule(`.integrated-terminal .monaco-hover code { background-color: ${codeBackground}; }`); - } -}); diff --git a/src/vs/workbench/contrib/hover/browser/media/hover.css b/src/vs/workbench/contrib/hover/browser/media/hover.css new file mode 100644 index 0000000000000..7b193b9e658b4 --- /dev/null +++ b/src/vs/workbench/contrib/hover/browser/media/hover.css @@ -0,0 +1,27 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +.monaco-workbench .workbench-hover { + position: relative; + font-size: 14px; + line-height: 19px; + animation: fadein 100ms linear; + /* Must be higher than sash's z-index and terminal canvases */ + z-index: 40; + overflow: hidden; +} + +.monaco-workbench .workbench-hover a { + color: #3794ff; +} + +.monaco-workbench .workbench-hover.right-aligned .hover-row.status-bar .actions { + flex-direction: row-reverse; +} + +.monaco-workbench .workbench-hover.right-aligned .hover-row.status-bar .actions .action-container { + margin-right: 0; + margin-left: 16px; +} diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts index 208b05dbed92b..08c093358ee98 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts @@ -97,7 +97,7 @@ export class TerminalLinkManager extends DisposableStore { terminalDimensions, modifierDownCallback, modifierUpCallback - }, this._getLinkHoverString(link.text, link.label), (text) => link.activate(undefined, text), link); + }, this._getLinkHoverString(link.text + 'kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf lkjan sdflkjfdasnlkfadsjnlkjfdsabjlkhadfs jklbadsfljkbdfas ljkbfadslbkjhfdslbkhabldsf kjlbdfasbljkfd abjklsdfbkjl adsfblkadfj blakdsf bjdasflkj dbfslkjafd sjbdfslkjsfbkjfds', link.label), (text) => link.activate(undefined, text), link); } private _showHover( diff --git a/src/vs/workbench/contrib/terminal/browser/media/widgets.css b/src/vs/workbench/contrib/terminal/browser/media/widgets.css index d33cfe12a546a..51a9ab8eff272 100644 --- a/src/vs/workbench/contrib/terminal/browser/media/widgets.css +++ b/src/vs/workbench/contrib/terminal/browser/media/widgets.css @@ -12,29 +12,6 @@ overflow: visible; } -.monaco-workbench .terminal-hover-widget { - position: fixed; - font-size: 14px; - line-height: 19px; - animation: fadein 100ms linear; - /* Must be higher than sash's z-index and terminal canvases */ - z-index: 40; - overflow: hidden; -} - -.monaco-workbench .terminal-hover-widget a { - color: #3794ff; -} - -.monaco-workbench .terminal-hover-widget.right-aligned .hover-row.status-bar .actions { - flex-direction: row-reverse; -} - -.monaco-workbench .terminal-hover-widget.right-aligned .hover-row.status-bar .actions .action-container { - margin-right: 0; - margin-left: 16px; -} - .monaco-workbench .terminal-overlay-widget { position: absolute; left: 0; diff --git a/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts b/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts index 974d0777202fa..b005268b6c66f 100644 --- a/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts +++ b/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts @@ -6,9 +6,10 @@ import { Widget } from 'vs/base/browser/ui/widget'; import { IEnvironmentVariableInfo } from 'vs/workbench/contrib/terminal/common/environmentVariable'; import { MarkdownString } from 'vs/base/common/htmlContent'; -import { ITerminalWidget, IHoverTarget, IHoverAnchor, HorizontalAnchorSide, VerticalAnchorSide } from 'vs/workbench/contrib/terminal/browser/widgets/widgets'; +import { ITerminalWidget } from 'vs/workbench/contrib/terminal/browser/widgets/widgets'; +import { IHoverTarget, IHoverAnchor } from 'vs/workbench/contrib/hover/browser/hover'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { HoverWidget } from 'vs/workbench/contrib/terminal/browser/widgets/hoverWidget'; +import { HoverWidget } from 'vs/workbench/contrib/hover/browser/hoverWidget'; import { RunOnceScheduler } from 'vs/base/common/async'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import * as dom from 'vs/base/browser/dom'; @@ -84,7 +85,8 @@ export class EnvironmentVariableInfoWidget extends Widget implements ITerminalWi } const target = new ElementHoverTarget(this._domNode); const actions = this._info.getActions ? this._info.getActions() : undefined; - this._hoverWidget = this._instantiationService.createInstance(HoverWidget, this._container, target, new MarkdownString(this._info.getInfo()), () => { }, actions); + this._hoverWidget = this._instantiationService.createInstance(HoverWidget, target, new MarkdownString(this._info.getInfo()), () => { }, actions); + this._hoverWidget.render(this._container); this._register(this._hoverWidget); this._register(this._hoverWidget.onDispose(() => this._hoverWidget = undefined)); } @@ -103,13 +105,15 @@ class ElementHoverTarget implements IHoverTarget { const position = dom.getDomNodePagePosition(this._element); return { x: position.left, - horizontalAnchorSide: HorizontalAnchorSide.Left, y: document.documentElement.clientHeight - position.top - 1, - verticalAnchorSide: VerticalAnchorSide.Bottom, fallbackY: position.top + position.height }; } + render(container: HTMLElement): void { + // TODO: Should render be optional? + } + dispose(): void { } } diff --git a/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts b/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts index 438ae87c290ba..b000f47f3281b 100644 --- a/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts +++ b/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts @@ -6,11 +6,12 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { IMarkdownString } from 'vs/base/common/htmlContent'; import { Widget } from 'vs/base/browser/ui/widget'; -import { ITerminalWidget, IHoverAnchor, IHoverTarget, HorizontalAnchorSide, VerticalAnchorSide } from 'vs/workbench/contrib/terminal/browser/widgets/widgets'; -import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { HoverWidget } from 'vs/workbench/contrib/terminal/browser/widgets/hoverWidget'; +import { ITerminalWidget } from 'vs/workbench/contrib/terminal/browser/widgets/widgets'; import * as dom from 'vs/base/browser/dom'; import { IViewportRange } from 'xterm'; +import { IHoverTarget, IHoverAnchor, IHoverService } from 'vs/workbench/contrib/hover/browser/hover'; +import { registerThemingParticipant } from 'vs/platform/theme/common/themeService'; +import { editorHoverHighlight } from 'vs/platform/theme/common/colorRegistry'; const $ = dom.$; @@ -29,7 +30,7 @@ export class TerminalHover extends Disposable implements ITerminalWidget { private readonly _targetOptions: ILinkHoverTargetOptions, private readonly _text: IMarkdownString, private readonly _linkHandler: (url: string) => void, - @IInstantiationService private readonly _instantiationService: IInstantiationService + @IHoverService private readonly _hoverService: IHoverService ) { super(); } @@ -40,81 +41,84 @@ export class TerminalHover extends Disposable implements ITerminalWidget { attach(container: HTMLElement): void { const target = new CellHoverTarget(container, this._targetOptions); - this._register(this._instantiationService.createInstance(HoverWidget, container, target, this._text, this._linkHandler, [])); + + this._hoverService.showHover({ + target, + text: this._text, + linkHandler: this._linkHandler + }); + + // const hover = this._instantiationService.createInstance(HoverWidget, target, this._text, this._linkHandler, []); + // hover.layout(container); + // this._register(hover); } } -class CellHoverTarget extends Widget implements IHoverTarget { - private _domNode: HTMLElement; - private _isDisposed: boolean = false; +export class CellHoverTarget extends Widget implements IHoverTarget { + private _domNode: HTMLElement | undefined; + private readonly _targetElements: HTMLElement[] = []; - readonly targetElements: readonly HTMLElement[]; + get targetElements(): readonly HTMLElement[] { return this._targetElements; } constructor( - private readonly _container: HTMLElement, - o: ILinkHoverTargetOptions + container: HTMLElement, + private readonly _options: ILinkHoverTargetOptions ) { super(); - this._domNode = $('div.terminal-hover-targets'); - const targets: HTMLElement[] = []; - const rowCount = o.viewportRange.end.y - o.viewportRange.start.y + 1; + this._domNode = $('div.terminal-hover-targets.xterm-hover'); + const rowCount = this._options.viewportRange.end.y - this._options.viewportRange.start.y + 1; // Add top target row - const width = (o.viewportRange.end.y > o.viewportRange.start.y ? o.terminalDimensions.width - o.viewportRange.start.x : o.viewportRange.end.x - o.viewportRange.start.x + 1) * o.cellDimensions.width; + const width = (this._options.viewportRange.end.y > this._options.viewportRange.start.y ? this._options.terminalDimensions.width - this._options.viewportRange.start.x : this._options.viewportRange.end.x - this._options.viewportRange.start.x + 1) * this._options.cellDimensions.width; const topTarget = $('div.terminal-hover-target.hoverHighlight'); - topTarget.style.left = `${o.viewportRange.start.x * o.cellDimensions.width}px`; - topTarget.style.bottom = `${(o.terminalDimensions.height - o.viewportRange.start.y - 1) * o.cellDimensions.height}px`; + topTarget.style.left = `${this._options.viewportRange.start.x * this._options.cellDimensions.width}px`; + topTarget.style.bottom = `${(this._options.terminalDimensions.height - this._options.viewportRange.start.y - 1) * this._options.cellDimensions.height}px`; topTarget.style.width = `${width}px`; - topTarget.style.height = `${o.cellDimensions.height}px`; - targets.push(this._domNode.appendChild(topTarget)); + topTarget.style.height = `${this._options.cellDimensions.height}px`; + this._targetElements.push(this._domNode.appendChild(topTarget)); // Add middle target rows if (rowCount > 2) { const middleTarget = $('div.terminal-hover-target.hoverHighlight'); middleTarget.style.left = `0px`; - middleTarget.style.bottom = `${(o.terminalDimensions.height - o.viewportRange.start.y - 1 - (rowCount - 2)) * o.cellDimensions.height}px`; - middleTarget.style.width = `${o.terminalDimensions.width * o.cellDimensions.width}px`; - middleTarget.style.height = `${(rowCount - 2) * o.cellDimensions.height}px`; - targets.push(this._domNode.appendChild(middleTarget)); + middleTarget.style.bottom = `${(this._options.terminalDimensions.height - this._options.viewportRange.start.y - 1 - (rowCount - 2)) * this._options.cellDimensions.height}px`; + middleTarget.style.width = `${this._options.terminalDimensions.width * this._options.cellDimensions.width}px`; + middleTarget.style.height = `${(rowCount - 2) * this._options.cellDimensions.height}px`; + this._targetElements.push(this._domNode.appendChild(middleTarget)); } // Add bottom target row if (rowCount > 1) { const bottomTarget = $('div.terminal-hover-target.hoverHighlight'); bottomTarget.style.left = `0px`; - bottomTarget.style.bottom = `${(o.terminalDimensions.height - o.viewportRange.end.y - 1) * o.cellDimensions.height}px`; - bottomTarget.style.width = `${(o.viewportRange.end.x + 1) * o.cellDimensions.width}px`; - bottomTarget.style.height = `${o.cellDimensions.height}px`; - targets.push(this._domNode.appendChild(bottomTarget)); + bottomTarget.style.bottom = `${(this._options.terminalDimensions.height - this._options.viewportRange.end.y - 1) * this._options.cellDimensions.height}px`; + bottomTarget.style.width = `${(this._options.viewportRange.end.x + 1) * this._options.cellDimensions.width}px`; + bottomTarget.style.height = `${this._options.cellDimensions.height}px`; + this._targetElements.push(this._domNode.appendChild(bottomTarget)); } - this.targetElements = targets; - - if (o.modifierDownCallback && o.modifierUpCallback) { + if (this._options.modifierDownCallback && this._options.modifierUpCallback) { let down = false; this._register(dom.addDisposableListener(document, 'keydown', e => { if (e.ctrlKey && !down) { down = true; - o.modifierDownCallback!(); + this._options.modifierDownCallback!(); } })); this._register(dom.addDisposableListener(document, 'keyup', e => { if (!e.ctrlKey) { down = false; - o.modifierUpCallback!(); + this._options.modifierUpCallback!(); } })); } - this._container.appendChild(this._domNode); + container.appendChild(this._domNode); } dispose(): void { - if (!this._isDisposed) { - this._container.removeChild(this._domNode); - } - this._isDisposed = true; + this._domNode?.parentElement?.removeChild(this._domNode); super.dispose(); } @@ -122,10 +126,18 @@ class CellHoverTarget extends Widget implements IHoverTarget { const firstPosition = dom.getDomNodePagePosition(this.targetElements[0]); return { x: firstPosition.left, - horizontalAnchorSide: HorizontalAnchorSide.Left, y: document.documentElement.clientHeight - firstPosition.top - 1, - verticalAnchorSide: VerticalAnchorSide.Bottom, fallbackY: firstPosition.top + firstPosition.height - 1 }; } } + +registerThemingParticipant((theme, collector) => { + let editorHoverHighlightColor = theme.getColor(editorHoverHighlight); + if (editorHoverHighlightColor) { + if (editorHoverHighlightColor.isOpaque()) { + editorHoverHighlightColor = editorHoverHighlightColor.transparent(0.5); + } + collector.addRule(`.integrated-terminal .hoverHighlight { background-color: ${editorHoverHighlightColor}; }`); + } +}); diff --git a/src/vs/workbench/contrib/terminal/browser/widgets/widgets.ts b/src/vs/workbench/contrib/terminal/browser/widgets/widgets.ts index d9071035e9713..50bc2ee7e5e28 100644 --- a/src/vs/workbench/contrib/terminal/browser/widgets/widgets.ts +++ b/src/vs/workbench/contrib/terminal/browser/widgets/widgets.ts @@ -12,32 +12,3 @@ export interface ITerminalWidget extends IDisposable { id: string; attach(container: HTMLElement): void; } - -export enum HorizontalAnchorSide { - Left, - Right -} - -export enum VerticalAnchorSide { - Top, - Bottom -} - -export interface IHoverAnchor { - x: number; - y: number; - horizontalAnchorSide: HorizontalAnchorSide; - verticalAnchorSide: VerticalAnchorSide; - /** - * Fallback Y value to try with opposite VerticalAlignment if the hover does not fit vertically. - */ - fallbackY: number; -} - -/** - * A target for a hover which can know about domain-specific locations. - */ -export interface IHoverTarget extends IDisposable { - readonly targetElements: readonly HTMLElement[]; - readonly anchor: IHoverAnchor; -} diff --git a/src/vs/workbench/workbench.common.main.ts b/src/vs/workbench/workbench.common.main.ts index dfa6a353a3460..4663e8b4160f8 100644 --- a/src/vs/workbench/workbench.common.main.ts +++ b/src/vs/workbench/workbench.common.main.ts @@ -281,4 +281,7 @@ import 'vs/workbench/contrib/welcome/common/viewsWelcome.contribution'; // Timeline import 'vs/workbench/contrib/timeline/browser/timeline.contribution'; +// Hover +import 'vs/workbench/contrib/hover/browser/hover.contribution'; + //#endregion From cfdea057f43ef2bfe983c4622564bbe858ec1da2 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 Jun 2020 04:50:13 -0700 Subject: [PATCH 02/11] Allow a element for the target --- .../workbench/contrib/hover/browser/hover.ts | 10 ------- .../contrib/hover/browser/hoverService.ts | 3 -- .../contrib/hover/browser/hoverWidget.ts | 25 ++++++++++++++-- .../widgets/environmentVariableInfoWidget.ts | 30 +------------------ .../browser/widgets/terminalHoverWidget.ts | 18 ++--------- 5 files changed, 26 insertions(+), 60 deletions(-) diff --git a/src/vs/workbench/contrib/hover/browser/hover.ts b/src/vs/workbench/contrib/hover/browser/hover.ts index 4561bf43390e8..50366a06ded78 100644 --- a/src/vs/workbench/contrib/hover/browser/hover.ts +++ b/src/vs/workbench/contrib/hover/browser/hover.ts @@ -50,14 +50,4 @@ export interface IHoverAction { */ export interface IHoverTarget extends IDisposable { readonly targetElements: readonly HTMLElement[]; - readonly anchor: IHoverAnchor; -} - -export interface IHoverAnchor { - x: number; - y: number; - /** - * Fallback Y value to try with opposite VerticalAlignment if the hover does not fit vertically. - */ - fallbackY: number; } diff --git a/src/vs/workbench/contrib/hover/browser/hoverService.ts b/src/vs/workbench/contrib/hover/browser/hoverService.ts index 040ca144c7f77..5a04c6f97c272 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverService.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverService.ts @@ -29,9 +29,6 @@ export class HoverService implements IHoverService { }, anchorPosition: AnchorPosition.ABOVE, getAnchor: () => { - console.log('anchor', options.target!.anchor); - if (options.target) { - } return { x: hover.x, y: hover.y diff --git a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts index a67b60c5c7896..593d752454339 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts @@ -24,6 +24,7 @@ export class HoverWidget extends Widget { private readonly _mouseTracker: CompositeMouseTracker; private readonly _hover: BaseHoverWidget; + private readonly _target: IHoverTarget; private _isDisposed: boolean = false; private _anchor: AnchorPosition = AnchorPosition.ABOVE; @@ -40,8 +41,13 @@ export class HoverWidget extends Widget { get x(): number { return this._x; } get y(): number { return this._y; } + /** + * @param target The target for the hover, this determines the position of the hover. A + * HTMLElement can be used for simple cases and a IHoverTarget for more complex cases where + * multiple elements and/or a dispose method is required. + */ constructor( - private _target: IHoverTarget, + target: IHoverTarget | HTMLElement, private _text: IMarkdownString, private _linkHandler: (url: string) => void, private _actions: { label: string, iconClass?: string, run: (target: HTMLElement) => void, commandId: string }[] | undefined, @@ -50,6 +56,8 @@ export class HoverWidget extends Widget { ) { super(); + this._target = 'targetElements' in target ? target : new ElementHoverTarget(target); + this._hover = this._register(new BaseHoverWidget()); // TODO: Move xterm-hover into terminal this._hover.containerDomNode.classList.add('workbench-hover', 'fadeIn', 'xterm-hover'); @@ -97,7 +105,7 @@ export class HoverWidget extends Widget { this._hover.containerDomNode.appendChild(statusBarElement); } - this._mouseTracker = new CompositeMouseTracker([this._hover.containerDomNode, ..._target.targetElements]); + this._mouseTracker = new CompositeMouseTracker([this._hover.containerDomNode, ...this._target.targetElements]); this._register(this._mouseTracker.onMouseOut(() => this.dispose())); this._register(this._mouseTracker); } @@ -199,3 +207,16 @@ class CompositeMouseTracker extends Widget { } } } + +class ElementHoverTarget implements IHoverTarget { + readonly targetElements: readonly HTMLElement[]; + + constructor( + private _element: HTMLElement + ) { + this.targetElements = [this._element]; + } + + dispose(): void { + } +} diff --git a/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts b/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts index b005268b6c66f..588b280dd5fcd 100644 --- a/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts +++ b/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts @@ -7,7 +7,6 @@ import { Widget } from 'vs/base/browser/ui/widget'; import { IEnvironmentVariableInfo } from 'vs/workbench/contrib/terminal/common/environmentVariable'; import { MarkdownString } from 'vs/base/common/htmlContent'; import { ITerminalWidget } from 'vs/workbench/contrib/terminal/browser/widgets/widgets'; -import { IHoverTarget, IHoverAnchor } from 'vs/workbench/contrib/hover/browser/hover'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { HoverWidget } from 'vs/workbench/contrib/hover/browser/hoverWidget'; import { RunOnceScheduler } from 'vs/base/common/async'; @@ -83,37 +82,10 @@ export class EnvironmentVariableInfoWidget extends Widget implements ITerminalWi if (!this._domNode || !this._container || this._hoverWidget) { return; } - const target = new ElementHoverTarget(this._domNode); const actions = this._info.getActions ? this._info.getActions() : undefined; - this._hoverWidget = this._instantiationService.createInstance(HoverWidget, target, new MarkdownString(this._info.getInfo()), () => { }, actions); + this._hoverWidget = this._instantiationService.createInstance(HoverWidget, this._domNode, new MarkdownString(this._info.getInfo()), () => { }, actions); this._hoverWidget.render(this._container); this._register(this._hoverWidget); this._register(this._hoverWidget.onDispose(() => this._hoverWidget = undefined)); } } - -class ElementHoverTarget implements IHoverTarget { - readonly targetElements: readonly HTMLElement[]; - - constructor( - private _element: HTMLElement - ) { - this.targetElements = [this._element]; - } - - get anchor(): IHoverAnchor { - const position = dom.getDomNodePagePosition(this._element); - return { - x: position.left, - y: document.documentElement.clientHeight - position.top - 1, - fallbackY: position.top + position.height - }; - } - - render(container: HTMLElement): void { - // TODO: Should render be optional? - } - - dispose(): void { - } -} diff --git a/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts b/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts index b000f47f3281b..ac66f5f8e20ee 100644 --- a/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts +++ b/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts @@ -9,7 +9,7 @@ import { Widget } from 'vs/base/browser/ui/widget'; import { ITerminalWidget } from 'vs/workbench/contrib/terminal/browser/widgets/widgets'; import * as dom from 'vs/base/browser/dom'; import { IViewportRange } from 'xterm'; -import { IHoverTarget, IHoverAnchor, IHoverService } from 'vs/workbench/contrib/hover/browser/hover'; +import { IHoverTarget, IHoverService } from 'vs/workbench/contrib/hover/browser/hover'; import { registerThemingParticipant } from 'vs/platform/theme/common/themeService'; import { editorHoverHighlight } from 'vs/platform/theme/common/colorRegistry'; @@ -41,20 +41,15 @@ export class TerminalHover extends Disposable implements ITerminalWidget { attach(container: HTMLElement): void { const target = new CellHoverTarget(container, this._targetOptions); - this._hoverService.showHover({ target, text: this._text, linkHandler: this._linkHandler }); - - // const hover = this._instantiationService.createInstance(HoverWidget, target, this._text, this._linkHandler, []); - // hover.layout(container); - // this._register(hover); } } -export class CellHoverTarget extends Widget implements IHoverTarget { +class CellHoverTarget extends Widget implements IHoverTarget { private _domNode: HTMLElement | undefined; private readonly _targetElements: HTMLElement[] = []; @@ -121,15 +116,6 @@ export class CellHoverTarget extends Widget implements IHoverTarget { this._domNode?.parentElement?.removeChild(this._domNode); super.dispose(); } - - get anchor(): IHoverAnchor { - const firstPosition = dom.getDomNodePagePosition(this.targetElements[0]); - return { - x: firstPosition.left, - y: document.documentElement.clientHeight - firstPosition.top - 1, - fallbackY: firstPosition.top + firstPosition.height - 1 - }; - } } registerThemingParticipant((theme, collector) => { From 5941f84bf3b1b03d1c44fee3a4e989ec85e9a846 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 Jun 2020 05:38:44 -0700 Subject: [PATCH 03/11] Prevent hover from re-displaying if it's currently showing the same one --- .../workbench/contrib/hover/browser/hover.ts | 11 ++++---- .../contrib/hover/browser/hoverService.ts | 28 ++++++++++++------- .../contrib/hover/browser/hoverWidget.ts | 23 +++++++++------ .../contrib/hover/browser/media/hover.css | 1 + .../widgets/environmentVariableInfoWidget.ts | 27 ++++++++++-------- 5 files changed, 55 insertions(+), 35 deletions(-) diff --git a/src/vs/workbench/contrib/hover/browser/hover.ts b/src/vs/workbench/contrib/hover/browser/hover.ts index 50366a06ded78..edc44ce52c32a 100644 --- a/src/vs/workbench/contrib/hover/browser/hover.ts +++ b/src/vs/workbench/contrib/hover/browser/hover.ts @@ -23,14 +23,15 @@ export interface IHoverOptions { text: IMarkdownString; // TODO: Link handler not necessary? - linkHandler: (url: string) => void; + linkHandler?: (url: string) => void; /** - * A hover target holding 1+ elements that will hide the hover when the mouse leaves any of the - * elements or the hover. When this is not provided, only mouseout on the hover will hide the - * hover. + * The target for the hover. This determines the position of the hover and it will only be + * hidden when the mouse leaves both the hover and the target. A HTMLElement can be used for + * simple cases and a IHoverTarget for more complex cases where multiple elements and/or a + * dispose method is required. */ - target: IHoverTarget; + target: IHoverTarget | HTMLElement; /** * A set of actions for the hover's "status bar". diff --git a/src/vs/workbench/contrib/hover/browser/hoverService.ts b/src/vs/workbench/contrib/hover/browser/hoverService.ts index 5a04c6f97c272..ca69faeae4581 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverService.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverService.ts @@ -7,11 +7,13 @@ import { IHoverService, IHoverOptions } from 'vs/workbench/contrib/hover/browser import { IContextViewService } from 'vs/platform/contextview/browser/contextView'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { HoverWidget } from 'vs/workbench/contrib/hover/browser/hoverWidget'; -import { IContextViewProvider, AnchorPosition } from 'vs/base/browser/ui/contextview/contextview'; +import { IContextViewProvider, AnchorPosition, IDelegate } from 'vs/base/browser/ui/contextview/contextview'; export class HoverService implements IHoverService { declare readonly _serviceBrand: undefined; + private _currentHoverOptions: IHoverOptions | undefined; + constructor( @IInstantiationService private readonly _instantiationService: IInstantiationService, @IContextViewService private readonly _contextViewService: IContextViewService @@ -19,25 +21,31 @@ export class HoverService implements IHoverService { } showHover(options: IHoverOptions): void { - // TODO: Prevent hover when the same one is already visible - const hover = this._instantiationService.createInstance(HoverWidget, options.target, options.text, options.linkHandler, []); + if (this._currentHoverOptions === options) { + return; + } + this._currentHoverOptions = options; + + const hover = this._instantiationService.createInstance(HoverWidget, options.target, options.text, options.linkHandler || (() => { }), []); const provider = this._contextViewService as IContextViewProvider; - provider.showContextView({ + const contextViewDelegate: IDelegate = { render: container => { hover.render(container); + hover.onDispose(() => this._currentHoverOptions = undefined); return hover; }, anchorPosition: AnchorPosition.ABOVE, getAnchor: () => { - return { - x: hover.x, - y: hover.y - }; - } - }); + return { x: hover.x, y: hover.y }; + }, + layout: () => hover.layout() + }; + provider.showContextView(contextViewDelegate); + hover.onRequestLayout(() => provider.layout()); } hideHover(): void { + this._currentHoverOptions = undefined; this._contextViewService.hideContextView(); } } diff --git a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts index 593d752454339..4de782e534616 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts @@ -34,18 +34,15 @@ export class HoverWidget extends Widget { get isDisposed(): boolean { return this._isDisposed; } get domNode(): HTMLElement { return this._hover.containerDomNode; } - private readonly _onDispose = new Emitter(); + private readonly _onDispose = this._register(new Emitter()); get onDispose(): Event { return this._onDispose.event; } + private readonly _onRequestLayout = this._register(new Emitter()); + get onRequestLayout(): Event { return this._onRequestLayout.event; } get anchor(): AnchorPosition { return this._anchor; } get x(): number { return this._x; } get y(): number { return this._y; } - /** - * @param target The target for the hover, this determines the position of the hover. A - * HTMLElement can be used for simple cases and a IHoverTarget for more complex cases where - * multiple elements and/or a dispose method is required. - */ constructor( target: IHoverTarget | HTMLElement, private _text: IMarkdownString, @@ -86,7 +83,9 @@ export class HoverWidget extends Widget { }, codeBlockRenderCallback: () => { contentsElement.classList.add('code-hover-contents'); - this.render(); + // This changes the dimensions of the hover to trigger a render + this._onRequestLayout.fire(); + // this.render(); } }); contentsElement.appendChild(markdownElement); @@ -115,6 +114,12 @@ export class HoverWidget extends Widget { container?.appendChild(this._hover.containerDomNode); } + console.log(this._hover.containerDomNode.clientWidth, this._hover.containerDomNode.clientHeight); + + this.layout(); + } + + public layout() { this._hover.containerDomNode.classList.remove('right-aligned'); this._hover.contentsDomNode.style.maxHeight = ''; @@ -122,7 +127,7 @@ export class HoverWidget extends Widget { const targetBounds = this._target.targetElements.map(e => e.getBoundingClientRect()); const targetLeft = Math.min(...targetBounds.map(e => e.left)); if (targetLeft + this._hover.containerDomNode.clientWidth >= document.documentElement.clientWidth) { - // TODO: Communicate horizontal alignment + // TODO: Communicate horizontal alignment to contextviewservice? this._x = document.documentElement.clientWidth; this._hover.containerDomNode.classList.add('right-aligned'); } else { @@ -138,6 +143,7 @@ export class HoverWidget extends Widget { } else { this._y = targetTop; } + console.log('hover y = ', this._y); this._hover.onContentsChanged(); } @@ -152,6 +158,7 @@ export class HoverWidget extends Widget { public dispose(): void { if (!this._isDisposed) { + console.log('dispose'); this._onDispose.fire(); this._hover.containerDomNode.parentElement?.removeChild(this.domNode); this._messageListeners.dispose(); diff --git a/src/vs/workbench/contrib/hover/browser/media/hover.css b/src/vs/workbench/contrib/hover/browser/media/hover.css index 7b193b9e658b4..47d8ab484c639 100644 --- a/src/vs/workbench/contrib/hover/browser/media/hover.css +++ b/src/vs/workbench/contrib/hover/browser/media/hover.css @@ -11,6 +11,7 @@ /* Must be higher than sash's z-index and terminal canvases */ z-index: 40; overflow: hidden; + max-width: 700px; } .monaco-workbench .workbench-hover a { diff --git a/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts b/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts index 588b280dd5fcd..6bc27808de7cb 100644 --- a/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts +++ b/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts @@ -7,27 +7,26 @@ import { Widget } from 'vs/base/browser/ui/widget'; import { IEnvironmentVariableInfo } from 'vs/workbench/contrib/terminal/common/environmentVariable'; import { MarkdownString } from 'vs/base/common/htmlContent'; import { ITerminalWidget } from 'vs/workbench/contrib/terminal/browser/widgets/widgets'; -import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { HoverWidget } from 'vs/workbench/contrib/hover/browser/hoverWidget'; import { RunOnceScheduler } from 'vs/base/common/async'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import * as dom from 'vs/base/browser/dom'; import { IDisposable } from 'vs/base/common/lifecycle'; +import { IHoverService, IHoverOptions } from 'vs/workbench/contrib/hover/browser/hover'; export class EnvironmentVariableInfoWidget extends Widget implements ITerminalWidget { readonly id = 'env-var-info'; private _domNode: HTMLElement | undefined; private _container: HTMLElement | undefined; - private _hoverWidget: HoverWidget | undefined; private _mouseMoveListener: IDisposable | undefined; + private _hoverOptions: IHoverOptions | undefined; get requiresAction() { return this._info.requiresAction; } constructor( private _info: IEnvironmentVariableInfo, - @IInstantiationService private readonly _instantiationService: IInstantiationService, - @IConfigurationService private readonly _configurationService: IConfigurationService + @IConfigurationService private readonly _configurationService: IConfigurationService, + @IHoverService private readonly _hoverService: IHoverService ) { super(); } @@ -75,17 +74,21 @@ export class EnvironmentVariableInfoWidget extends Widget implements ITerminalWi focus() { this._showHover(); - this._hoverWidget?.focus(); + // TODO: Focus hover here for a11y } private _showHover() { - if (!this._domNode || !this._container || this._hoverWidget) { + if (!this._domNode || !this._container) { return; } - const actions = this._info.getActions ? this._info.getActions() : undefined; - this._hoverWidget = this._instantiationService.createInstance(HoverWidget, this._domNode, new MarkdownString(this._info.getInfo()), () => { }, actions); - this._hoverWidget.render(this._container); - this._register(this._hoverWidget); - this._register(this._hoverWidget.onDispose(() => this._hoverWidget = undefined)); + if (!this._hoverOptions) { + const actions = this._info.getActions ? this._info.getActions() : undefined; + this._hoverOptions = { + target: this._domNode, + text: new MarkdownString(this._info.getInfo()), + actions + }; + } + this._hoverService.showHover(this._hoverOptions); } } From 7961880011c064bcd23cf96519a000bfdb799123 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 Jun 2020 05:44:28 -0700 Subject: [PATCH 04/11] Hide hover on action --- src/vs/workbench/contrib/hover/browser/hoverService.ts | 2 +- src/vs/workbench/contrib/hover/browser/hoverWidget.ts | 10 +++++++++- .../browser/widgets/environmentVariableInfoWidget.ts | 1 - 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/hover/browser/hoverService.ts b/src/vs/workbench/contrib/hover/browser/hoverService.ts index ca69faeae4581..7ef478690d251 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverService.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverService.ts @@ -26,7 +26,7 @@ export class HoverService implements IHoverService { } this._currentHoverOptions = options; - const hover = this._instantiationService.createInstance(HoverWidget, options.target, options.text, options.linkHandler || (() => { }), []); + const hover = this._instantiationService.createInstance(HoverWidget, options.target, options.text, options.linkHandler || (() => { }), options.actions); const provider = this._contextViewService as IContextViewProvider; const contextViewDelegate: IDelegate = { render: container => { diff --git a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts index 4de782e534616..1fbbcf8ed39b8 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts @@ -98,7 +98,15 @@ export class HoverWidget extends Widget { this._actions.forEach(action => { const keybinding = this._keybindingService.lookupKeybinding(action.commandId); const keybindingLabel = keybinding ? keybinding.getLabel() : null; - renderHoverAction(actionsElement, action, keybindingLabel); + renderHoverAction(actionsElement, { + label: action.label, + commandId: action.commandId, + run: e => { + action.run(e); + this.dispose(); + }, + iconClass: action.iconClass + }, keybindingLabel); }); statusBarElement.appendChild(actionsElement); this._hover.containerDomNode.appendChild(statusBarElement); diff --git a/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts b/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts index 6bc27808de7cb..23e9ce47be202 100644 --- a/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts +++ b/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts @@ -40,7 +40,6 @@ export class EnvironmentVariableInfoWidget extends Widget implements ITerminalWi } container.appendChild(this._domNode); - const timeout = this._configurationService.getValue('editor.hover.delay'); const scheduler: RunOnceScheduler = new RunOnceScheduler(() => this._showHover(), timeout); this._register(scheduler); From aea5cdce7d088eb046cb5bb3616bfee245ef0991 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 Jun 2020 05:46:02 -0700 Subject: [PATCH 05/11] Remove test calls --- src/vs/workbench/contrib/hover/browser/hoverWidget.ts | 7 +------ .../contrib/terminal/browser/links/terminalLinkManager.ts | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts index 1fbbcf8ed39b8..ad349643204ef 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts @@ -83,9 +83,8 @@ export class HoverWidget extends Widget { }, codeBlockRenderCallback: () => { contentsElement.classList.add('code-hover-contents'); - // This changes the dimensions of the hover to trigger a render + // This changes the dimensions of the hover so trigger a layout this._onRequestLayout.fire(); - // this.render(); } }); contentsElement.appendChild(markdownElement); @@ -122,8 +121,6 @@ export class HoverWidget extends Widget { container?.appendChild(this._hover.containerDomNode); } - console.log(this._hover.containerDomNode.clientWidth, this._hover.containerDomNode.clientHeight); - this.layout(); } @@ -151,7 +148,6 @@ export class HoverWidget extends Widget { } else { this._y = targetTop; } - console.log('hover y = ', this._y); this._hover.onContentsChanged(); } @@ -166,7 +162,6 @@ export class HoverWidget extends Widget { public dispose(): void { if (!this._isDisposed) { - console.log('dispose'); this._onDispose.fire(); this._hover.containerDomNode.parentElement?.removeChild(this.domNode); this._messageListeners.dispose(); diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts index 08c093358ee98..208b05dbed92b 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts @@ -97,7 +97,7 @@ export class TerminalLinkManager extends DisposableStore { terminalDimensions, modifierDownCallback, modifierUpCallback - }, this._getLinkHoverString(link.text + 'kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf kljnasdfljkasdgljkansdf lkjan sdflkjfdasnlkfadsjnlkjfdsabjlkhadfs jklbadsfljkbdfas ljkbfadslbkjhfdslbkhabldsf kjlbdfasbljkfd abjklsdfbkjl adsfblkadfj blakdsf bjdasflkj dbfslkjafd sjbdfslkjsfbkjfds', link.label), (text) => link.activate(undefined, text), link); + }, this._getLinkHoverString(link.text, link.label), (text) => link.activate(undefined, text), link); } private _showHover( From bbcd5175c03acf9bdf029c272e69849c560c5976 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 Jun 2020 06:01:50 -0700 Subject: [PATCH 06/11] Doc API, make linkHandler optional --- .../workbench/contrib/hover/browser/hover.ts | 35 +++++++++++++++++-- .../contrib/hover/browser/hoverService.ts | 5 ++- .../contrib/hover/browser/hoverWidget.ts | 9 +++-- .../browser/widgets/terminalHoverWidget.ts | 2 +- 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/hover/browser/hover.ts b/src/vs/workbench/contrib/hover/browser/hover.ts index edc44ce52c32a..ee2426b76f8b4 100644 --- a/src/vs/workbench/contrib/hover/browser/hover.ts +++ b/src/vs/workbench/contrib/hover/browser/hover.ts @@ -9,10 +9,21 @@ import { IMarkdownString } from 'vs/base/common/htmlContent'; export const IHoverService = createDecorator('hoverService'); +/** + * A service that enables the convenient display of rich markdown-based hovers in the workbench. + */ export interface IHoverService { readonly _serviceBrand: undefined; + /** + * Shows a hover. + * @param options A set of options defining the characteristics of the hover. + */ showHover(options: IHoverOptions): void; + + /** + * Hides the hover if it was visible. + */ hideHover(): void; } @@ -22,7 +33,10 @@ export interface IHoverOptions { */ text: IMarkdownString; - // TODO: Link handler not necessary? + /** + * An custom link handler for markdown links, if this is not provided the IOpenerService will + * be used to open the links using its default options. + */ linkHandler?: (url: string) => void; /** @@ -40,10 +54,27 @@ export interface IHoverOptions { } export interface IHoverAction { + /** + * The label to use in the hover's status bar. + */ label: string; + + /** + * An optional class of an icon that will be displayed before the label. + */ iconClass?: string; - run: (target: HTMLElement) => void; + + /** + * The command ID of the action, this is used to resolve the keybinding to display after the + * action label. + */ commandId: string; + + /** + * The callback to run the action. + * @param target The action element that was activated. + */ + run(target: HTMLElement): void; } /** diff --git a/src/vs/workbench/contrib/hover/browser/hoverService.ts b/src/vs/workbench/contrib/hover/browser/hoverService.ts index 7ef478690d251..af65b337fe935 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverService.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverService.ts @@ -26,7 +26,7 @@ export class HoverService implements IHoverService { } this._currentHoverOptions = options; - const hover = this._instantiationService.createInstance(HoverWidget, options.target, options.text, options.linkHandler || (() => { }), options.actions); + const hover = this._instantiationService.createInstance(HoverWidget, options.target, options.text, options.linkHandler, options.actions); const provider = this._contextViewService as IContextViewProvider; const contextViewDelegate: IDelegate = { render: container => { @@ -45,6 +45,9 @@ export class HoverService implements IHoverService { } hideHover(): void { + if (!this._currentHoverOptions) { + return; + } this._currentHoverOptions = undefined; this._contextViewService.hideContextView(); } diff --git a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts index ad349643204ef..e8418f75a1811 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts @@ -16,6 +16,7 @@ import { EDITOR_FONT_DEFAULTS, IEditorOptions } from 'vs/editor/common/config/ed import { HoverWidget as BaseHoverWidget, renderHoverAction } from 'vs/base/browser/ui/hover/hoverWidget'; 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'; const $ = dom.$; @@ -25,6 +26,7 @@ export class HoverWidget extends Widget { private readonly _hover: BaseHoverWidget; private readonly _target: IHoverTarget; + private readonly _linkHandler: (url: string) => any; private _isDisposed: boolean = false; private _anchor: AnchorPosition = AnchorPosition.ABOVE; @@ -46,13 +48,16 @@ export class HoverWidget extends Widget { constructor( target: IHoverTarget | HTMLElement, private _text: IMarkdownString, - private _linkHandler: (url: string) => void, + linkHandler: ((url: string) => any) | undefined, private _actions: { label: string, iconClass?: string, run: (target: HTMLElement) => void, commandId: string }[] | undefined, @IKeybindingService private readonly _keybindingService: IKeybindingService, - @IConfigurationService private readonly _configurationService: IConfigurationService + @IConfigurationService private readonly _configurationService: IConfigurationService, + @IOpenerService private readonly _openerService: IOpenerService ) { super(); + this._linkHandler = linkHandler || this._openerService.open; + this._target = 'targetElements' in target ? target : new ElementHoverTarget(target); this._hover = this._register(new BaseHoverWidget()); diff --git a/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts b/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts index ac66f5f8e20ee..3b5a3b43ac21a 100644 --- a/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts +++ b/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts @@ -29,7 +29,7 @@ export class TerminalHover extends Disposable implements ITerminalWidget { constructor( private readonly _targetOptions: ILinkHoverTargetOptions, private readonly _text: IMarkdownString, - private readonly _linkHandler: (url: string) => void, + private readonly _linkHandler: (url: string) => any, @IHoverService private readonly _hoverService: IHoverService ) { super(); From ae0398e40083f13d7d5fdb91982d482e20c44266 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 Jun 2020 06:08:03 -0700 Subject: [PATCH 07/11] Add example usage --- src/vs/workbench/contrib/hover/browser/hover.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/vs/workbench/contrib/hover/browser/hover.ts b/src/vs/workbench/contrib/hover/browser/hover.ts index ee2426b76f8b4..7e60ba701223c 100644 --- a/src/vs/workbench/contrib/hover/browser/hover.ts +++ b/src/vs/workbench/contrib/hover/browser/hover.ts @@ -18,6 +18,15 @@ export interface IHoverService { /** * Shows a hover. * @param options A set of options defining the characteristics of the hover. + * + * **Example:** A simple usage with a single element target. + * + * ```typescript + * showHover({ + * text: new MarkdownString('Hello world'), + * target: someElement + * }); + * ``` */ showHover(options: IHoverOptions): void; From 65546b33e6e6f9b1aea29c014bffa222caa12df5 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 Jun 2020 06:27:59 -0700 Subject: [PATCH 08/11] More doc, move xterm-hover into terminal --- .../workbench/contrib/hover/browser/hover.ts | 36 ++++++++++++------- .../contrib/hover/browser/hoverService.ts | 3 +- .../contrib/hover/browser/hoverWidget.ts | 8 +++-- .../browser/widgets/terminalHoverWidget.ts | 4 ++- 4 files changed, 34 insertions(+), 17 deletions(-) diff --git a/src/vs/workbench/contrib/hover/browser/hover.ts b/src/vs/workbench/contrib/hover/browser/hover.ts index 7e60ba701223c..504e8d8b5672b 100644 --- a/src/vs/workbench/contrib/hover/browser/hover.ts +++ b/src/vs/workbench/contrib/hover/browser/hover.ts @@ -10,7 +10,7 @@ import { IMarkdownString } from 'vs/base/common/htmlContent'; export const IHoverService = createDecorator('hoverService'); /** - * A service that enables the convenient display of rich markdown-based hovers in the workbench. + * Enables the convenient display of rich markdown-based hovers in the workbench. */ export interface IHoverService { readonly _serviceBrand: undefined; @@ -42,12 +42,6 @@ export interface IHoverOptions { */ text: IMarkdownString; - /** - * An custom link handler for markdown links, if this is not provided the IOpenerService will - * be used to open the links using its default options. - */ - linkHandler?: (url: string) => void; - /** * The target for the hover. This determines the position of the hover and it will only be * hidden when the mouse leaves both the hover and the target. A HTMLElement can be used for @@ -60,6 +54,17 @@ export interface IHoverOptions { * A set of actions for the hover's "status bar". */ actions?: IHoverAction[]; + + /** + * An optional array of classes to add to the hover element. + */ + additionalClasses?: string[]; + + /** + * An optional link handler for markdown links, if this is not provided the IOpenerService will + * be used to open the links using its default options. + */ + linkHandler?(url: string): void; } export interface IHoverAction { @@ -68,17 +73,17 @@ export interface IHoverAction { */ label: string; - /** - * An optional class of an icon that will be displayed before the label. - */ - iconClass?: string; - /** * The command ID of the action, this is used to resolve the keybinding to display after the * action label. */ commandId: string; + /** + * An optional class of an icon that will be displayed before the label. + */ + iconClass?: string; + /** * The callback to run the action. * @param target The action element that was activated. @@ -87,8 +92,13 @@ export interface IHoverAction { } /** - * A target for a hover which can know about domain-specific locations. + * A target for a hover. */ export interface IHoverTarget extends IDisposable { + /** + * A set of target elements used to position the hover. If multiple elements are used the hover + * will try to not overlap any target element. An example use case for this is show a hover for + * wrapped text. + */ readonly targetElements: readonly HTMLElement[]; } diff --git a/src/vs/workbench/contrib/hover/browser/hoverService.ts b/src/vs/workbench/contrib/hover/browser/hoverService.ts index af65b337fe935..f31e6bdf7c189 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverService.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverService.ts @@ -26,7 +26,8 @@ export class HoverService implements IHoverService { } this._currentHoverOptions = options; - const hover = this._instantiationService.createInstance(HoverWidget, options.target, options.text, options.linkHandler, options.actions); + // TODO: change HoverWidget to take options object + const hover = this._instantiationService.createInstance(HoverWidget, options.target, options.text, options.linkHandler, options.actions, options.additionalClasses); const provider = this._contextViewService as IContextViewProvider; const contextViewDelegate: IDelegate = { render: container => { diff --git a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts index e8418f75a1811..7f43f00176a12 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts @@ -50,6 +50,7 @@ export class HoverWidget extends Widget { private _text: IMarkdownString, linkHandler: ((url: string) => any) | undefined, private _actions: { label: string, iconClass?: string, run: (target: HTMLElement) => void, commandId: string }[] | undefined, + additionalClasses: string[] | undefined, @IKeybindingService private readonly _keybindingService: IKeybindingService, @IConfigurationService private readonly _configurationService: IConfigurationService, @IOpenerService private readonly _openerService: IOpenerService @@ -61,8 +62,11 @@ export class HoverWidget extends Widget { this._target = 'targetElements' in target ? target : new ElementHoverTarget(target); this._hover = this._register(new BaseHoverWidget()); - // TODO: Move xterm-hover into terminal - this._hover.containerDomNode.classList.add('workbench-hover', 'fadeIn', 'xterm-hover'); + + this._hover.containerDomNode.classList.add('workbench-hover', 'fadeIn'); + if (additionalClasses) { + this._hover.containerDomNode.classList.add(...additionalClasses); + } // Don't allow mousedown out of the widget, otherwise preventDefault will call and text will // not be selected. diff --git a/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts b/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts index 3b5a3b43ac21a..460f320761e0e 100644 --- a/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts +++ b/src/vs/workbench/contrib/terminal/browser/widgets/terminalHoverWidget.ts @@ -44,7 +44,9 @@ export class TerminalHover extends Disposable implements ITerminalWidget { this._hoverService.showHover({ target, text: this._text, - linkHandler: this._linkHandler + linkHandler: this._linkHandler, + // .xterm-hover lets xterm know that the hover is part of a link + additionalClasses: ['xterm-hover'] }); } } From 5d60f564c83ef4efb288f19ed71d1456c60b2b37 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 Jun 2020 06:29:57 -0700 Subject: [PATCH 09/11] Use options object in HoverWidget --- .../contrib/hover/browser/hoverService.ts | 3 +-- .../contrib/hover/browser/hoverWidget.ts | 23 ++++++++----------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/vs/workbench/contrib/hover/browser/hoverService.ts b/src/vs/workbench/contrib/hover/browser/hoverService.ts index f31e6bdf7c189..6feb5ab3ed1e6 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverService.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverService.ts @@ -26,8 +26,7 @@ export class HoverService implements IHoverService { } this._currentHoverOptions = options; - // TODO: change HoverWidget to take options object - const hover = this._instantiationService.createInstance(HoverWidget, options.target, options.text, options.linkHandler, options.actions, options.additionalClasses); + const hover = this._instantiationService.createInstance(HoverWidget, options); const provider = this._contextViewService as IContextViewProvider; const contextViewDelegate: IDelegate = { render: container => { diff --git a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts index 7f43f00176a12..67c64e207c6f7 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts @@ -4,12 +4,11 @@ *--------------------------------------------------------------------------------------------*/ import { DisposableStore } from 'vs/base/common/lifecycle'; -import { IMarkdownString } from 'vs/base/common/htmlContent'; import { renderMarkdown } from 'vs/base/browser/markdownRenderer'; import { Event, Emitter } from 'vs/base/common/event'; import * as dom from 'vs/base/browser/dom'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; -import { IHoverTarget } from 'vs/workbench/contrib/hover/browser/hover'; +import { IHoverTarget, IHoverOptions } from 'vs/workbench/contrib/hover/browser/hover'; import { KeyCode } from 'vs/base/common/keyCodes'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { EDITOR_FONT_DEFAULTS, IEditorOptions } from 'vs/editor/common/config/editorOptions'; @@ -46,26 +45,22 @@ export class HoverWidget extends Widget { get y(): number { return this._y; } constructor( - target: IHoverTarget | HTMLElement, - private _text: IMarkdownString, - linkHandler: ((url: string) => any) | undefined, - private _actions: { label: string, iconClass?: string, run: (target: HTMLElement) => void, commandId: string }[] | undefined, - additionalClasses: string[] | undefined, + options: IHoverOptions, @IKeybindingService private readonly _keybindingService: IKeybindingService, @IConfigurationService private readonly _configurationService: IConfigurationService, @IOpenerService private readonly _openerService: IOpenerService ) { super(); - this._linkHandler = linkHandler || this._openerService.open; + this._linkHandler = options.linkHandler || this._openerService.open; - this._target = 'targetElements' in target ? target : new ElementHoverTarget(target); + this._target = 'targetElements' in options.target ? options.target : new ElementHoverTarget(options.target); this._hover = this._register(new BaseHoverWidget()); this._hover.containerDomNode.classList.add('workbench-hover', 'fadeIn'); - if (additionalClasses) { - this._hover.containerDomNode.classList.add(...additionalClasses); + if (options.additionalClasses) { + this._hover.containerDomNode.classList.add(...options.additionalClasses); } // Don't allow mousedown out of the widget, otherwise preventDefault will call and text will @@ -81,7 +76,7 @@ export class HoverWidget extends Widget { const rowElement = $('div.hover-row.markdown-hover'); const contentsElement = $('div.hover-contents'); - const markdownElement = renderMarkdown(this._text, { + const markdownElement = renderMarkdown(options.text, { actionHandler: { callback: (content) => this._linkHandler(content), disposeables: this._messageListeners @@ -100,10 +95,10 @@ export class HoverWidget extends Widget { rowElement.appendChild(contentsElement); this._hover.contentsDomNode.appendChild(rowElement); - if (this._actions && this._actions.length > 0) { + if (options.actions && options.actions.length > 0) { const statusBarElement = $('div.hover-row.status-bar'); const actionsElement = $('div.actions'); - this._actions.forEach(action => { + options.actions.forEach(action => { const keybinding = this._keybindingService.lookupKeybinding(action.commandId); const keybindingLabel = keybinding ? keybinding.getLabel() : null; renderHoverAction(actionsElement, { From 6d157e30688354957ced2ac0bbae1d46361f9cec Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 Jun 2020 06:33:00 -0700 Subject: [PATCH 10/11] Allow focusing the hover --- src/vs/workbench/contrib/hover/browser/hover.ts | 3 ++- src/vs/workbench/contrib/hover/browser/hoverService.ts | 5 ++++- .../browser/widgets/environmentVariableInfoWidget.ts | 7 +++---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/hover/browser/hover.ts b/src/vs/workbench/contrib/hover/browser/hover.ts index 504e8d8b5672b..aeda610a76e7e 100644 --- a/src/vs/workbench/contrib/hover/browser/hover.ts +++ b/src/vs/workbench/contrib/hover/browser/hover.ts @@ -18,6 +18,7 @@ export interface IHoverService { /** * Shows a hover. * @param options A set of options defining the characteristics of the hover. + * @param focus Whether to focus the hover (useful for keyboard accessibility). * * **Example:** A simple usage with a single element target. * @@ -28,7 +29,7 @@ export interface IHoverService { * }); * ``` */ - showHover(options: IHoverOptions): void; + showHover(options: IHoverOptions, focus?: boolean): void; /** * Hides the hover if it was visible. diff --git a/src/vs/workbench/contrib/hover/browser/hoverService.ts b/src/vs/workbench/contrib/hover/browser/hoverService.ts index 6feb5ab3ed1e6..b8f6c549db46c 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverService.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverService.ts @@ -20,7 +20,7 @@ export class HoverService implements IHoverService { ) { } - showHover(options: IHoverOptions): void { + showHover(options: IHoverOptions, focus?: boolean): void { if (this._currentHoverOptions === options) { return; } @@ -32,6 +32,9 @@ export class HoverService implements IHoverService { render: container => { hover.render(container); hover.onDispose(() => this._currentHoverOptions = undefined); + if (focus) { + hover.focus(); + } return hover; }, anchorPosition: AnchorPosition.ABOVE, diff --git a/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts b/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts index 23e9ce47be202..be7027f563f40 100644 --- a/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts +++ b/src/vs/workbench/contrib/terminal/browser/widgets/environmentVariableInfoWidget.ts @@ -72,11 +72,10 @@ export class EnvironmentVariableInfoWidget extends Widget implements ITerminalWi } focus() { - this._showHover(); - // TODO: Focus hover here for a11y + this._showHover(true); } - private _showHover() { + private _showHover(focus?: boolean) { if (!this._domNode || !this._container) { return; } @@ -88,6 +87,6 @@ export class EnvironmentVariableInfoWidget extends Widget implements ITerminalWi actions }; } - this._hoverService.showHover(this._hoverOptions); + this._hoverService.showHover(this._hoverOptions, focus); } } From 393d77cfbecdf904503fabc39e05fcb70f3ef1a2 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 16 Jun 2020 06:45:14 -0700 Subject: [PATCH 11/11] Respect anchor position --- src/vs/workbench/contrib/hover/browser/hoverService.ts | 8 +++----- src/vs/workbench/contrib/hover/browser/hoverWidget.ts | 2 -- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/hover/browser/hoverService.ts b/src/vs/workbench/contrib/hover/browser/hoverService.ts index b8f6c549db46c..590e3c2859d75 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverService.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverService.ts @@ -7,7 +7,7 @@ import { IHoverService, IHoverOptions } from 'vs/workbench/contrib/hover/browser import { IContextViewService } from 'vs/platform/contextview/browser/contextView'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { HoverWidget } from 'vs/workbench/contrib/hover/browser/hoverWidget'; -import { IContextViewProvider, AnchorPosition, IDelegate } from 'vs/base/browser/ui/contextview/contextview'; +import { IContextViewProvider, IDelegate } from 'vs/base/browser/ui/contextview/contextview'; export class HoverService implements IHoverService { declare readonly _serviceBrand: undefined; @@ -37,10 +37,8 @@ export class HoverService implements IHoverService { } return hover; }, - anchorPosition: AnchorPosition.ABOVE, - getAnchor: () => { - return { x: hover.x, y: hover.y }; - }, + anchorPosition: hover.anchor, + getAnchor: () => ({ x: hover.x, y: hover.y }), layout: () => hover.layout() }; provider.showContextView(contextViewDelegate); diff --git a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts index 67c64e207c6f7..e6926919f90fe 100644 --- a/src/vs/workbench/contrib/hover/browser/hoverWidget.ts +++ b/src/vs/workbench/contrib/hover/browser/hoverWidget.ts @@ -136,7 +136,6 @@ export class HoverWidget extends Widget { const targetBounds = this._target.targetElements.map(e => e.getBoundingClientRect()); const targetLeft = Math.min(...targetBounds.map(e => e.left)); if (targetLeft + this._hover.containerDomNode.clientWidth >= document.documentElement.clientWidth) { - // TODO: Communicate horizontal alignment to contextviewservice? this._x = document.documentElement.clientWidth; this._hover.containerDomNode.classList.add('right-aligned'); } else { @@ -146,7 +145,6 @@ export class HoverWidget extends Widget { // Get vertical alignment and position const targetTop = Math.min(...targetBounds.map(e => e.top)); if (targetTop - this._hover.containerDomNode.clientHeight < 0) { - // TODO: Cap max height this._anchor = AnchorPosition.BELOW; this._y = Math.max(...targetBounds.map(e => e.bottom)) - 2; } else {