Bioconductor / BiocStyle

Issues and pull requests for BiocStyle should go here.
12 stars 19 forks source link

Accessibility tweaks #91

Closed grimbough closed 2 years ago

grimbough commented 2 years ago

Hi @aoles

This pull request is in response to some issues @mtmorgan mentioned a while ago regarding accessibility for screen readers. The summary of that email was:

Table of contents

My understanding is that the TOC is dynamically generated by tocify.js, so we can't address point one by simply post processing the HTML file, since the TOC doesn't actually exist at this point. I've tried to tackle this by adding some additional javascript, which runs after the TOC is created and adds role=link and tabindex=0 to any element with the class tocify-item. It also adds an event handler so that an Enter button press is mapped to the same event as a mouse click i.e. scrolls to the appropriate section. I don't know how to test the usability of this for someone who actually relies on a screen reader, but here's an animation of the result.

https://user-images.githubusercontent.com/971237/136765024-b94d7384-fcbb-4bb6-8f23-15f12262578d.mp4

Alt text

I think only the first figure in the vignette was missing alternative text, since this was deliberately demonstrating something without a caption. At some point knitr added support for the fig.alt chunk option, so I've added that to the example. Hopefully that addresses the issue.

Additional vignette info

Given the above point about alt text I've also added a subsection to the vignette regarding accessibility considerations, to highlight the fig.alt option and why people might want to consider using it. I've also added some text mentioning the importance of palette choice when plotting. I don't know how you feel about adding information that isn't strictly about using BiocStyle, but this feels like a relatively easy place to gently remind users that these are important things to consider.

Happy to receive any feedback on this and iterate.

grimbough commented 2 years ago

I think I've addressed the comments if you want to take another look.

grimbough commented 2 years ago

Thanks @aoles My timing on this was pretty bad, as it's the code freeze for a new branch today. I'm not sure I want to merge this and then find it breaks a ton of vignettes in RELEASE_3_14 tomorrow, so I'll hold off for a day and merge once the devel branch is open again.

aoles commented 2 years ago

Actually, the freeze you mentioned is only about API. Any fixes to vignettes or documentation should still be possible until October 20. Besides, I don't expect the changes from this PR to have much braking potential. But I understand if you don't feel comfortable with going forward with the update now. Cheers!

grimbough commented 2 years ago

Ah, you looked in more detail than me! I just have "don't break Bioc stuff" marked in the calendar for tomorrow. I'm happy with a week to pickup any problems and fix / revert if needed. I'll make the merge now.

mtmorgan commented 2 years ago

Thanks @grimbough for acting on these suggestions!