Generic hover service support #11869
Conversation
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Looks good to me, so I endorse @vince-fugnitto approval. Only one minor comment.
| protected getHoverDelay(): number { | ||
| return Date.now() - this.lastHidHover < 200 | ||
| ? 0 | ||
| : this.preferences.get('workbench.hover.delay', isOSX ? 1500 : 500); | ||
| } |
There was a problem hiding this comment.
@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.
| @@ -135,6 +135,7 @@ import { bindStatusBar } from './status-bar'; | |||
| 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'; | |||
What it does
Fixes #11594
Fixes #10867
Also addresses a comment I've left in another PR #11083 (comment).
Uses the
StatusBarHoverManager(nowHoverService) as a base for building a generic hover service for markdown/string/html display as hovers/tooltips. This retires theTooltipService(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
resolveTreeItemtoTreeDataProvider#11708)Review checklist
Reminder for reviewers