Skip to content

Commit 28738da

Browse files
benibenjbenibenj
authored
managed hover handles native hover implementation (microsoft#254825)
* managed hover should handle native hover impl and not widgets * 💄
1 parent e3710cb commit 28738da

File tree

7 files changed

+55
-58
lines changed

7 files changed

+55
-58
lines changed

‎src/vs/base/browser/ui/actionbar/actionViewItems.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -228,16 +228,11 @@ export class BaseActionViewItem extends Disposable implements IActionViewItem {
228228
const title = this.getTooltip() ?? '';
229229
this.updateAriaLabel();
230230

231-
if (this.options.hoverDelegate?.showNativeHover) {
232-
/* While custom hover is not inside custom hover */
233-
this.element.title = title;
234-
} else {
235-
if (!this.customHover && title !== '') {
236-
const hoverDelegate = this.options.hoverDelegate ?? getDefaultHoverDelegate('element');
237-
this.customHover = this._store.add(getBaseLayerHoverDelegate().setupManagedHover(hoverDelegate, this.element, title));
238-
} else if (this.customHover) {
239-
this.customHover.update(title);
240-
}
231+
if (!this.customHover && title !== '') {
232+
const hoverDelegate = this.options.hoverDelegate ?? getDefaultHoverDelegate('element');
233+
this.customHover = this._store.add(getBaseLayerHoverDelegate().setupManagedHover(hoverDelegate, this.element, title));
234+
} else if (this.customHover) {
235+
this.customHover.update(title);
241236
}
242237
}
243238

‎src/vs/base/browser/ui/button/button.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -333,9 +333,7 @@ export class Button extends Disposable implements IButton {
333333
}
334334

335335
setTitle(title: string) {
336-
if (this.options.hoverDelegate?.showNativeHover) {
337-
this._element.title = title;
338-
} else if (!this._hover && title !== '') {
336+
if (!this._hover && title !== '') {
339337
this._hover = this._register(getBaseLayerHoverDelegate().setupManagedHover(this.options.hoverDelegate ?? getDefaultHoverDelegate('element'), this._element, title));
340338
} else if (this._hover) {
341339
this._hover.update(title);

‎src/vs/base/browser/ui/highlightedlabel/highlightedLabel.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -135,16 +135,11 @@ export class HighlightedLabel extends Disposable {
135135

136136
dom.reset(this.domNode, ...children);
137137

138-
if (this.options?.hoverDelegate?.showNativeHover) {
139-
/* While custom hover is not inside custom hover */
140-
this.domNode.title = this.title;
141-
} else {
142-
if (!this.customHover && this.title !== '') {
143-
const hoverDelegate = this.options?.hoverDelegate ?? getDefaultHoverDelegate('mouse');
144-
this.customHover = this._register(getBaseLayerHoverDelegate().setupManagedHover(hoverDelegate, this.domNode, this.title));
145-
} else if (this.customHover) {
146-
this.customHover.update(this.title);
147-
}
138+
if (!this.customHover && this.title !== '') {
139+
const hoverDelegate = this.options?.hoverDelegate ?? getDefaultHoverDelegate('mouse');
140+
this.customHover = this._register(getBaseLayerHoverDelegate().setupManagedHover(hoverDelegate, this.domNode, this.title));
141+
} else if (this.customHover) {
142+
this.customHover.update(this.title);
148143
}
149144

150145
this.didEverRender = true;

‎src/vs/base/browser/ui/iconLabel/iconLabel.ts

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ import { Range } from '../../../common/range.js';
1515
import { getDefaultHoverDelegate } from '../hover/hoverDelegateFactory.js';
1616
import type { IManagedHoverTooltipMarkdownString } from '../hover/hover.js';
1717
import { getBaseLayerHoverDelegate } from '../hover/hoverDelegate2.js';
18-
import { isString } from '../../../common/types.js';
19-
import { stripIcons } from '../../../common/iconLabels.js';
2018
import { URI } from '../../../common/uri.js';
2119

2220
export interface IIconLabelCreationOptions {
@@ -222,23 +220,9 @@ export class IconLabel extends Disposable {
222220
hoverTarget = this.creationOptions.hoverTargetOverride;
223221
}
224222

225-
if (this.hoverDelegate.showNativeHover) {
226-
function setupNativeHover(htmlElement: HTMLElement, tooltip: string | IManagedHoverTooltipMarkdownString | undefined): void {
227-
if (isString(tooltip)) {
228-
// Icons don't render in the native hover so we strip them out
229-
htmlElement.title = stripIcons(tooltip);
230-
} else if (tooltip?.markdownNotSupportedFallback) {
231-
htmlElement.title = tooltip.markdownNotSupportedFallback;
232-
} else {
233-
htmlElement.removeAttribute('title');
234-
}
235-
}
236-
setupNativeHover(hoverTarget, tooltip);
237-
} else {
238-
const hoverDisposable = getBaseLayerHoverDelegate().setupManagedHover(this.hoverDelegate, hoverTarget, tooltip);
239-
if (hoverDisposable) {
240-
this.customHovers.set(htmlElement, hoverDisposable);
241-
}
223+
const hoverDisposable = getBaseLayerHoverDelegate().setupManagedHover(this.hoverDelegate, hoverTarget, tooltip);
224+
if (hoverDisposable) {
225+
this.customHovers.set(htmlElement, hoverDisposable);
242226
}
243227
}
244228

‎src/vs/base/browser/ui/toggle/toggle.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ export class Toggle extends Widget {
132132
readonly domNode: HTMLElement;
133133

134134
private _checked: boolean;
135-
private _hover?: IManagedHover;
135+
private _hover: IManagedHover;
136136

137137
constructor(opts: IToggleOpts) {
138138
super();
@@ -153,11 +153,7 @@ export class Toggle extends Widget {
153153
}
154154

155155
this.domNode = document.createElement('div');
156-
if (this._opts.hoverDelegate?.showNativeHover) {
157-
this.domNode.title = this._opts.title;
158-
} else {
159-
this._hover = this._register(getBaseLayerHoverDelegate().setupManagedHover(opts.hoverDelegate ?? getDefaultHoverDelegate('mouse'), this.domNode, this._opts.title));
160-
}
156+
this._hover = this._register(getBaseLayerHoverDelegate().setupManagedHover(opts.hoverDelegate ?? getDefaultHoverDelegate('mouse'), this.domNode, this._opts.title));
161157
this.domNode.classList.add(...classes);
162158
if (!this._opts.notFocusable) {
163159
this.domNode.tabIndex = 0;
@@ -249,11 +245,7 @@ export class Toggle extends Widget {
249245
}
250246

251247
setTitle(newTitle: string): void {
252-
if (this._hover) {
253-
this._hover.update(newTitle);
254-
} else {
255-
this.domNode.title = newTitle;
256-
}
248+
this._hover.update(newTitle);
257249
this.domNode.setAttribute('aria-label', newTitle);
258250
}
259251

‎src/vs/editor/browser/services/hoverService/hoverService.ts

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,17 @@ import { IAccessibilityService } from '../../../../platform/accessibility/common
2020
import { ILayoutService } from '../../../../platform/layout/browser/layoutService.js';
2121
import { mainWindow } from '../../../../base/browser/window.js';
2222
import { ContextViewHandler } from '../../../../platform/contextview/browser/contextViewService.js';
23-
import type { IHoverLifecycleOptions, IHoverOptions, IHoverWidget, IManagedHover, IManagedHoverContentOrFactory, IManagedHoverOptions } from '../../../../base/browser/ui/hover/hover.js';
23+
import { isManagedHoverTooltipMarkdownString, type IHoverLifecycleOptions, type IHoverOptions, type IHoverWidget, type IManagedHover, type IManagedHoverContentOrFactory, type IManagedHoverOptions } from '../../../../base/browser/ui/hover/hover.js';
2424
import type { IHoverDelegate, IHoverDelegateTarget } from '../../../../base/browser/ui/hover/hoverDelegate.js';
2525
import { ManagedHoverWidget } from './updatableHoverWidget.js';
2626
import { timeout, TimeoutTimer } from '../../../../base/common/async.js';
2727
import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
28-
import { isNumber } from '../../../../base/common/types.js';
28+
import { isNumber, isString } from '../../../../base/common/types.js';
2929
import { KeyChord, KeyCode, KeyMod } from '../../../../base/common/keyCodes.js';
3030
import { KeybindingsRegistry, KeybindingWeight } from '../../../../platform/keybinding/common/keybindingsRegistry.js';
3131
import { EditorContextKeys } from '../../../common/editorContextKeys.js';
3232
import { IMarkdownString } from '../../../../base/common/htmlContent.js';
33+
import { stripIcons } from '../../../../base/common/iconLabels.js';
3334

3435
export class HoverService extends Disposable implements IHoverService {
3536
declare readonly _serviceBrand: undefined;
@@ -368,6 +369,10 @@ export class HoverService extends Disposable implements IHoverService {
368369
// TODO: Investigate performance of this function. There seems to be a lot of content created
369370
// and thrown away on start up
370371
setupManagedHover(hoverDelegate: IHoverDelegate, targetElement: HTMLElement, content: IManagedHoverContentOrFactory, options?: IManagedHoverOptions | undefined): IManagedHover {
372+
if (hoverDelegate.showNativeHover) {
373+
return setupNativeHover(targetElement, content);
374+
}
375+
371376
targetElement.setAttribute('custom-hover', 'true');
372377

373378
if (targetElement.title !== '') {
@@ -520,6 +525,36 @@ function getHoverIdFromContent(content: string | HTMLElement | IMarkdownString):
520525
return content.value;
521526
}
522527

528+
function getStringContent(contentOrFactory: IManagedHoverContentOrFactory): string | undefined {
529+
const content = typeof contentOrFactory === 'function' ? contentOrFactory() : contentOrFactory;
530+
if (isString(content)) {
531+
// Icons don't render in the native hover so we strip them out
532+
return stripIcons(content);
533+
}
534+
if (isManagedHoverTooltipMarkdownString(content)) {
535+
return content.markdownNotSupportedFallback;
536+
}
537+
return undefined;
538+
}
539+
540+
function setupNativeHover(targetElement: HTMLElement, content: IManagedHoverContentOrFactory): IManagedHover {
541+
function updateTitle(title: string | undefined) {
542+
if (title) {
543+
targetElement.setAttribute('title', title);
544+
} else {
545+
targetElement.removeAttribute('title');
546+
}
547+
}
548+
549+
updateTitle(getStringContent(content));
550+
return {
551+
update: (content) => updateTitle(getStringContent(content)),
552+
show: () => { },
553+
hide: () => { },
554+
dispose: () => updateTitle(undefined),
555+
};
556+
}
557+
523558
class HoverContextViewDelegate implements IDelegate {
524559

525560
// Render over all other context views

‎src/vs/platform/opener/browser/link.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,7 @@ export class Link extends Disposable {
128128
}
129129

130130
private setTooltip(title: string | undefined): void {
131-
if (this.hoverDelegate.showNativeHover) {
132-
this.el.title = title ?? '';
133-
} else if (!this.hover && title) {
131+
if (!this.hover && title) {
134132
this.hover = this._register(this._hoverService.setupManagedHover(this.hoverDelegate, this.el, title));
135133
} else if (this.hover) {
136134
this.hover.update(title);

0 commit comments

Comments
 (0)