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

Fix blockquote color contrasts #1786

Merged
merged 4 commits into from
Apr 29, 2024

Conversation

gabalafou
Copy link
Collaborator

@gabalafou gabalafou commented Apr 25, 2024

Accessibility fix, see issue #1428

To fix the link contrast contrast violation, I just reused the --pst-color-inline-code-links variable.

I also realized that the contrast between the blockquote background (--pst-color-surface) and the site background color (--pst-color-background) was very low. It would be very hard to increase the contrast between these two colors without creating a cascade of other contrast failures. However, blockquotes also have a left border ("notch"), and so I decided to increase the contrast of the notch with both backgrounds—both the site and the blockquote—so that its ratio with both is above 3.0. To be honest, I'm not entirely sure that we strictly have to meet this contrast threshold in order to satisfy WCAG 1.4.11 Non-text Contrast, but it seemed like a good idea nonetheless so that blockquotes on the site are perceivable by low-vision users.

Here are links to the color contrast grids for the blockquote notch:

Screenshots

Before After
blockquote-light-contrast-fail blockquote-light-contrast-pass
blockquote-dark-contrast-fail blockquote-dark-contrast-pass

@gabalafou gabalafou requested a review from trallard April 25, 2024 10:08
@gabalafou
Copy link
Collaborator Author

Thanks for your review @drammock! I would like to give @trallard a chance to review this, since she has the best idea of the vision for the overall color scheme. She is at a conference this week, so we probably can't get this PR merged until next week.

@gabalafou
Copy link
Collaborator Author

Do I need to update the "Theme variables and CSS" page or any other in the docs?

@gabalafou gabalafou added the tag: accessibility Issues related to accessibility issues or efforts label Apr 25, 2024
@drammock
Copy link
Collaborator

Thanks for your review @drammock!

Was more of a drive by comment than a review :)

@drammock
Copy link
Collaborator

Do I need to update the "Theme variables and CSS" page or any other in the docs?

I don't think so. From that page we link out to the actual CSS file where colors are defined

Copy link
Collaborator

@trallard trallard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gabalafou

I'm not entirely sure that we strictly have to meet this contrast threshold

Technically we do not, but having good contrast throughout is something good to aim for.

@@ -23,6 +23,11 @@ blockquote {
@include legacy-backdrop-placeholder;
background-color: var(--pst-color-surface);

// Ensure there is enough contrast against the background
a {
color: var(--pst-color-inline-code-links);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice I had not realised we had contrast issues here, so thanks for fixing this.

@trallard trallard merged commit 30be4d4 into pydata:main Apr 29, 2024
19 checks passed
@gabalafou gabalafou deleted the fix-blockquote-color-constrasts branch April 30, 2024 10:01
ivanov pushed a commit to ivanov/pydata-sphinx-theme that referenced this pull request Jun 5, 2024
* Fix blockquote color contrasts

* Increase contrast of blockquote notch in dark mode

* Update comment in src/pydata_sphinx_theme/assets/styles/variables/_color.scss

* Update comment in src/pydata_sphinx_theme/assets/styles/variables/_color.scss
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag: accessibility Issues related to accessibility issues or efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants