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

A11Y: Add title attribute to theme switcher #1924

Merged
merged 2 commits into from
Sep 11, 2024
Merged

Conversation

Carreau
Copy link
Collaborator

@Carreau Carreau commented Jul 11, 2024

This will add a <title> attribute to the SVG generated by font awesome, which will in turn update the Accessibility tree, which will now be

  • button "light/dark" focusable: true focused: bool
    • image: $title-text

Should fix Quansight-Labs/czi-scientific-python-mgmt#76

Copy link

github-actions bot commented Jul 18, 2024

Coverage report

This PR does not seem to contain any modification to coverable code.

Copy link
Collaborator

@gabalafou gabalafou left a comment

Choose a reason for hiding this comment

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

Sweet!

I'm wondering about word choice, though. I think it would be useful to get Isabela to weigh in during the team sync.

I also think the strings should be translatable, right?

@Carreau
Copy link
Collaborator Author

Carreau commented Jul 18, 2024

I'm wondering about word choice, though. I think it would be useful to get Isabela to weigh in during the team sync.

I also think the strings should be translatable, right?

Yep, sounds good, I'm usually bad at narative and text bing @isabela-pf for the text

@gabalafou
Copy link
Collaborator

After merging #1921, this now has conflicts

@Carreau
Copy link
Collaborator Author

Carreau commented Aug 6, 2024

No worries, I'll rebase.

This will add a `<title>` attribute to the SVG generated by font
awesome, which will in turn update the Accessibility tree, which will
now be

  - button "light/dark" focusable: true focused: bool
     - image: $title-text

Should fix Quansight-Labs/czi-scientific-python-mgmt#76
@trallard trallard added kind: enhancement New feature or request tag: accessibility Issues related to accessibility issues or efforts labels Aug 20, 2024
Copy link
Collaborator

@gabalafou gabalafou left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! I like the addition of the translation test :)

@trallard trallard merged commit 6da5715 into pydata:main Sep 11, 2024
19 of 29 checks passed
@Carreau
Copy link
Collaborator Author

Carreau commented Sep 11, 2024

Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement New feature or request tag: accessibility Issues related to accessibility issues or efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The light/dark toggle button does not communicate its state
5 participants