Skip to content
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.

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

Add more contrast for document_highlight() #39

Closed
strash opened this issue Jun 5, 2024 · 13 comments · Fixed by #45
Closed

Add more contrast for document_highlight() #39

strash opened this issue Jun 5, 2024 · 13 comments · Fixed by #45

Comments

@strash
Copy link

strash commented Jun 5, 2024

On the light scheme the highlight is fine but on the dark scheme it is almost invisible.
It would be nice to add some contrast, just enough to be distinguishable but not annoying.

Frame 2242 Screenshot 2024-06-05 at 23 08 15 Screenshot 2024-06-05 at 23 09 41
@windowsrefund
Copy link
Contributor

If anything, that should be user configurable as I personally see this theme's lower contrast as being a really good thing.

@ramojus
Copy link
Owner

ramojus commented Jun 7, 2024

I think these could be a bit lighter on dark bg. On light bg they do seem to be more visible.
I may think about configuration here, but I am currently not convinced that it's worthwhile. Let's see when this change is applied.

I am currently refactoring and changing how shades are applied everywhere, so, as part of that, I will try to make these a bit lighter while keeping the low contrast vibe.

@antoineco
Copy link
Contributor

One of the difficulties here is to not overlap with the Search highlight group.

In the screenshot below, dict is highlighted as the result of a current search (the background is slightly lighter), whereas msg is highlighted by vim.lsp.buf.document_highlight():

image

Using a colorful background for searches would solve that and "free" bg4 for the LspReferenceText highlight group. Here is how Zenbones does it, for comparison:

image

@antoineco
Copy link
Contributor

antoineco commented Jun 7, 2024

By the way, out of interest I compared the APCA Lc1 value for LspReferenceText in both dark and light modes. @strash is right, the contrast is objectively lower in dark mode (9 vs. 0).

Here are the values:

  • bg_contrast = 'soft'
  • bg_contrast = 'medium'
  • bg_contrast = 'hard'

(edit: changed URLs to the non-productive "SAPC" version of the tool, which is more precise in very low contrast ranges.)

Similarly, the contrast for Search is 12.9 vs. 0.

APCA is the algorithm used by W3C in the draft version of the WCAG accessibility guidelines. It is a very useful tool for designing colorschemes and ensuring that perceived contrasts are proportional between dark and light modes.

Footnotes

  1. Lightness contrast

@ramojus
Copy link
Owner

ramojus commented Jun 8, 2024

Interesting. My eyes don't agree with this algorithm though 😀 Like if I try to make dark bg contrast match the contrast of light bg, it doesn't look like the same contrast at all...

image
image

I guess it also depends a lot on the environment and the display, e.g. I get much higher contrast on my laptop screen than on my monitor. And normally you would (at least I would) use light bg in bright environment compared to dark bg in darker environment. So even if light bg objectively does have more contrast, it would make sense when considering the practicality of described scenario.

@ramojus
Copy link
Owner

ramojus commented Jun 8, 2024

@strash would you say this is enough of a difference? I'm hesitant to add more contrast, as it will probably start being distracting. I would say it's still lower contrast than with light bg, but as I said before, dark bg is more targeted towards not-that-bright environments.

@antoineco
Copy link
Contributor

antoineco commented Jun 8, 2024

My eyes don't agree with this algorithm [...]
even if light bg objectively does have more contrast, it would make sense when considering the practicality of described scenario.

That's fair, and to be clear I don't disagree with the existing highlights, and use primarily dark mode.
Keep in mind that although the algorithm takes into accounts the different needs of light and dark modes, it is geared towards accessibility use cases, primarily for text-on-background ratios (as opposed to background-on-background).

@strash
Copy link
Author

strash commented Jun 9, 2024

@strash would you say this is enough of a difference? I'm hesitant to add more contrast, as it will probably start being distracting. I would say it's still lower contrast than with light bg, but as I said before, dark bg is more targeted towards not-that-bright environments.

@ramojus It's hard to say. I see the difference between "before" and "after" but I also see a big difference between your screenshots and mine, so I'm a bit confused. But overall it's better now.

Group 2242

@ramojus
Copy link
Owner

ramojus commented Jun 10, 2024

@strash Probably not what you meant, but I'll say this just in case - I wasn't trying to match your suggestion (on your second screenshot) exactly. Instead, I was aiming for a more subtle change.

I think the difference between my before screenshot and your unmodified screenshot is that the font you are using has thicker strokes compared to the font that I'm using. That makes all of the colors appear brighter (as they have more area to fully show themselves, without antialiasing "weakening" them), and thus the contrast of bg layers is harder to see. I've checked with a color picker, and LspReferenceText bg is the same color in both screenshots.

I think different fonts affecting the perceived contrast of bg layers may be enough of a reason for a config option. I'll think about it.

@strash
Copy link
Author

strash commented Jun 10, 2024

Indeed, it's very subtle change (0 0 16 -> 0 0 17 HSB 😀), but I think I see it better now. Thank you!

@strash
Copy link
Author

strash commented Jun 11, 2024

@ramojus should I close the issue?

@ramojus
Copy link
Owner

ramojus commented Jun 11, 2024

Better to keep it open for now. I'll add configuration for bg layers contrast and that's a bit related.

@ramojus
Copy link
Owner

ramojus commented Jun 24, 2024

@strash I've added the ability to override shades in #45, but it's a breaking change, so it was merged to v1 branch. After longer usage, I ended not liking the changes in #40, so I will revert them. This is how you can replicate them (from v1 branch):

require 'mellifluous'.setup({
    color_overrides = {
        dark = {
            colors = function(colors)
                return {
                    bg2 = colors.bg:lightened(4),
                    bg3 = colors.bg:lightened(7),
                    bg4 = colors.bg:lightened(10),
                    bg5 = colors.bg:lightened(13),
                }
            end,
        }
    },
})

LspReferenceText is linked to bg3 -- you can change it if you want even more contrast.

After a couple of weeks, v1 branch should be merged to main (when v1.0.0 will be released).

@ramojus ramojus closed this as completed Jun 24, 2024
ramojus added a commit that referenced this issue Jul 4, 2024
Related to #10 and #39. The Search highlight group was a bit difficult
to distinguish and had no similarity to IncSearch highlight group.
Also added CurSearch group, but decided to not make purposeful use of
it, as it just seemed more distracting than actually useful.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants