Skip to content
This repository has been archived by the owner on Feb 25, 2020. It is now read-only.

Adding accessibility role and state to bottom bar #90

Merged
merged 2 commits into from
Mar 30, 2019
Merged

Adding accessibility role and state to bottom bar #90

merged 2 commits into from
Mar 30, 2019

Conversation

fawcilize
Copy link
Contributor

Motivation

Exposing accessibilityRole and accessibilityStates through props in the same way as accessibilityLabel. This allows screen readers to announce things like: Selected. Banana. Button.

Reasonable defaults were provided, however, consumers can provide custom overrides in the cases where the role might be imagebutton or the state might be disabled.

Test plan

Select a button with VoiceOver (iOS) or Talkback (Android) enabled. The screen reader should read something like Selected. <AccessibilityLabel>. Button. or something slightly different depending on which platform you are on. Buttons that are not focused should not have Selected announced.

src/views/BottomTabBar.js Outdated Show resolved Hide resolved
route,
});
const accessibilityLabel = this.props.getAccessibilityLabel({ route });
const accessibilityRole = this.props.getAccessibilityRole({ route }) || 'button';
Copy link
Member

Choose a reason for hiding this comment

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

What other accessibility roles instead of button do you have in mind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think having button is a good default, but if someone only had an icon with no label, they may want to use imagebutton. Overriding getAccessibilityRole would allow that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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


let accessibilityStates = this.props.getAccessibilityStates({ route });
if (!accessibilityStates) {
accessibilityStates = focused ? ['selected'] : [];
Copy link
Member

Choose a reason for hiding this comment

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

What other accessibility states instead of selected/not selected do you have in mind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

selected and disabled are the only options allowed through the API, but this library would not really know if a tab is disabled (perhaps for an app's custom permissions?). If someone had a disabled tab, they could override getAccessibilityStates and return ['disabled'] or even ['selected', 'disabled'].

src/views/BottomTabBar.js Outdated Show resolved Hide resolved
@orzarchi
Copy link

orzarchi commented Mar 6, 2019

Any chance of this PR getting some attention? This is currently the only hold out on making my entire app accessible.

I would also like it if the focused prop will be passed to each of the new props for better control.
For example, it is currently impossible to create different accessibility labels for selected/unselected buttons.
The default talkback/voiceover behaviour when combining a control's state and label is inadequate in some non-english languages, and a custom label workaround is required.

@fawcilize
Copy link
Contributor Author

There has been a merge conflict since I first opened the PR, so I pushed up a fix.

@satya164 Was there something else you'd like me to address?

@satya164
Copy link
Member

Sorry for the delay. Can you rebase against master so the CI runs? I'll merge it after the CI passes.

@fawcilize
Copy link
Contributor Author

I rebased and fixed some linting errors. I did not include the states on the MaterialTopBar because they are not exposed by the underlying lib react-native-tab-view

@satya164 satya164 requested a review from brentvatne March 26, 2019 13:41
Copy link
Member

@brentvatne brentvatne left a comment

Choose a reason for hiding this comment

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

lgtm!

@satya164 satya164 merged commit e50447d into react-navigation:master Mar 30, 2019
satya164 pushed a commit that referenced this pull request Apr 1, 2019
Exposing `accessibilityRole` and `accessibilityStates` through props in the same way as `accessibilityLabel`. This allows screen readers to announce things like: `Selected. Banana. Button.`

Reasonable defaults were provided, however, consumers can provide custom overrides in the cases where the role might be `imagebutton` or the state might be `disabled`.

Select a button with VoiceOver (iOS) or Talkback (Android) enabled. The screen reader should read something like `Selected. <AccessibilityLabel>. Button.` or something slightly different depending on which platform you are on. Buttons that are not focused should not have `Selected` announced.
@MoOx
Copy link

MoOx commented Jul 31, 2019

Any reason 'tab' didn't end up being the default? I tried some apple native apps, and they all say ". Tab. of " (I guess the parent need to be set with accessibilityRole="tablist".

@satya164
Copy link
Member

Any reason 'tab' didn't end up being the default? I tried some apple native apps, and they all say ". Tab. of "

I remember that's what we used to do. Maybe accidentally changed it.

I guess the parent need to be set with accessibilityRole="tablist"

AFAIK RN doesn't expose this value?

If you could send a PR to fix it, it'll be awesome.

@MoOx
Copy link

MoOx commented Jul 31, 2019

image

Ok I will see what I can do :)

@satya164
Copy link
Member

Ah, must be a recent addition 😅

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants