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

iOS Accessibility - add headerBackAllowFontScaling option - #87

Merged
brentvatne merged 3 commits into
react-navigation:masterfrom
substantial:add-back-font-scaling
Feb 20, 2019
Merged

brentvatne merged 3 commits into
react-navigation:masterfrom
substantial:add-back-font-scaling

Conversation

@mikelovesrobots

@mikelovesrobots mikelovesrobots commented Feb 19, 2019 •

Copy link
Copy Markdown
Contributor

On iOS, apps can support larger font sizes for accessibility.

screenshot 2019-02-19 11 48 40

React-native supports this out-of-the-box, which is awesome. But the navigation bar turns into kind of a UX disaster:

screenshot 2019-02-19 11 53 05

Looking at default iOS apps provided by Apple, they often don't scale the navigation bar, which makes sense given the limited real-estate, so it's OK to turn that off. Like for example, here's what the settings app looks like with font-sizes maxed out.

screenshot 2019-02-19 12 11 39

And react-navigation provides an option for turning off font scaling for the title, getting us closer to a usable navigation bar:

navigationOptions: {
        headerTitleAllowFontScaling: false,
}

screenshot 2019-02-19 11 55 35

But it doesn't have an option for the back button, which this PR addresses.

navigationOptions: {
        headerBackAllowFontScaling: false, // new!
        headerTitleAllowFontScaling: false,
}

screenshot 2019-02-19 13 20 59

I'm excited about getting this change into react-navigation. If there's anything I can do to help that process please let me know.

@mikelovesrobots mikelovesrobots changed the title iOS Accessibility - adds headerBackAllowFontScaling option iOS Accessibility - add headerBackAllowFontScaling option Feb 19, 2019
@satya164
satya164 requested a review from brentvatne February 19, 2019 21:56
@brentvatne

Copy link
Copy Markdown
Member

thank you @mikelovesrobots! we should make this default to false

Comment thread src/views/Header/HeaderBackButton.js Outdated
onLayout={this._onTextLayout}
style={[styles.title, !!tintColor && { color: tintColor }, titleStyle]}
numberOfLines={1}
allowFontScaling={allowFontScaling}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
allowFontScaling={allowFontScaling}
allowFontScaling={!!allowFontScaling}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should be good enough to make it default to false

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

made this change directly myself

@brentvatne

Copy link
Copy Markdown
Member

once this PR lands can you open a PR to update docs on https://github.com/react-navigation/react-navigation.github.io please?

@brentvatne
brentvatne merged commit 77d9513 into react-navigation:master Feb 20, 2019
@mikelovesrobots

Copy link
Copy Markdown
Contributor Author

once this PR lands can you open a PR to update docs on https://github.com/react-navigation/react-navigation.github.io please?

Absolutely. I'll open a PR in a bit.

@mikelovesrobots

Copy link
Copy Markdown
Contributor Author

@brentvatne Here's a link to the documentation update:
react-navigation/react-navigation.github.io#366

Looking at it, the back button now defaults to false (for font scaling), but the title defaults to true. Is it weird to have those two defaults out of sync? Feels weird to me.

satya164 pushed a commit to react-navigation/react-navigation.github.io that referenced this pull request Feb 21, 2019
Here's a link to PR #87 over on react-navigation-stack where the feature was added:
react-navigation/stack#87
@brentvatne

Copy link
Copy Markdown
Member

good point, we should make title default to false as well

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.

3 participants