Closed thomcc closed 3 years ago
Great addition, merging this since it greatly improves text visibility while using the background
marker type. Maybe in the future we will replace it with a library. I tried the Color.isDark()
function but it still is not as reliable as this implementation.
In particular, the issue that drove me to fix this is below.
Before:
After:
(Ah, much better)
Anyway, the previous computation was wrong in a few ways, although most of them are due to the spec it's derived from (the old W3C AERT spec) being vague (in particular, it leaves out the necessary gamma-correction step).
Anyway, there's a newer spec that is extremely explicit about the color spaces (and thus, the gamma-correction operation needed), and it's also better for color-blind users (admittedly... there may not be many of them using this addon) so I used some code for it that I had lying around1.
Anyway, I just used that with updated comments, to hopefully make it clear what the parts are doing, and etc. If you'd rather not have the code in question in your project, I can publish it as a package instead, or look harder for something correct.
Closes #111.
P.S. To actually test this out I had to fix another couple issues, which will have an incoming PR, (separated, since they might have been due to my own confusion).
1: Bugs in this sort of code are a personal peeve of mine, and in the past I failed to find a good/efficient implementation of the contrast computation on NPM, so I wrote that one. I never published it to NPM, mostly since I'm lazy and don't write JS regularly anymore. I was going to do it now, although publishing a package and immediately PRing someone to use it seems sketchy.