Skip to content

Conversation

@msujew
Copy link
Member

@msujew msujew commented Nov 15, 2022

What it does

Fixes #11594
Fixes #10867
Also addresses a comment I've left in another PR #11083 (comment).

Uses the StatusBarHoverManager (now HoverService) as a base for building a generic hover service for markdown/string/html display as hovers/tooltips. This retires the TooltipService (introduced in #10108), which was the source of the error messages in #11594.

One feature that is missing here is that the hover could appear at the cursor position, right now it's bound to the position of the target element. I plan to add that in a separate PR later on.

How to test

  1. Confirm that hovers still appear correctly on status bars (see also Implement missing API for [email protected] #11083)
  2. Confirm that hovers appear correctly on icons of widgets on the side bar panels (navigator, debug, extension, preferences, etc.)
  3. Confirm that hovers appear correctly on tree nodes (see also vscode: Add support for resolveTreeItem to TreeDataProvider #11708)
  4. Confirm that hovers appear correctly in the vsx extension view (see also Add rich tooltips for plugin trees and VSX extensions #10108)

Review checklist

Reminder for reviewers

@msujew msujew added the ui/ux issues related to user interface / user experience label Nov 15, 2022
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I can confirm that the tooltips work really well with the new hover service 👍

I might actually open an issue in vscode for them to standardize using these tooltips everywhere for a more consistent look and feel (ex: explorer file paths, tree-view hover, toolbar item hover).

One improvement would be to adjust the hr rule for the vsx-extensions view as the horizontal rule is not styled similarly to vscode since we have a padding on each side:

hr.mp4

Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

Looks good to me, so I endorse @vince-fugnitto approval. Only one minor comment.

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look good to me 👍
I confirmed that my previous comment about the styling was addressed.

@msujew msujew merged commit 909f410 into master Nov 22, 2022
@msujew msujew deleted the msujew/generalize-hover-manager branch November 22, 2022 15:36
@github-actions github-actions bot added this to the 1.32.0 milestone Nov 22, 2022
Comment on lines +96 to +100
protected getHoverDelay(): number {
return Date.now() - this.lastHidHover < 200
? 0
: this.preferences.get('workbench.hover.delay', isOSX ? 1500 : 500);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@msujew, I notice that in Theia with this change, if any hover has recently been rendered, then any other hover, of the same or different kind, will be rendered immediately, but in VSCode, it appears to differ by source. So e.g. if you hover over an extension tree node, get the hover, then hover over a sidebar tabbar icon, the tabbar icon's title hover will appear immediately, but in VSCode, it will only hover after a delay.

import { MarkdownRenderer, MarkdownRendererFactory, MarkdownRendererImpl } from './markdown-rendering/markdown-renderer';
import { StylingParticipant, StylingService } from './styling-service';
import { bindCommonStylingParticipants } from './common-styling-participants';
import { HoverService } from './hover-service';
Copy link

Choose a reason for hiding this comment

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

_

@colin-grant-work
Copy link
Contributor

@msujew, I was looking at the asynchronous tooltip content provision part of #16280 and came across code for the old TooltipService from #10108. In my view, this PR rendered that service obsolete, and it's no longer used by this repo - we just attach it to the DOM and never use it again. Should we perhaps delete / deprecate that service in favor of this?

@msujew
Copy link
Member Author

msujew commented Oct 3, 2025

@colin-grant-work Yeah, makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui/ux issues related to user interface / user experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New React error in TreeWidget after react-virtuoso change React Tooltips can get stuck

4 participants