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 accessibility of theme toggle button #1669

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

C-Loftus
Copy link

@C-Loftus C-Loftus commented Jan 31, 2025

What does this PR change? What problem does it solve?

  • The button to change the theme has title Alt + T
  • this gives it a good tooltip on hover but it a screen reader user would just hear Alt + T and not know what the button does.
    • this is important to add since it is on the front page of every papermod site
  • Adding aria-label="Toggle theme" makes the screen reader speak out Toggle theme which is the semantic purpose of the button without overriding the title or tooltip

This isn't perfect and the button could have a white rectangle around it as well for focus but wanted to just add this since it doesn't require me going into the CSS as a first PR

Was the change discussed in an issue or in the Discussions before?

Not that I am aware.

PR Checklist

  • [x ] I have enabled maintainer edits for this PR.
  • [ x] I have verified that the code works as described/as intended.
  • [x ] This change does not include any CDN resources/links.
  • [x ] This change does not include any unrelated scripts such as bash and python scripts.

@C-Loftus
Copy link
Author

Not clear why merging is blocked, I just did a git push on my new fork and opened a PR on the github UI

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.

1 participant