Feature: render markdown and fix tooltip dimensions

[akinsho]

Following comments in #1237 I've had a look at implementing some changes to the tooltip:

1. Fix margin so text isn't clipped
2. Change sizing so it isn't rendered at almost full width (which is arguably a bit too big)
3. Render markdown from LSP in quickinfo using marked.

This is still a WIP as outstanding things include:

• rendering links with a better color than standard 90s blue - used theme["highlight.normal.bg"]
• pass font family to code blocks using theme from styled-components?
• Maybe? syntax highlighting separate pr
• Fix tooltip rendering flicker due to width change

@bryphe was hoping to get your feedback regarding the outstanding point especially syntax highlighting as I feel you likely would have had something in mind for this ?external lib like highlight.js or prism ? home grown highlighting to match themes

Secondly the tweaks I made re the dimensions of the tooltip, I think that as per #1237 an almost full width tooltip is subjectively not ideal, I tried containing this width to a max-width which it seems like vscode does (it seems to have a relative max-width of say 40%) but what I personally like about oni's hover bar is that it gives you a lot of the type info. I initially tried a slightly larger max-width than vscode possibly/maybe uses but this causes an issue with padding, so i've tried to preserve this by rendering any element that will fit the screen, adding wrapping to the quickinfo so text wraps vertically and also adding a width: min-content only for elements which are too big for the screen.

here are some screenshots:

[bryphe]

Cool, thanks for looking at this @Akin909 ! The hover experience could definitely use some love 😄

@bryphe was hoping to get your feedback regarding the outstanding point especially syntax highlighting as I feel you likely would have had something in mind for this ?external lib like highlight.js or prism ? home grown highlighting to match themes

Regarding the syntax highlighting - this is something I'm still trying to figure out. It would be interesting to see what strategy @TalAmuyal used for the markdown-preview plugin - I wonder if we could share/reuse pieces between this?

One challenge we have with the syntax highlighting is being able to read the highlight groups. Right now, we don't really have the information on the Oni side related to the colors for each of the highlight groups. It'd be really nice to have this! One option would be to explicitly set it in the themes (like a tokenColors property - we have editor.tokenColors today, but it points to a highlight group - I'd like for it to point to an actual color eventually!) - and if it isn't specified, we just wouldn't highlight.

It might also be possible to use our existing TextMate highlighting for code blocks - we can run a TextMate grammar on each line. The code where this happens is here:

oni/browser/src/Services/SyntaxHighlighting/SyntaxHighlightingPeriodicJob.ts

Line 87 in c01ae06

const tokenizeResult = this._grammar.tokenizeLine(line.line, previousStack)

, it could potentially be more streamlined in this case (this code is set up to do a few lines up to a budget, and then continue again later, so that async-job stuff causes a bit more complexity). The TextMate grammar gives us a list of tokens+scopes, which we could then match up via editor.tokenColors. But still need to figure out how to get actual colors there 😄

Ideally, with whatever strategy we choose - the highlighting colors can match our theme.

One thing I noticed too between our experience and VSCode is that we render the first line in a special way:

versus VSCode:

I'm wondering if we should do away with that special handling (box-shadow/different font/different formatting) of our 'title'?

Oh, and the width changes you made look good to me - thanks for digging in there and looking at options! 👍

[akinsho]

Having had a peek over a co-worker's shoulder at the vscode tooltip I also realised that for code blocks in paragraphs the backgrounds for those were being rendered differently so I've also employed the [WIP] darkening method to have the same really effect and increased line height as well it might be best to render these the way github does but this might not look great in all colorshemes as opposed to darkening an existing color in the palette.

[bryphe]

Re. the light themes do you mean choosing tool tip colors for each supported theme as those can be set to handpicked darker colors?
EDIT: also I concur that some of these might be due to quirks with colors especially solarized, re the lighter ones I was thinking of somehow finding out if vim bg is set to light or dark and changing the degree of darkening based on that.

We should have a way at least to specify handpicked colors for the hover info, to override the darkening behavior, since it might not work correctly in every case (or perhaps someone wants some exotic color for their scheme 😄 ). Using the strategy of seeing if the theme is light/dark would help, but there will still be situations where the user might still want to override it.

Perhaps we could use editor.hover.title.background, editor.hover.title.foreground, editor.hover.border, editor.hover.contents.background, and editor.hover.contents.foreground - and if these aren't specified, we'll fallback to deriving them by darkening existing colors.

Finally re the icon rendering I'm not able to reproduce it with the default font or with my slew of go to fonts can I ask what font you are using in the screen shot, my only change was to force the children of the tooltip to use the editor's font family in css but I can just make that specific to the code blocks which is where I specifically wanted the change

Hmm, I'll grab the latest changes and see if I can reproduce it again.

[bryphe]

Here's what i saw in the devtools:

Seems like somehow the icon is being overrided with the Consolas font.

[akinsho]

Ah thanks for the info and the screen grab seems that definitely relates to forcing the font-family down through css which I did to bypass having to use prop drilling to pass it down from the container that has this state, I can revert that and apply that change only for code blocks.

I'll add the config options as you suggest as well as darkening based on vim's background setting

[akinsho]

@bryphe I've added your recommend config options which if not set default to the darkened tooltip options.

I haven't implemented the vim background based alteration as I realised I can determine the lightness or darkness of the original color just using the color package which is what I've done to moderate the darkening.

I also reverted the font-family change to only affect the code blocks which leaves the icon elements untouched.

[bryphe]

Changes looks good to me, @Akin909 ! I'm going to re-run the Linux job, as that should be passing. I'll bring it in after that runs.

Thanks for your work on this! 👍

[akinsho]

@bryphe nice! ✨, I can add a section to the docs about the changes that users can now configure like hover.contents.background also did the same for the code block which show in the markdown in case ppl want that sort of granular control. Also didn't quite want to raise it as a separate issue though maybe it should be but this change raises the lol issue of how to handle opening links in oni.

I imagine your intent would be to use the internal layer-browser but just wanted to mention it though I know that's still a work in progress.

[bryphe]

I can add a section to the docs about the changes that users can now configure like hover.contents.background also did the same for the code block which show in the markdown in case ppl want that sort of granular control

That would be awesome, thanks!

Also didn't quite want to raise it as a separate issue though maybe it should be but this change raises the lol issue of how to handle opening links in oni.

I imagine your intent would be to use the internal layer-browser but just wanted to mention it though I know that's still a work in progress.

Yep, eventually 😄 But for now I'd like to funnel them through the browser.openUrl command - this will do the right thing - either open in a new window, or an internal browser window, depending on if the experimental.browser.enabled flag is set.

[bryphe]

Looks like the linux build failed, but due to the code coverage changes - I'm pulling those out in #1338 until we have a sturdier foundation for builds. I'll bring this in now. Thanks again for your work on this, @Akin909 ! 💯