From c9242053216cae5d8b80c5513b13ef8cc5a8c4b5 Mon Sep 17 00:00:00 2001 From: Benjamin Christopher Simmonds <44439583+benibenj@users.noreply.github.com> Date: Wed, 15 May 2024 18:39:03 +0200 Subject: [PATCH] Make List / Tree Hovers Focusable (#212818) * fixes #211951 * fix test * Support updatable hover --- src/vs/base/browser/ui/hover/hover.ts | 6 ++++ .../base/browser/ui/hover/hoverDelegate2.ts | 1 + .../services/hoverService/hoverService.ts | 28 ++++++++++++---- .../hoverService/updatableHoverWidget.ts | 2 +- .../hover/test/browser/nullHoverService.ts | 1 + .../workbench/browser/actions/listCommands.ts | 33 ++----------------- .../extensions/browser/extensionsWidgets.ts | 7 ++-- 7 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/vs/base/browser/ui/hover/hover.ts b/src/vs/base/browser/ui/hover/hover.ts index a0f9422ce05..f2b7582d7fa 100644 --- a/src/vs/base/browser/ui/hover/hover.ts +++ b/src/vs/base/browser/ui/hover/hover.ts @@ -43,6 +43,11 @@ export interface IHoverDelegate2 { // TODO: Change hoverDelegate arg to exclude the actual delegate and instead use the new options setupUpdatableHover(hoverDelegate: IHoverDelegate, htmlElement: HTMLElement, content: IUpdatableHoverContentOrFactory, options?: IUpdatableHoverOptions): IUpdatableHover; + + /** + * Shows the hover for the given element if one has been setup. + */ + triggerUpdatableHover(htmlElement: HTMLElement): void; } export interface IHoverWidget extends IDisposable { @@ -246,6 +251,7 @@ export type IUpdatableHoverContentOrFactory = IUpdatableHoverContent | (() => IU export interface IUpdatableHoverOptions { actions?: IHoverAction[]; linkHandler?(url: string): void; + trapFocus?: boolean; } export interface IUpdatableHover extends IDisposable { diff --git a/src/vs/base/browser/ui/hover/hoverDelegate2.ts b/src/vs/base/browser/ui/hover/hoverDelegate2.ts index 90c71d65a1d..13a379222c1 100644 --- a/src/vs/base/browser/ui/hover/hoverDelegate2.ts +++ b/src/vs/base/browser/ui/hover/hoverDelegate2.ts @@ -10,6 +10,7 @@ let baseHoverDelegate: IHoverDelegate2 = { hideHover: () => undefined, showAndFocusLastHover: () => undefined, setupUpdatableHover: () => null!, + triggerUpdatableHover: () => undefined }; /** diff --git a/src/vs/editor/browser/services/hoverService/hoverService.ts b/src/vs/editor/browser/services/hoverService/hoverService.ts index bd74d1b3d67..8c5e7319331 100644 --- a/src/vs/editor/browser/services/hoverService/hoverService.ts +++ b/src/vs/editor/browser/services/hoverService/hoverService.ts @@ -62,7 +62,9 @@ export class HoverService extends Disposable implements IHoverService { // HACK, remove this check when #189076 is fixed if (!skipLastFocusedUpdate) { if (trapFocus && activeElement) { - this._lastFocusedElementBeforeOpen = activeElement as HTMLElement; + if (!activeElement.classList.contains('monaco-hover')) { + this._lastFocusedElementBeforeOpen = activeElement as HTMLElement; + } } else { this._lastFocusedElementBeforeOpen = undefined; } @@ -187,6 +189,8 @@ export class HoverService extends Disposable implements IHoverService { } } + private readonly _existingHovers = new Map(); + // TODO: Investigate performance of this function. There seems to be a lot of content created // and thrown away on start up setupUpdatableHover(hoverDelegate: IHoverDelegate, htmlElement: HTMLElement, content: IUpdatableHoverContentOrFactory, options?: IUpdatableHoverOptions | undefined): IUpdatableHover { @@ -216,15 +220,13 @@ export class HoverService extends Disposable implements IHoverService { hoverDelegate.onDidHideHover?.(); hoverWidget = undefined; } - htmlElement.removeAttribute('custom-hover-active'); }; - const triggerShowHover = (delay: number, focus?: boolean, target?: IHoverDelegateTarget) => { + const triggerShowHover = (delay: number, focus?: boolean, target?: IHoverDelegateTarget, trapFocus?: boolean) => { return new TimeoutTimer(async () => { if (!hoverWidget || hoverWidget.isDisposed) { hoverWidget = new UpdatableHoverWidget(hoverDelegate, target || htmlElement, delay > 0); - await hoverWidget.update(typeof content === 'function' ? content() : content, focus, options); - htmlElement.setAttribute('custom-hover-active', 'true'); + await hoverWidget.update(typeof content === 'function' ? content() : content, focus, { ...options, trapFocus }); } }, delay); }; @@ -299,7 +301,7 @@ export class HoverService extends Disposable implements IHoverService { const hover: IUpdatableHover = { show: focus => { hideHover(false, true); // terminate a ongoing mouse over preparation - triggerShowHover(0, focus); // show hover immediately + triggerShowHover(0, focus, undefined, focus); // show hover immediately }, hide: () => { hideHover(true, true); @@ -309,6 +311,7 @@ export class HoverService extends Disposable implements IHoverService { await hoverWidget?.update(content, undefined, hoverOptions); }, dispose: () => { + this._existingHovers.delete(htmlElement); mouseOverDomEmitter.dispose(); mouseLeaveEmitter.dispose(); mouseDownEmitter.dispose(); @@ -317,8 +320,21 @@ export class HoverService extends Disposable implements IHoverService { hideHover(true, true); } }; + this._existingHovers.set(htmlElement, hover); return hover; } + + triggerUpdatableHover(target: HTMLElement): void { + const hover = this._existingHovers.get(target); + if (hover) { + hover.show(true); + } + } + + public override dispose(): void { + this._existingHovers.forEach(hover => hover.dispose()); + super.dispose(); + } } function getHoverOptionsIdentity(options: IHoverOptions | undefined): IHoverOptions | number | string | undefined { diff --git a/src/vs/editor/browser/services/hoverService/updatableHoverWidget.ts b/src/vs/editor/browser/services/hoverService/updatableHoverWidget.ts index 869e493c1f9..762b6626aff 100644 --- a/src/vs/editor/browser/services/hoverService/updatableHoverWidget.ts +++ b/src/vs/editor/browser/services/hoverService/updatableHoverWidget.ts @@ -42,7 +42,7 @@ export class UpdatableHoverWidget implements IDisposable { // show 'Loading' if no hover is up yet if (!this._hoverWidget) { - this.show(localize('iconLabel.loading', "Loading..."), focus); + this.show(localize('iconLabel.loading', "Loading..."), focus, options); } // compute the content diff --git a/src/vs/platform/hover/test/browser/nullHoverService.ts b/src/vs/platform/hover/test/browser/nullHoverService.ts index 1040ba4d772..6b7b728325b 100644 --- a/src/vs/platform/hover/test/browser/nullHoverService.ts +++ b/src/vs/platform/hover/test/browser/nullHoverService.ts @@ -12,4 +12,5 @@ export const NullHoverService: IHoverService = { showHover: () => undefined, setupUpdatableHover: () => Disposable.None as any, showAndFocusLastHover: () => undefined, + triggerUpdatableHover: () => undefined }; diff --git a/src/vs/workbench/browser/actions/listCommands.ts b/src/vs/workbench/browser/actions/listCommands.ts index 12e356a7ea7..01de60e8080 100644 --- a/src/vs/workbench/browser/actions/listCommands.ts +++ b/src/vs/workbench/browser/actions/listCommands.ts @@ -18,11 +18,11 @@ import { ITreeNode } from 'vs/base/browser/ui/tree/tree'; import { CommandsRegistry } from 'vs/platform/commands/common/commands'; import { Table } from 'vs/base/browser/ui/table/tableWidget'; import { AbstractTree, TreeFindMatchType, TreeFindMode } from 'vs/base/browser/ui/tree/abstractTree'; -import { EventType, getActiveWindow, isActiveElement } from 'vs/base/browser/dom'; +import { isActiveElement } from 'vs/base/browser/dom'; import { Action2, registerAction2 } from 'vs/platform/actions/common/actions'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { localize, localize2 } from 'vs/nls'; -import { IDisposable } from 'vs/base/common/lifecycle'; +import { IHoverService } from 'vs/platform/hover/browser/hover'; function ensureDOMFocus(widget: ListWidget | undefined): void { // it can happen that one of the commands is executed while @@ -60,10 +60,6 @@ async function navigate(widget: WorkbenchListWidget | undefined, updateFocusFn: return; } - if (activeHover) { - toggleCustomHover(activeHover, widget); - } - await updateFocus(widget, updateFocusFn); const listFocus = widget.getFocus(); @@ -733,33 +729,10 @@ KeybindingsRegistry.registerCommandAndKeybindingRule({ return; } - toggleCustomHover(elementWithHover as HTMLElement, lastFocusedList); + accessor.get(IHoverService).triggerUpdatableHover(elementWithHover as HTMLElement); }, }); -let activeHover: undefined | HTMLElement; -let disposable: IDisposable | undefined; -function toggleCustomHover(element: HTMLElement, list: WorkbenchListWidget) { - const show = !element.getAttribute('custom-hover-active'); - const mouseEvent = new MouseEvent(show ? EventType.MOUSE_OVER : EventType.MOUSE_LEAVE, { - view: getActiveWindow(), - bubbles: true, - cancelable: true, - }); - element.dispatchEvent(mouseEvent); - - if (activeHover === element && !show) { - activeHover = undefined; - disposable?.dispose(); - disposable = undefined; - } else { - activeHover = element; - disposable = list.onDidBlur(() => { - toggleCustomHover(element, list); - }); - } -} - KeybindingsRegistry.registerCommandAndKeybindingRule({ id: 'list.toggleExpand', weight: KeybindingWeight.WorkbenchContrib, diff --git a/src/vs/workbench/contrib/extensions/browser/extensionsWidgets.ts b/src/vs/workbench/contrib/extensions/browser/extensionsWidgets.ts index e94ab96d100..3d725460a20 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionsWidgets.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionsWidgets.ts @@ -553,7 +553,7 @@ export class ExtensionHoverWidget extends ExtensionWidget { if (this.extension) { this.hover.value = this.hoverService.setupUpdatableHover({ delay: this.configurationService.getValue('workbench.hover.delay'), - showHover: (options) => { + showHover: (options, focus) => { return this.hoverService.showHover({ ...options, additionalClasses: ['extension-hover'], @@ -561,7 +561,10 @@ export class ExtensionHoverWidget extends ExtensionWidget { hoverPosition: this.options.position(), forcePosition: true, }, - }); + persistence: { + hideOnKeyDown: true, + } + }, focus); }, placement: 'element' }, this.options.target, { markdown: () => Promise.resolve(this.getHoverMarkdown()), markdownNotSupportedFallback: undefined });