Skip to content

Remove deprecated managedhover from findwidget + toggle #270313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
+131 −60
Draft

Remove deprecated managedhover from findwidget + toggle #270313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
73782ae
Remove deprecated managedhover from findwidget + toggle
Tyriar Oct 8, 2025
c5cb367
Cover tree find/filter
Tyriar Oct 8, 2025
dd36ce9
Migrate showContextToggle in search editor
Tyriar Oct 8, 2025
57a1fbd
Pass lifecycle options into nb find filters
Tyriar Oct 8, 2025
15baeaa
Adopt in patterns input widget
Tyriar Oct 8, 2025
6a66031
Remove hoverDelegate from ICheckboxStyles
Tyriar Oct 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/vs/base/browser/ui/findinput/findInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import { KeyCode } from '../../../common/keyCodes.js';
import './findInput.css';
import * as nls from '../../../../nls.js';
import { DisposableStore, MutableDisposable } from '../../../common/lifecycle.js';
import { createInstantHoverDelegate } from '../hover/hoverDelegateFactory.js';
import { IHistory } from '../../../common/history.js';
import type { IHoverLifecycleOptions } from '../hover/hover.js';


export interface IFindInputOptions {
Expand All @@ -38,6 +38,7 @@ export interface IFindInputOptions {
readonly toggleStyles: IToggleStyles;
readonly inputBoxStyles: IInputBoxStyles;
readonly history?: IHistory<string>;
readonly hoverLifecycleOptions?: IHoverLifecycleOptions;
}

const NLS_DEFAULT_LABEL = nls.localize('defaultLabel', "input");
Expand Down Expand Up @@ -114,13 +115,12 @@ export class FindInput extends Widget {
history: options.history
}));

const hoverDelegate = this._register(createInstantHoverDelegate());

if (this.showCommonFindToggles) {
const hoverLifecycleOptions: IHoverLifecycleOptions = options?.hoverLifecycleOptions || { groupId: 'find-input' };
this.regex = this._register(new RegexToggle({
appendTitle: appendRegexLabel,
isChecked: false,
hoverDelegate,
hoverLifecycleOptions,
...options.toggleStyles
}));
this._register(this.regex.onChange(viaKeyboard => {
Expand All @@ -137,7 +137,7 @@ export class FindInput extends Widget {
this.wholeWords = this._register(new WholeWordsToggle({
appendTitle: appendWholeWordsLabel,
isChecked: false,
hoverDelegate,
hoverLifecycleOptions,
...options.toggleStyles
}));
this._register(this.wholeWords.onChange(viaKeyboard => {
Expand All @@ -151,7 +151,7 @@ export class FindInput extends Widget {
this.caseSensitive = this._register(new CaseSensitiveToggle({
appendTitle: appendCaseSensitiveLabel,
isChecked: false,
hoverDelegate,
hoverLifecycleOptions,
...options.toggleStyles
}));
this._register(this.caseSensitive.onChange(viaKeyboard => {
Expand Down
14 changes: 8 additions & 6 deletions src/vs/base/browser/ui/findinput/findInputToggles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,18 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { getDefaultHoverDelegate } from '../hover/hoverDelegateFactory.js';
import { IHoverDelegate } from '../hover/hoverDelegate.js';
import { Toggle } from '../toggle/toggle.js';
import { Codicon } from '../../../common/codicons.js';
import * as nls from '../../../../nls.js';
import { HoverStyle, type IHoverLifecycleOptions } from '../hover/hover.js';

export interface IFindInputToggleOpts {
readonly appendTitle: string;
readonly isChecked: boolean;
readonly inputActiveOptionBorder: string | undefined;
readonly inputActiveOptionForeground: string | undefined;
readonly inputActiveOptionBackground: string | undefined;
readonly hoverDelegate?: IHoverDelegate;
readonly hoverLifecycleOptions?: IHoverLifecycleOptions;
}

const NLS_CASE_SENSITIVE_TOGGLE_LABEL = nls.localize('caseDescription', "Match Case");
Expand All @@ -28,7 +27,8 @@ export class CaseSensitiveToggle extends Toggle {
icon: Codicon.caseSensitive,
title: NLS_CASE_SENSITIVE_TOGGLE_LABEL + opts.appendTitle,
isChecked: opts.isChecked,
hoverDelegate: opts.hoverDelegate ?? getDefaultHoverDelegate('element'),
hoverStyle: HoverStyle.Pointer,
hoverLifecycleOptions: opts.hoverLifecycleOptions,
inputActiveOptionBorder: opts.inputActiveOptionBorder,
inputActiveOptionForeground: opts.inputActiveOptionForeground,
inputActiveOptionBackground: opts.inputActiveOptionBackground
Expand All @@ -42,7 +42,8 @@ export class WholeWordsToggle extends Toggle {
icon: Codicon.wholeWord,
title: NLS_WHOLE_WORD_TOGGLE_LABEL + opts.appendTitle,
isChecked: opts.isChecked,
hoverDelegate: opts.hoverDelegate ?? getDefaultHoverDelegate('element'),
hoverStyle: HoverStyle.Pointer,
hoverLifecycleOptions: opts.hoverLifecycleOptions,
inputActiveOptionBorder: opts.inputActiveOptionBorder,
inputActiveOptionForeground: opts.inputActiveOptionForeground,
inputActiveOptionBackground: opts.inputActiveOptionBackground
Expand All @@ -56,7 +57,8 @@ export class RegexToggle extends Toggle {
icon: Codicon.regex,
title: NLS_REGEX_TOGGLE_LABEL + opts.appendTitle,
isChecked: opts.isChecked,
hoverDelegate: opts.hoverDelegate ?? getDefaultHoverDelegate('element'),
hoverStyle: HoverStyle.Pointer,
hoverLifecycleOptions: opts.hoverLifecycleOptions,
inputActiveOptionBorder: opts.inputActiveOptionBorder,
inputActiveOptionForeground: opts.inputActiveOptionForeground,
inputActiveOptionBackground: opts.inputActiveOptionBackground
Expand Down
3 changes: 1 addition & 2 deletions src/vs/base/browser/ui/findinput/replaceInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { Emitter, Event } from '../../../common/event.js';
import { KeyCode } from '../../../common/keyCodes.js';
import './findInput.css';
import * as nls from '../../../../nls.js';
import { getDefaultHoverDelegate } from '../hover/hoverDelegateFactory.js';
import { IHistory } from '../../../common/history.js';


Expand Down Expand Up @@ -46,7 +45,7 @@ class PreserveCaseToggle extends Toggle {
icon: Codicon.preserveCase,
title: NLS_PRESERVE_CASE_LABEL + opts.appendTitle,
isChecked: opts.isChecked,
hoverDelegate: opts.hoverDelegate ?? getDefaultHoverDelegate('element'),
hoverLifecycleOptions: opts.hoverLifecycleOptions,
inputActiveOptionBorder: opts.inputActiveOptionBorder,
inputActiveOptionForeground: opts.inputActiveOptionForeground,
inputActiveOptionBackground: opts.inputActiveOptionBackground,
Expand Down
16 changes: 16 additions & 0 deletions src/vs/base/browser/ui/hover/hover.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,17 @@ export interface IHoverWidget extends IDisposable {
readonly isDisposed: boolean;
}

export const enum HoverStyle {
/**
* The hover is anchored below the element with a pointer above it pointing at the target.
*/
Pointer = 1,
/**
* The hover is anchored to the bottom right of the cursor's location.
*/
Mouse = 2,
}

export interface IHoverOptions {
/**
* The content to display in the primary section of the hover. The type of text determines the
Expand Down Expand Up @@ -205,6 +216,11 @@ export interface IHoverOptions {
*/
trapFocus?: boolean;

/**
* The style of the hover, this sets default values of {@link position} and {@link appearance}:
*/
style?: HoverStyle;

/**
* Options that defines where the hover is positioned.
*/
Expand Down
27 changes: 17 additions & 10 deletions src/vs/base/browser/ui/toggle/toggle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@ import { ThemeIcon } from '../../../common/themables.js';
import { $, addDisposableListener, EventType, isActiveElement } from '../../dom.js';
import { IKeyboardEvent } from '../../keyboardEvent.js';
import { BaseActionViewItem, IActionViewItemOptions } from '../actionbar/actionViewItems.js';
import type { IManagedHover } from '../hover/hover.js';
import { HoverStyle, IHoverLifecycleOptions } from '../hover/hover.js';
import { IHoverDelegate } from '../hover/hoverDelegate.js';
import { getBaseLayerHoverDelegate } from '../hover/hoverDelegate2.js';
import { getDefaultHoverDelegate } from '../hover/hoverDelegateFactory.js';
import { Widget } from '../widget.js';
import './toggle.css';

Expand All @@ -24,7 +23,11 @@ export interface IToggleOpts extends IToggleStyles {
readonly title: string;
readonly isChecked: boolean;
readonly notFocusable?: boolean;
// TODO: Remove this, the previous default was mouse, so anything not mouse needs to be explicit
/** @deprecated Prefer hoverStyle and hoverLifecycleOptions instead */
readonly hoverDelegate?: IHoverDelegate;
readonly hoverStyle?: HoverStyle;
readonly hoverLifecycleOptions?: IHoverLifecycleOptions;
Comment on lines +27 to +30
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benibenj what do you think about this as a way to get rid of hoverDelegate in base? I think base only needs to set a groupId and a style (pointer or mouse). We don't want to be passing around callbacks to setupDelayedHover or setupDelayedHoverAtMouse inside options so simple options like this are preferable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I build upon this in #270749 attempting to do the same for IBaseActionViewItemOptions but I might abandon it there as it's a lot of work to validate and a huge change that can't really be done in stages AFAICT and I wanted to get your view on the HoverStyle and lifecycle options replacing hoverDelegate.

}

export interface IToggleStyles {
Expand All @@ -40,7 +43,7 @@ export interface ICheckboxStyles {
readonly checkboxDisabledBackground: string | undefined;
readonly checkboxDisabledForeground: string | undefined;
readonly size?: number;
readonly hoverDelegate?: IHoverDelegate;
readonly hoverLifecycleOptions?: IHoverLifecycleOptions;
}

export const unthemedToggleStyles = {
Expand All @@ -66,7 +69,7 @@ export class ToggleActionViewItem extends BaseActionViewItem {
inputActiveOptionBackground: options.toggleStyles?.inputActiveOptionBackground,
inputActiveOptionBorder: options.toggleStyles?.inputActiveOptionBorder,
inputActiveOptionForeground: options.toggleStyles?.inputActiveOptionForeground,
hoverDelegate: options.hoverDelegate
hoverDelegate: options.hoverDelegate,
}));
this._register(this.toggle.onChange(() => {
this._action.checked = !!this.toggle && this.toggle.checked;
Expand Down Expand Up @@ -128,16 +131,17 @@ export class Toggle extends Widget {
get onKeyDown(): Event<IKeyboardEvent> { return this._onKeyDown.event; }

private readonly _opts: IToggleOpts;
private _title: string;
private _icon: ThemeIcon | undefined;
readonly domNode: HTMLElement;

private _checked: boolean;
private _hover: IManagedHover;

constructor(opts: IToggleOpts) {
super();

this._opts = opts;
this._title = this._opts.title;
this._checked = this._opts.isChecked;

const classes = ['monaco-custom-toggle'];
Expand All @@ -153,15 +157,18 @@ export class Toggle extends Widget {
}

this.domNode = document.createElement('div');
this._hover = this._register(getBaseLayerHoverDelegate().setupManagedHover(opts.hoverDelegate ?? getDefaultHoverDelegate('mouse'), this.domNode, this._opts.title));
this._register(getBaseLayerHoverDelegate().setupDelayedHover(this.domNode, () => ({
content: this._title,
style: this._opts.hoverStyle ?? HoverStyle.Mouse,
}), this._opts.hoverLifecycleOptions));
this.domNode.classList.add(...classes);
if (!this._opts.notFocusable) {
this.domNode.tabIndex = 0;
}
this.domNode.setAttribute('role', 'checkbox');
this.domNode.setAttribute('aria-checked', String(this._checked));
this.domNode.setAttribute('aria-label', this._opts.title);

this.setTitle(this._opts.title);
this.applyStyles();

this.onclick(this.domNode, (ev) => {
Expand Down Expand Up @@ -245,7 +252,7 @@ export class Toggle extends Widget {
}

setTitle(newTitle: string): void {
this._hover.update(newTitle);
this._title = newTitle;
this.domNode.setAttribute('aria-label', newTitle);
}

Expand Down Expand Up @@ -316,7 +323,7 @@ abstract class BaseCheckbox extends Widget {

export class Checkbox extends BaseCheckbox {
constructor(title: string, isChecked: boolean, styles: ICheckboxStyles) {
const toggle = new Toggle({ title, isChecked, icon: Codicon.check, actionClassName: BaseCheckbox.CLASS_NAME, hoverDelegate: styles.hoverDelegate, ...unthemedToggleStyles });
const toggle = new Toggle({ title, isChecked, icon: Codicon.check, actionClassName: BaseCheckbox.CLASS_NAME, hoverLifecycleOptions: styles.hoverLifecycleOptions, ...unthemedToggleStyles });
super(toggle, toggle.domNode, styles);

this._register(toggle);
Expand Down Expand Up @@ -368,7 +375,7 @@ export class TriStateCheckbox extends BaseCheckbox {
isChecked: _state === true,
icon,
actionClassName: Checkbox.CLASS_NAME,
hoverDelegate: styles.hoverDelegate,
hoverLifecycleOptions: styles.hoverLifecycleOptions,
...unthemedToggleStyles
});
super(
Expand Down
16 changes: 9 additions & 7 deletions src/vs/base/browser/ui/tree/abstractTree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,10 @@ import { clamp } from '../../../common/numbers.js';
import { ScrollEvent } from '../../../common/scrollable.js';
import './media/tree.css';
import { localize } from '../../../../nls.js';
import { IHoverDelegate } from '../hover/hoverDelegate.js';
import { createInstantHoverDelegate } from '../hover/hoverDelegateFactory.js';
import { autorun, constObservable } from '../../../common/observable.js';
import { alert } from '../aria/aria.js';
import { IMouseWheelEvent } from '../../mouseEvent.js';
import { HoverStyle, type IHoverLifecycleOptions } from '../hover/hover.js';

class TreeElementsDragAndDropData<T, TFilterData, TContext> extends ElementsDragAndDropData<T, TContext> {

Expand Down Expand Up @@ -708,15 +707,16 @@ class TreeFindToggle extends Toggle {

readonly id: string;

constructor(contribution: ITreeFindToggleContribution, opts: IToggleStyles, hoverDelegate?: IHoverDelegate) {
constructor(contribution: ITreeFindToggleContribution, opts: IToggleStyles, hoverLifecycleOptions?: IHoverLifecycleOptions) {
super({
icon: contribution.icon,
title: contribution.title,
isChecked: contribution.isChecked,
inputActiveOptionBorder: opts.inputActiveOptionBorder,
inputActiveOptionForeground: opts.inputActiveOptionForeground,
inputActiveOptionBackground: opts.inputActiveOptionBackground,
hoverDelegate,
hoverStyle: HoverStyle.Pointer,
hoverLifecycleOptions,
});

this.id = contribution.id;
Expand Down Expand Up @@ -840,8 +840,9 @@ class FindWidget<T, TFilterData> extends Disposable {
this.elements.root.style.boxShadow = `0 0 8px 2px ${styles.listFilterWidgetShadow}`;
}

const toggleHoverDelegate = this._register(createInstantHoverDelegate());
this.toggles = toggleContributions.map(contribution => this._register(new TreeFindToggle(contribution, styles.toggleStyles, toggleHoverDelegate)));
// const toggleHoverDelegate = this._register(createInstantHoverDelegate());
const hoverLifecycleOptions: IHoverLifecycleOptions = { groupId: 'abstract-tree' };
this.toggles = toggleContributions.map(contribution => this._register(new TreeFindToggle(contribution, styles.toggleStyles, hoverLifecycleOptions)));
this.onDidToggleChange = Event.any(...this.toggles.map(toggle => Event.map(toggle.onChange, () => ({ id: toggle.id, isChecked: toggle.checked }))));

const history = options?.history || [];
Expand All @@ -852,7 +853,8 @@ class FindWidget<T, TFilterData> extends Disposable {
showCommonFindToggles: false,
inputBoxStyles: styles.inputBoxStyles,
toggleStyles: styles.toggleStyles,
history: new Set(history)
history: new Set(history),
hoverLifecycleOptions,
}));

this.actionbar = this._register(new ActionBar(this.elements.actionbar));
Expand Down
20 changes: 19 additions & 1 deletion src/vs/editor/browser/services/hoverService/hoverWidget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { localize } from '../../../../nls.js';
import { isMacintosh } from '../../../../base/common/platform.js';
import { IAccessibilityService } from '../../../../platform/accessibility/common/accessibility.js';
import { status } from '../../../../base/browser/ui/aria/aria.js';
import type { IHoverOptions, IHoverTarget, IHoverWidget } from '../../../../base/browser/ui/hover/hover.js';
import { HoverStyle, type IHoverOptions, type IHoverTarget, type IHoverWidget } from '../../../../base/browser/ui/hover/hover.js';
import { TimeoutTimer } from '../../../../base/common/async.js';
import { isNumber } from '../../../../base/common/types.js';

Expand Down Expand Up @@ -113,6 +113,24 @@ export class HoverWidget extends Widget implements IHoverWidget {

this._target = 'targetElements' in options.target ? options.target : new ElementHoverTarget(options.target);

if (options.style) {
switch (options.style) {
case HoverStyle.Pointer: {
options.appearance ??= {};
options.appearance.compact ??= true;
options.appearance.showPointer ??= true;
options.position ??= {};
options.position.hoverPosition ??= HoverPosition.BELOW;
break;
}
case HoverStyle.Mouse: {
options.appearance ??= {};
options.appearance.compact ??= true;
break;
}
}
}

this._hoverPointer = options.appearance?.showPointer ? $('div.workbench-hover-pointer') : undefined;
this._hover = this._register(new BaseHoverWidget(!options.appearance?.skipFadeInAnimation));
this._hover.containerDomNode.classList.add('workbench-hover');
Expand Down
10 changes: 5 additions & 5 deletions src/vs/editor/contrib/find/browser/findOptionsWidget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { FIND_IDS } from './findModel.js';
import { FindReplaceState } from './findState.js';
import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js';
import { asCssVariable, inputActiveOptionBackground, inputActiveOptionBorder, inputActiveOptionForeground } from '../../../../platform/theme/common/colorRegistry.js';
import { createInstantHoverDelegate } from '../../../../base/browser/ui/hover/hoverDelegateFactory.js';
import type { IHoverLifecycleOptions } from '../../../../base/browser/ui/hover/hover.js';

export class FindOptionsWidget extends Widget implements IOverlayWidget {

Expand Down Expand Up @@ -53,12 +53,12 @@ export class FindOptionsWidget extends Widget implements IOverlayWidget {
inputActiveOptionBackground: asCssVariable(inputActiveOptionBackground),
};

const hoverDelegate = this._register(createInstantHoverDelegate());
const hoverLifecycleOptions: IHoverLifecycleOptions = { groupId: 'find-options-widget' };

this.caseSensitive = this._register(new CaseSensitiveToggle({
appendTitle: this._keybindingLabelFor(FIND_IDS.ToggleCaseSensitiveCommand),
isChecked: this._state.matchCase,
hoverDelegate,
hoverLifecycleOptions,
...toggleStyles
}));
this._domNode.appendChild(this.caseSensitive.domNode);
Expand All @@ -71,7 +71,7 @@ export class FindOptionsWidget extends Widget implements IOverlayWidget {
this.wholeWords = this._register(new WholeWordsToggle({
appendTitle: this._keybindingLabelFor(FIND_IDS.ToggleWholeWordCommand),
isChecked: this._state.wholeWord,
hoverDelegate,
hoverLifecycleOptions,
...toggleStyles
}));
this._domNode.appendChild(this.wholeWords.domNode);
Expand All @@ -84,7 +84,7 @@ export class FindOptionsWidget extends Widget implements IOverlayWidget {
this.regex = this._register(new RegexToggle({
appendTitle: this._keybindingLabelFor(FIND_IDS.ToggleRegexCommand),
isChecked: this._state.isRegex,
hoverDelegate,
hoverLifecycleOptions,
...toggleStyles
}));
this._domNode.appendChild(this.regex.domNode);
Expand Down
Loading
Loading