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

Harmonize and accessibilize the focus, hover and other styles across the theme #1402

Closed
wants to merge 1 commit into from

Conversation

gabalafou
Copy link
Collaborator

@gabalafou gabalafou commented Aug 8, 2023

This PR went through a few different phases, but ultimately it is superseded by work on the feature-focus branch

@gabalafou
Copy link
Collaborator Author

gabalafou commented Aug 8, 2023

This is a first pass so that people can take a look at the results sooner rather than later.

There are a lot of rough edges that need to be cleaned up, such as:

  • skip link (keep background color of the full-page-width banner, only style the centered link)
  • right side of focus-visible outline chopped off in left TOC
  • not good on (dark) image links (because no outline offset)
  • not good on many other kinds of links (such as the ipyleaflet map zoom in/out buttons)

@gabalafou
Copy link
Collaborator Author

Styling looks pretty consistent across browsers I tested on Mac OS: Chrome, Safari, Firefox. Exception: outline does not surround image link on Safari and Firefox.

@drammock
Copy link
Collaborator

drammock commented Aug 8, 2023

Thanks for tackling this @gabalafou. Link to CI build: https://pydata-sphinx-theme--1402.org.readthedocs.build/en/1402/

tabbing through the homepage (firefox on linux) I see:

  1. the skip-to-content link is nearly invisible / unreadable (not sure if that's the same problem you were seeing with the skip link?)

focus2

  1. consistency issues, discussed after the image:

focus1

  • search box gets accent color, not primary color (should be consistent, though I like the pink!)
  • announcement banner link looks odd with changed background color (might be worth overriding the focus BG color for the banner)
  • version warning button looks odd (color clash; not sure how to solve)
  • a couple of the tiles are focusable when they shouldn't be (separate problem -> could be addressed in separate PR). Oddly, they appear with just a thick underline.

Seeing the search box remindes me that it's currently the only place we use our "accent" color, and makes me realize that we didn't even consider using the accent color as focus indicator in #1397. Was that an intentional omission @smeragoel?

@trallard trallard self-requested a review August 9, 2023 11:37
@trallard trallard added the tag: accessibility Issues related to accessibility issues or efforts label Aug 9, 2023
@smeragoel
Copy link
Contributor

That's a great suggestion about using the accent colour, I hadn't considered it before but it seems like a very viable idea. The colour clash and inconsistent styling will be addressed soon. This styling was just intended for links, and I am working on creating the focus styles for other interactive elements - buttons, cards, dropdowns etc. I'm going to consider both our primary and accent colour for focus states for now, and make a decision based on what works better for all elements.

Thanks again for the suggestion @drammock!

@gabalafou gabalafou marked this pull request as draft August 14, 2023 09:04
@gabalafou
Copy link
Collaborator Author

Great feedback yall! This is exactly what I was hoping to generate by putting up some code quickly so people could play with the results. While we re-think the design, I've put this PR in draft mode.

@gabalafou gabalafou changed the title More accessible link focus style Harmonize and accessibilize the focus, hover and other styles across the theme Oct 27, 2023
@gabalafou gabalafou closed this Oct 27, 2023
@gabalafou gabalafou force-pushed the link-focus-visible-style branch from 9ef331a to e86e0ab Compare October 27, 2023 12:50
@gabalafou gabalafou reopened this Oct 27, 2023
@gabalafou gabalafou closed this Oct 30, 2023
@gabalafou gabalafou deleted the link-focus-visible-style branch June 11, 2024 06:38
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.

4 participants