gabalafou commented 9 months ago

Closes #30.

This PR updates the a11y-light and a11y-high-contrast-light themes.

For a11y-light, the main changes are:

Change highlight color from #e6e5d8 (this is the same as #7971292e blended with #fefefe) to #fdf2e2
Decrease the value in the HSV value of two colors in the palette to ensure that all of the colors meet 4.5 against #f2f2f2 (I chose this color as the darkest off-white or gray background color that the theme will support)
The end result is summarized in a contrast grid

For a11y-high-contrast-light:

Use colors from perceptually uniform color
Change highlight color from #e6e5d8 to #fae4c2
High contrast color grid

github-actions[bot] commented 9 months ago

PR Preview Action v1.2.0 :---: :rocket: Deployed preview to https://Quansight-Labs.github.io/accessible-pygments/pr-preview/pr-33/ on branch gh-pages at 2024-03-28 17:01 UTC

gabalafou commented 9 months ago

I have some caveats with my work so far on this PR:

I'm not sure what exactly the "high contrast" in this theme is supposed to mean. The contrast ratios in the WCAG table for high contrast are almost the same as in the table for a11y-light.
I'm not sure what constraints went into creating a11y-high-contrast. For example, was there a constraint that the value for orange must be equal to the value for yellow?
Even though this PR fixes the contrast ratio for the text on an off-white background and for the text against a line highlight, there's still a contrast failure: between the background color of the code block (#fefefe on the demo site) and the background color of a highlighted line (#e6e4d9). WCAG guidelines require that the contrast ratio between those two to be 3:1, but they're actually 1.3:1. I suspect that it's not possible (or very hard) to find a color palette with 8 different text colors that each have to contrast at least 4.5 to two different background colors that have to contrast 3:1 to each other. Eric Bailey's solution was to put a top and bottom border on highlighted lines. We could do the same thing but we would have to modify the CSS that we get back from Pygments, which we already do in the generate_css() function in utils.py.

gabalafou commented 9 months ago

@trallard, I still have some misgivings about this PR.

Is it okay for us to so drastically change these themes now that we've published this repo?

trallard commented 9 months ago

I am not sure we are drastically changing the themes. I mean - hues are being preserved as much as possible and tweaks are being made to improve the accessibility so I would not be too worried about this.

The only question I have is why the yellow in the light theme? I mean this is a much brighter one that the one we currently have

gabalafou commented 9 months ago

Okay, I think this is ready for review

gabalafou commented 9 months ago

Note to self. I want to create some issues for this repo based on my experience of updating these two themes:

[x] Updating a theme is cumbersome and error-prone (too many manual steps, no clear guide)
[x] a11y-light has 7 foreground colors whereas a11y-high-contrast-light has 8
[x] Line highlight color against background color does not meet WCAG (3:1)
[x] Contributing instructions are incomplete (missing Hatch)

trallard commented 9 months ago

Looking at the proposed light and high_contrast_light themes above my first impression was that now the colours in both looked different 🤔 so I attempted to make these two closer looking (using the perceptually uniform colour palettes we have)

a11y-light - see the corresponding contrast grid

EightShapes_Contrast_Grid

In the grid, you can see the current colours (with names) vs. the proposed ones (no names). All the colours meet AA against the reference backgrounds. I also added a magenta to match the high-contrast theme.

⚠️ You will also notice that I changed the black and grey colours. Since the slat-gray in the high-contrast theme is the same as we are using in PST I thought we could use an alternative black (and shades) to keep this closer to the original theme's hue (and not directly use the PST theme colours)

See a11y-high-contrast-light and the corresponding contrast grid

EightShapes_Contrast_Grid

With these proposed changes, both themes are more consistent and we keep a higher contrast in the a11y-high-contrast-light theme vs the light one.

I can make the changes unless you are dying to make them @gabalafou or you have major reservations on the proposed changes/updates

gabalafou commented 8 months ago

Putting this on hold while we get #42 through

trallard commented 7 months ago

I was going to review again until I saw the last comment 😓 ping me when this is ready then

Carreau commented 7 months ago

42 is in, this got a bunch of conflicts now. I think most of them should be solvable by just re-generating the files.

gabalafou commented 7 months ago

I resolved some merge conflicts, but this is not yet ready for review

gabalafou commented 7 months ago

OK this is now ready for review

gabalafou commented 7 months ago

@Carreau thanks for approving this!

I think that @trallard should give this one final look and approval before I merge it in.

Carreau commented 7 months ago

Let's wait 48 hours and merge otherwise to have one less things on @trallard todo list, we can revise later if we wish.

Carreau commented 7 months ago

Seen with @trallard in person, merging.

Quansight-Labs / accessible-pygments

Fix contrast failures for high contrast light theme #33

42 is in, this got a bunch of conflicts now. I think most of them should be solvable by just re-generating the files.