Skip to content

Conversation

@gabalafou
Copy link
Collaborator

This pull request brings the accessibility docs up to date.

Some of the changes in this pull request involve hard-wrapping the text in the docs files. I feel like this itself is an accessibility improvement as long lines are harder to read. It's also makes text diffing in git better.

@github-actions
Copy link

Coverage report

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

@gabalafou gabalafou force-pushed the update-accessibility-docs branch from 7faf842 to 04d26d2 Compare November 14, 2024 10:51
Copy link
Collaborator Author

@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.

self review

Comment on lines -14 to -18
We run a [Lighthouse](https://developers.google.com/web/tools/lighthouse) job in our CI/CD, which generates a "score" for all pages in our **Kitchen Sink** example documentation.
The configuration for Lighthouse can be found in the `.github/workflows/lighthouserc.json` file.
## Known limitations and manual auditing

For more information about configuring Lighthouse, see [the Lighthouse documentation](https://github.com/GoogleChrome/lighthouse-ci/blob/main/docs/configuration.md).
For more information about Accessibility in general, see [](../../user_guide/accessibility.md).
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unless somebody says not to, I am going to create a follow-up PR to remove the Lighthouse checks and configuration.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fine by me

Comment on lines -100 to -102
"text": "PyData Theme",
"image_dark": "_static/logo-dark.svg",
"alt_text": "PyData Theme home",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This example is misleading because our code now sets alt="" if a theme adopter sets the logo["text"] field. This is to avoid hearing the same text repeated twice by the screen reader, for example: "PyData Theme home PyData Theme".

I created what I think is a more realistic example based on the assumption that most sites will only put their logo in the top left and not the project text.

I guess I should add a note explaining that only one of text or alt_text should be specified; only rarely should both be specified together.

I'm thinking the need for this much nuance also suggests that we should simplify the config somehow.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess I should add a note explaining that only one of text or alt_text should be specified; only rarely should both be specified together.

Agree, let's add a note about this.

I'm thinking the need for this much nuance also suggests that we should simplify the config somehow.

+1, do you have any ideas?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know if there really is a way to simplify it because users would encounter the same issue if they were writing HTML. But I guess this is a good example of why I personally dislike config files, because it's a bit hard to convey through the config object itself how its state will affect the state of the end application.

Anyway, I realized that all of this is covered in much greater detail in a separate page, Branding and logo. So in my last commit I slimmed down this section and linked to that page.

Comment on lines -138 to -140
however, be aware that they do not follow accessibility best practices for
native button components such as using the correct `ARIA attributes
<https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/button_role>`__.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I felt that this did not help the theme adopter because it did not make a recommendation nor was the info helpful enough for a non-accessibility expert to make a judgment call, so I updated it with a specific recommendation (to use sparingly) and a specific example (about how the space bar doesn't work on links that look like buttons).

drammock
drammock previously approved these changes Nov 14, 2024
Co-authored-by: Daniel McCloy <dan@mccloy.info>
@gabalafou
Copy link
Collaborator Author

Thanks for reviewing @drammock ☺️

@trallard trallard added kind: documentation Improvements or additions to documentation tag: accessibility Issues related to accessibility issues or efforts labels Nov 18, 2024
trallard
trallard previously approved these changes Nov 18, 2024
Copy link
Collaborator

@trallard trallard left a comment

Choose a reason for hiding this comment

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

Thank you @gabalafou this looks pretty good.
Left a couple of suggestions but this looks ready to merge IMO.

Comment on lines -14 to -18
We run a [Lighthouse](https://developers.google.com/web/tools/lighthouse) job in our CI/CD, which generates a "score" for all pages in our **Kitchen Sink** example documentation.
The configuration for Lighthouse can be found in the `.github/workflows/lighthouserc.json` file.
## Known limitations and manual auditing

For more information about configuring Lighthouse, see [the Lighthouse documentation](https://github.com/GoogleChrome/lighthouse-ci/blob/main/docs/configuration.md).
For more information about Accessibility in general, see [](../../user_guide/accessibility.md).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fine by me

Comment on lines -100 to -102
"text": "PyData Theme",
"image_dark": "_static/logo-dark.svg",
"alt_text": "PyData Theme home",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess I should add a note explaining that only one of text or alt_text should be specified; only rarely should both be specified together.

Agree, let's add a note about this.

I'm thinking the need for this much nuance also suggests that we should simplify the config somehow.

+1, do you have any ideas?

and [Chrome](https://developers.google.com/web/tools/chrome-devtools/accessibility/reference),
have accessibility tools built-in as part of their web developer tools.
These tools can help to quickly identify accessibility issues and often include links to standards.
#### Browser tools
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to include polypane? IMO is a great tool, the downside is one needs a license.

Copy link
Collaborator Author

@gabalafou gabalafou Nov 18, 2024

Choose a reason for hiding this comment

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

Yeah, I had the same thought on my first pass but didn't add it because the document was structured a bit differently and I wasn't sure if we should promote a paid product.

But given that we both find it useful, I agree with you that it could help some people to know about it, so I added a sentence about it.

Co-authored-by: Tania Allard <taniar.allard@gmail.com>
@gabalafou gabalafou requested a review from trallard November 18, 2024 21:42
@gabalafou
Copy link
Collaborator Author

@trallard, I think because I made changes I need a re-review

@gabalafou gabalafou requested a review from drammock November 18, 2024 21:43
@trallard trallard merged commit 08f2b78 into pydata:main Nov 19, 2024
25 checks passed
@gabalafou gabalafou deleted the update-accessibility-docs branch November 20, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: documentation Improvements or additions to documentation tag: accessibility Issues related to accessibility issues or efforts

3 participants