Consistent focus ring (first pass)

[gabalafou]

Note: this PR is against feature-focus branch not main. Style fixes related to interaction state (focus, hover, current) will be collected in small increments into feature-focus before being merged into main.

This PR is a first pass at making the focus rings across the theme consistent. That means that they are all solid and 3px thick (at 100% zoom). In light mode, all focus rings should be pink-500 (#c132af), and in dark mode, pink-400 (#e47fd7). (Exception: skip link, which uses a black focus ring.)

Known TODOs (for future PRs):

• ipyleaflet receives focus but does not show focus ring
• Sidebar TOC dropdown "chevron" button (does not receive focus and does not work with space or enter key)
• There are too many copy buttons in notebook outputs
• Focus ring for tabs not yet implemented
• Some weird artifact for API "source" links, where a second blank focus ring appears at the end of the API method name:
  
• Toggle (hamburger) button for left/primary mobile sidebar overlay (does not receive focus and does not work with space key or enter key). Open question: does it need to work with keyboard?
• Toggle (hamburger) button for right/secondary mobile sidebar overlay (receives focus, has focus ring, but does not respond to space or enter key). Open question: does it need to work with keyboard?
• Button focus rings need to be changed to match the color of the button

[gabalafou]

Done self-reviewing! One nice thing about the code: all focus ring-related styles can be found by searching the codebase for "focus-visible"

[gabalafou]

Bootstrap uses box-shadow for focus rings, so I changed the code here to use border-left instead of using a box shadow to draw a vertical line or notch on the left side of the link in the TOC that corresponds to the current page that the user is on.

[jorisvandenbossche]

I think this caused a small glitch, because the border has an actual width compared to the box-shadow?

Comparing the readthedocs preview of this PR vs the latest docs:

https://pydata-sphinx-theme.readthedocs.io/en/latest/user_guide/layout.html#templates-and-components:

And on the PR (https://pydata-sphinx-theme--1549.org.readthedocs.build/en/1549/user_guide/layout.html#templates-and-components):

[drammock]

ooh, good catch @jorisvandenbossche

[gabalafou]

Yes, thanks for catching that! Follow up PR: #1561

[gabalafou]

The new :focus-visible rule in the base stylesheet now provides this focus ring.

[gabalafou]

This is needed to override a rule I added in _header.scss that slightly rounds the focus rings for the links in the header, but the search button we want the focus ring to "hug" its border, so the border-radius must match.

[gabalafou]

In several places where we want the focus ring to be rounded at the corners, I use this rule. I only apply the border-radius rule on :focus-visible because if I apply it even when the element doesn't have focus, it will change the way borders appear, which will likely mess up other styles (for example, the vertical notch when the link corresponds to the current page that the user is on).

[gabalafou]

We want all of the focusable links and buttons in the header nav to have rounded corners, so I decided to use this blanket rule, but this means I have to override the border-radius for the search button.

I thought about writing this rule as:

:focus-visible:not(.search-button-field) {
  border-radius: 0.125rem;
}

But I noticed that this file has no other reference to anything search related. It's completely ignorant of the fact that it contains a search button, so I decided that it would be better to add an override rule in the search stylesheet rather than couple this stylesheet to the search button. Just to keep things separate.

[gabalafou]

The default focus ring for the links at the top of the mobile sidebar (under "Site Navigation") was hugging the edges too closely, so this rule just adds a gap, or offset.

[gabalafou]

I couldn't figure out why this rule was here but it was clipping the focus ring, so I removed it. I asked @12rambau if he could remember why he added it and if I could remove it, and he replied, "I guess the only way to find out is to remove it 😄"

[gabalafou]

I re-factored the skip link markup because if we use a single HTML tag then we have a focus ring that spans the entire width of the page, rather than just the link text.

[gabalafou]

This is how to customize the focus ring styles that Bootstrap provides.

[gabalafou]

This is ready for review. @drammock, do you know who I should ask to review CSS changes? (Tania is still out on vacation.)

[gabalafou]

This now gets a rounded focus ring so I decided to remove this border-radius rule.

[12rambau]

Thanks for the nice PR, from my side I don't have any specific comments on the technical choices but I'm not competent enough in a11y to be relevant evaluating this PR, Nonetheless you removed some css that were controlling the search-button display when used instead of search-field. Could you reintroduce them as this button is a lot used by people with real estate issues in the header navbar.

[gabalafou]

For both the search button and the theme switcher button, I decided to go ahead and make them consistent with the other icons at the end of the nav bar.

[gabalafou]

Cross-reference

• Proposed Design Update for Focus State - Links #1397
• FOCUS - PyData Sphinx enhancements and documentation Quansight-Labs/czi-scientific-python-mgmt#27
• Fix: focus visible in PyData theme Quansight-Labs/czi-scientific-python-mgmt#37

[drammock]

one request: can we fix the homepage card focus problem in this PR? It's come up before (can't find the issue/PR at the moment) that the last 2 cards the whole card is a link, which does weird things with the focus highlighting (even on this PR, it's still inconsistent: you get a bottom border on the card, not a ring around it).

I think the easiest fix is to:

• for the "supports pydata and jupyter" card: remove the whole-card link, and link the text "jupyter extensions" to examples/execution.html and the text "pydata execution outputs" to examples/pydata.html
• for the "example gallery" card, remove the whole-card link and link the text "gallery of projects" to the same url.

OK if you want to do in a separate PR I guess, but since this PR title is "consistent focus ring" I thought "well, let's fix the last nagging inconsistency"

[gabalafou]

@drammock I'm happy to take a look at fixing the cards on the home page, but if it's more than half an hour, I would prefer to do it in a follow-up PR. Partly because it's really not the last inconsistency. If you check the PR description, you'll find a laundry list of inconsistencies 😅

[gabalafou]

Okay, I fixed the focus ring for the SD link cards. My last commit made two changes:

1. Brought the links inside the SD cards on the homepage (see inline comments)
2. Add CSS rules to move suppress the focus ring from appearing on the heightless A-tag in favor of applying them to the A-tag's ::after pseudo-element.

These changes can be checked at the following preview build URLs:

• https://pydata-sphinx-theme--1549.org.readthedocs.build/en/1549/index.html
• https://pydata-sphinx-theme--1549.org.readthedocs.build/en/1549/examples/gallery.html

[gabalafou]

The build step was auto-linking idtracker.ai, outputting two anchor tags instead of one.

[gabalafou]

Since there is another link on the home page to the same URL, it's considered good practice for accessibility to make both of them also have the same name: "Sphinx Design components."

[gabalafou]

The HTML template for cards with links decouples the link text (which should be the card content) from the link element, causing them to appear to assistive tech as their hrefs, in this case "examples/pydata.html" and "examples/gallery.html."

I felt it would be better to bring the links into the cards for two reasons:

1. Seeing/hearing "Pydata execution outputs" and "our gallery" is arguably nicer than hearing the hrefs
2. It makes these two cards consistent with the other 4 cards, which do not have link fields. I always felt it was a little weird that the last two cards had hover and focus states while the others did not.

[gabalafou]

Snuck in a hover color correction with this commit.

[gabalafou]

These focus ring CSS rules that I had put under .sd-card more properly belong in this section of the CSS file.

[drammock]

thanks @gabalafou !

[jorisvandenbossche]

Something I noticed that changed here in the preview compared to main:

Latest main:

Preview from this PR:

So the un-collapse button for subsections moved a bit to the left, it's not as close to the vertical line anymore.

I don't really know if it is necessarily a bad change, but I assume it was probably not intentional?

[gabalafou]

@jorisvandenbossche thanks for your careful eye! I've created a fix: #1562