Bug 492187

Summary:	LSP popup box has very poor contrast with surrounding code and general styling issues
Product:	[Applications] kate	Reporter:	Adam Fontenot <adam.m.fontenot+kde>
Component:	general	Assignee:	KWrite Developers <kwrite-bugs-null>
Status:	RESOLVED FIXED
Severity:	normal	CC:	waqar.17a
Priority:	NOR
Version First Reported In:	Git
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:	https://invent.kde.org/utilities/kate/-/commit/27bc98986306f55a4076d21f4d25852dd05a1687	Version Fixed/Implemented In:
Sentry Crash Report:
Attachments:	screenshot of various issues with popup window appearance screenshot showing appearance of lsp plugin widget as of d84009c4

Description Adam Fontenot 2024-08-25 23:53:36 UTC

Created attachment 172953 [details]
screenshot of various issues with popup window appearance

SUMMARY

There are some overlapping issues with the style of the LSP popup box. Style is subject to opinion, of course, but I'm going to try to highlight some objective issues.

I'm happy to split these issues into additional reports as necessary / as requested, but I think the most likely resolution is going to happen in a single merge request where someone takes the time to improve the widget styling.

1. It's difficult to visually distinguish the popup box from the surrounding code editor widget. This seems true regardless of what color scheme you're using in Kate.

2. Some of the text in the box is very large, drawing attention where it doesn't seem to be warranted. See attached screenshot for example.

3. Popup boxes that contain a lot of information are a bit difficult to read and "cramped" feeling.

4. It's difficult to visually tell what is a link and what isn't. It's impossible to distinguish some code elements unless they are syntax highlighted.


POSSIBLE RESOLUTIONS

1. Instead of using the editor background color from the user's theme as the background color for the popup, use a different color from the same theme (if it's possible to choose one appropriate for popups - this isn't clear to me). Alternatively, create contrast with the editor background by brightening the theme background color if it's below a certain value, and darkening it if it's above that value.

Also, increasing the padding on the internal document slightly would help create more separation.

2. I think the text styling issues come primarily from naively asking Qt to render Markdown - in the screenshot, Qt seems to be rendering a `#` header in the Markdown source text the way it would render an `h1` HTML element - that is, with really large text. But this rendering doesn't accurately convey the semantic meaning of a header in documentation. It's not usually used to outline a huge document with a hierarchy, e.g. h1, h2, h3. It's more often used to break documentation into a few sections by category (with no hierarchy). For this reason, smaller text - perhaps bold, perhaps underlined - would be more appropriate and not feel as "shouty". 

In addition, the huge heading shown in the screenshot seems to justify additional padding following the heading, but it's uncomfortably close to the succeeding line of text.

3. Increasing the document padding (as in 1, above) would help here. A bit more space between paragraph elements would be useful. There's also a feeling of cramping caused when the box is too small a proportion of the window to show all the contents without horizontal scrolling, as in Bug 492090. I'd suggest setting the maximum size to 50% of the text area width.

The popup box doesn't appear to respect the editor-wide "line height multiplier" setting.

Additionally, I think it would be great to at least have the *option* to use a different text size for popup boxes. Since these are more ephemeral, I think a good default would be to use a size 1 pt smaller than is used for document body text. But giving users more options on this would be good. Perhaps it would be possible to even allow setting a CSS file, similar to how the LSP plugin window accepts (and validates) JSON settings for the language servers. I'm not sure how plausible implementing that is.

4. Primarily this concerns code objects that appear in body text, with `backticks` in the Markdown source. Because all text rendered in the LSP plugin popup uses the user's default font (presumably monospace), code presented in backticks has no visual distinction. Perhaps setting a background color for any code element in the popup would help distinguish these from the surrounding text. If this were done, it would be more clear that bits of code with color are intended to be clickable (*is* that the intent? I'm genuinely not sure).

Alternatively, a sans-serif font could be used for non-code in the popup?

Alternatively, anything clickable could be underlined. As things stand now, clickable links aren't even underlined on hover.


SYSTEM INFORMATION
Operating System: Arch Linux 
KDE Plasma Version: 6.1.4
KDE Frameworks Version: 6.5.0
Qt Version: 6.7.2
Kernel Version: 6.10.5-arch1-1 (64-bit)
Graphics Platform: Wayland

Comment 1 Waqar Ahmed 2024-09-26 07:20:21 UTC

Git commit cff43ef0e47fc8f632d2697a171ca5608c3ed0ec by Waqar Ahmed.
Committed on 26/09/2024 at 07:20.
Pushed by waqar into branch 'master'.

tooltip: Underline the links and reduce heading height

M  +18   -2    apps/lib/texthint/tooltip.cpp

https://invent.kde.org/utilities/kate/-/commit/cff43ef0e47fc8f632d2697a171ca5608c3ed0ec

Comment 2 Waqar Ahmed 2024-09-26 07:41:32 UTC

Git commit fd7f81fcd1261fe87fbed02e33236f1da9322c18 by Waqar Ahmed.
Committed on 26/09/2024 at 07:41.
Pushed by waqar into branch 'master'.

tooltip: Fix frame color in darkmode

The frame color is barely visible in darkmode and too vibrant in
lightmode. Use Separator color from the theme for the frame instead.
Makes it much easier to distinguish the tooltip from the editor.

M  +6    -1    apps/lib/texthint/tooltip.cpp

https://invent.kde.org/utilities/kate/-/commit/fd7f81fcd1261fe87fbed02e33236f1da9322c18

Comment 3 Waqar Ahmed 2024-09-26 08:00:14 UTC

Git commit 38e6e9c8f6d732fd7fec5c23a97af377548d3db4 by Waqar Ahmed.
Committed on 26/09/2024 at 07:59.
Pushed by waqar into branch 'master'.

tooltip: Highlight the background of inline codespans

Makes them look a bit different from the surrounding text

M  +12   -0    apps/lib/texthint/tooltip.cpp

https://invent.kde.org/utilities/kate/-/commit/38e6e9c8f6d732fd7fec5c23a97af377548d3db4

Comment 4 Waqar Ahmed 2024-09-26 08:19:12 UTC

Git commit d84009c49bd1f3b153235dcd821c84e845da583b by Waqar Ahmed.
Committed on 26/09/2024 at 08:18.
Pushed by waqar into branch 'master'.

tooltip: Use 1 pt smaller font than editor

But dont go below 11 pt so that we don't end up being too small

M  +5    -1    apps/lib/texthint/tooltip.cpp

https://invent.kde.org/utilities/kate/-/commit/d84009c49bd1f3b153235dcd821c84e845da583b

Comment 5 Waqar Ahmed 2024-09-26 08:21:23 UTC

The linked commits should improve the situation a bit.

I have tried to address most of the points. Creating new bugs out of these points would be a time waste so I addressed as much as could be easily done. There might still be issues but hopefully fewer than before which we can look at individually. You can reopen if you feel we can tune a bit more here or the points are not really addressed.

Comment 6 Adam Fontenot 2024-09-26 15:03:07 UTC

Created attachment 174110 [details]
screenshot showing appearance of lsp plugin widget as of d84009c4

Thanks for working on this, it looks a lot better to me!

I'm reopening because I think one specific thing isn't really addressed, which is the insufficient contrast of the widget border. Maybe this is just a quirk of the theme I'm using, but I think it might actually be worse after the changes? See attached screenshot.

Probably important to note that I'm not using my global color scheme in Kate. I use a global light theme, but I use Catppuccin Frappe Flamingo as my Kate window color scheme, and Catppuccin Macchiato as the editor color theme (slightly darker alternative, for more contrast). These themes can be found here: https://github.com/catppuccin/kde

I mention that just in case the separator color is being taken from e.g. the global light theme, and is inappropriate for the much darker editor color theme.

-----

I also think it would be very nice to increase the spacing around header "#" elements slightly if possible, and I'd also consider increasing the padding on the widget itself so that the text in the widget is given a bit more distance from surrounding code.

I still like the idea of having the widget background be a slightly different color than the code editor background, but maybe that's just me. I wouldn't have reopened this for nitpicky subjective stuff like this of course. I probably shouldn't be the sole source of input on the LSP widget design, just because I happen to be the only person filing bug reports about it. :-)

Comment 7 Waqar Ahmed 2024-09-26 16:53:36 UTC

The separator color should be coming from the "editor color theme". Separator color is used for the line between LineNumber area and editor area. But it seems like its dark in your case so it doesn't really work. I will try to use a variant foreground color of instead, seems to work much better.

> I still like the idea of having the widget background be a slightly different color than the code editor background, but maybe that's just me. I wouldn't have reopened this for nitpicky subjective stuff like this of course. I probably shouldn't be the sole source of input on the LSP widget design, just because I happen to be the only person filing bug reports about it. :-)

That is not a bad idea. The problem here is finding the "right" color which works for all themes. That part is not easy. VSCode does this by having dedicated colors for things like this, so if the color is bad, blame it on the theme. I tried to use lightened/darkened background color variants but didn't get too far (but I also didn't try very hard tbh). See my new commit (that will land soon). If you want to help with something like that, it will give you a good idea. We also have contrast helpers in KGuiAddons library that can probably be utilized for finding a good enough match.

----

For the extra padding or margins around headers, its doable if I look at the api. One needs to iterate over all the blocks in the document and find the ones which have a headingLevel() and then change their blockformat to have a block margin. I would encourage you to make an MR, can help finding the right APIs if you want. :)

Increasing padding in general should also be doable via setDocumentMargins I think.

Comment 8 Waqar Ahmed 2024-09-26 16:54:20 UTC

Git commit 27bc98986306f55a4076d21f4d25852dd05a1687 by Waqar Ahmed.
Committed on 26/09/2024 at 16:54.
Pushed by waqar into branch 'master'.

tooltip: Use a variant of foreground color for tooltip border

It will have better contrast.

M  +1    -1    apps/lib/texthint/tooltip.cpp

https://invent.kde.org/utilities/kate/-/commit/27bc98986306f55a4076d21f4d25852dd05a1687