-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Add "Testing Instructions for Keyboard" to PR template to encourage accessibility testing #45957
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
Conversation
|
|
|
Raw: Rendered: What?Why?How?Testing InstructionsTesting Instructions for KeyboardScreenshots or screencast |
tellthemachines
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR! I'm happy to approve as-is, but also leaving an an alternative suggestion here: we could instead make it clear in the testing instructions comments that steps to test have to be keyboard accessible. The current approach with the separate heading might be easier to notice though, especially if folks are just skimming the headings, so let's try it out!
|
I left this open for a little while to see if it received additional feedback. I see some reactions, thanks @tellthemachines for the thoughts. Let's give it a try and see what people think and if PRs improve. |
|
I think it's a great idea to flag this as many people might not appreciate that testing instructions can be difficult to follow when using a keyboard. Can we include it all in the one Testing Instructions section though? Otherwise we're creeping back to having a long and arduous pull request template—the kind that we had before #39229. |
|
I'm inclined to keep it like this for a little while to see how much of a difference it makes - my concern with including it in the Testing Instructions section is that folks will skim read and ignore it. |
|
Kind of like that old accessibility checkbox. Everyone checked it, true or not. I was pushing for some accountability here. If you cannot test it with the keyboard, it likely does not belong in the project. That is my hard stance on A11Y. |
What?
This PR adds a new heading 3 to the PR template, "Testing Instructions for Keyboard".
Why?
It is my thinking that if users are forced to think about keyboard testing for user interface (UI) changes, some accessibility regressions could be prevented and new features could go out with fewer A11Y concerns. This also comes with the added benefit that it is more inclusive for everyone. Different people learn in different ways and also have different abilities. For example, someone may really benefit from being able to see the changes in a screenshot where I as a reviewer need to understand how I can test with my screen reader. This requires following a certain pattern on the keyboard that may not be obvious from the standard "Testing Instructions" heading 2 as they could be steps more geared towards sighted reviewers.
How?
A quick edit to the pull request template file.
Testing Instructions
Check the code diff. That is the best way for this. I will include the new sample template in a comment.
Screenshots or screencast