-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Docs: Resolves #11860: Add guidelines for making new contributions accessible #11863
base: dev
Are you sure you want to change the base?
Docs: Resolves #11860: Add guidelines for making new contributions accessible #11863
Conversation
readme/dev/accessibility.md
Outdated
- **Labels**: All focusable controls should have labels. Usually, these labels are provided by text inside the controls. Components that don't have any text may need to be labelled manually. | ||
- **Minimum size**: Buttons should be at least 24px by 24px. | ||
- **Keyboard accessibility**: It shouldn't be necessary to use a mouse to use a component. | ||
- **Screen reader accessibility**: It should be possible to use the desktop and mobile apps with a [screen reader](https://en.wikipedia.org/wiki/Screen_reader). | ||
- **Contrast**: Text and images should have high contrast with the background (see [the WebAIM contrast checker](https://webaim.org/resources/contrastchecker/)). |
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.
I would suggest making these H3 headers and give more details. Maybe some general pointers (for example "if a focusable control doesn't have a label, set an aria-label instead" or links that explains more precisely what to do for that particular section. We can't cover everything here but there are certain things that need to be done more often than not and those things we can mention
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.
I've replaced most of this information with links to the relevant WCAG 2.2 "Understanding" docs.
readme/dev/accessibility.md
Outdated
|
||
### Desktop: Automated accessibility testing | ||
|
||
Existing Playwright automated tests can be found in `packages/app-desktop/integration-tests`. These tests can be run with `yarn test-ui` from `packages/app-desktop`. It's possible to run a subset of all tests using `yarn test-ui -g "test name search pattern"`, where `-g` stands for "grep". |
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.
As I understand Playwrite is more generally for our UI tests. In that case is there a document in our own documentation we can link to?
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.
I've moved some of this information to app-desktop/integration-tests/README.md
. In the future, it may make sense to move this README to the readme/dev
folder.
|
||
### Mobile: Automated accessibility testing | ||
|
||
On mobile, [react-native-testing-library](https://callstack.github.io/react-native-testing-library/) also provides a [getByRole](https://callstack.github.io/react-native-testing-library/docs/api/queries#by-role) selector that can be used to ensure that accessibility information is present. See, for example, [this test for list of installed plugins](https://github.com/laurent22/joplin/blob/bf58a52394947acc42f8ca527b3ce22464d989c3/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.installed.test.tsx#L231). |
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.
Likewise we should link in priority to our own documentation (assuming this is documented somewhere)
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.
This doesn't seem to be documented anywhere.
Converts bullet points to links to the WCAG "Understanding" documents
integration-tests/README
Summary
This pull request documents Joplin's approach to accessibility.
Resolves #11860.
Screenshot
At present: