Skip to content

Fix: Add keyboard and screen reader accessible submenu navigation#309

Closed
IsabelBirds wants to merge 3 commits into
executablebooks:masterfrom
IsabelBirds:accessible-navigation
Closed

Fix: Add keyboard and screen reader accessible submenu navigation#309
IsabelBirds wants to merge 3 commits into
executablebooks:masterfrom
IsabelBirds:accessible-navigation

Conversation

@IsabelBirds

Copy link
Copy Markdown
Contributor

The current chevron icon solution does not allow keyboard or screen reader users to open their respective menus.

This change surrounds existing icons with button tags and the necessary aria values to explain to screen readers user's what they do.

The sighted mouse user should not be affected.

To test use keyboard alone to navigate to sub menu.

@RobPrecious for queries.

@welcome

welcome Bot commented Mar 21, 2021

Copy link
Copy Markdown

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

@choldgraf

Copy link
Copy Markdown
Member

This is excellent! Thanks so much for the improvement - nobody in the EBP community has strong expertise with accessibility issues so this is much appreciated 🙏

I think the tests are failing because of the regression checks:

https://github.com/executablebooks/sphinx-book-theme/pull/309/checks?check_run_id=2159861280#step:5:44

Each time the tests run, it checks whether the outputs of these files changes:

https://github.com/executablebooks/sphinx-book-theme/tree/master/tests/test_build

since this changes the sidebar HTML, I think that's why the check is failing.

you could update the HTML by deleting the files in that folder that are failing, and running the tests twice (it'll generate the file if no file exists). That said, if this is too much heavy lifting let me know and I can try to push a fix myself to your branch 👍

@IsabelBirds

Copy link
Copy Markdown
Contributor Author

@choldgraf - Thanks for the info on the tests, will take a look at sorting them out on this and our other PR later :)

@choldgraf

Copy link
Copy Markdown
Member

many thanks @IsabelBirds for all of this great work!

@scmmmh

scmmmh commented Nov 7, 2022

Copy link
Copy Markdown

What needs to be done to progress this? There's a few other accessibility things I would like to start looking at, but it would be useful if this PR could be merged first?

@choldgraf

choldgraf commented Nov 7, 2022

Copy link
Copy Markdown
Member

Right now we're trying to simplify theme as much as possible to inherit structure/features/fixes from the pydata sphinx theme. Check out this PR here:

I think this PR is likely too out-of-date to be mergeable at this point. It is sorely needed, but I think that the codebase has changed a lot and it would probably be easier to start from scratch. I'd focus efforts on improving accessibility of the pydata sphinx theme, and we can then inherit here.

@scmmmh

scmmmh commented Nov 7, 2022

Copy link
Copy Markdown

Yeah, I saw that, which is why I asked. I'll look into providing some input into the pydata sphinx theme.

@choldgraf

Copy link
Copy Markdown
Member

I'll close this one so that we can iterate in the pydata theme and inherit here - thanks all for the discussion, and apologies that this one never quite made it to the finish line!

@choldgraf choldgraf closed this Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants