Skip to content
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

fix: add internal isNewBackTitleImplementation const #1791

Merged
merged 3 commits into from Jun 19, 2023

Conversation

kacperkapusciak
Copy link
Member

@kacperkapusciak kacperkapusciak commented Jun 16, 2023

Description

Because of a bug introduced in #1646 react-native-screens v3.21 changed how header's backTitle handles whitespace strings in #1726

To allow for backwards compatibility in @react-navigation/native-stack we need have a way to check if this version or newer is used

See react-navigation/react-navigation#11423 for more context.

Changes

Added new isNewBackTitleImplementation internal constant that can be used in @react-navigation/native-stack.

Screenshots / GIFs

This change & react-navigation/react-navigation#11423 applied:

Screen.Recording.2023-06-16.at.15.12.46.mov

Without this change and react-navigation/react-navigation#11423:

Screen.Recording.2023-06-16.at.15.14.19.mov

Test code and steps to reproduce

Test1791.tsx

You need to apply changes introduced in react-navigation/react-navigation#11423 to @react-navigation/native-stack to test these canges.

Checklist

  • Included code example that can be used to test this change

@kacperkapusciak kacperkapusciak changed the title fix: add isNewBackTitleImplementation constant fix: add internal isNewBackTitleImplementation Jun 16, 2023
@kacperkapusciak kacperkapusciak marked this pull request as ready for review June 16, 2023 13:16
@kacperkapusciak kacperkapusciak changed the title fix: add internal isNewBackTitleImplementation fix: add internal isNewBackTitleImplementation const Jun 16, 2023
Copy link
Member

@tboba tboba left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

Copy link
Member

@WoLewicki WoLewicki left a comment

Choose a reason for hiding this comment

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

LGTM!

src/utils.ts Outdated

// Because of a bug introduced in https://github.com/software-mansion/react-native-screens/pull/1646
// react-native-screens v3.21 changed how header's backTitle handles whitespace strings in https://github.com/software-mansion/react-native-screens/pull/1726
// To allow for backwards compatibility in @react-navigation/native-stack we need have a way to check if this version or newer is used
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// To allow for backwards compatibility in @react-navigation/native-stack we need have a way to check if this version or newer is used
// To allow for backwards compatibility in @react-navigation/native-stack we need a way to check if this version or newer is used.

Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

🚀

@kkafar kkafar merged commit d481b3e into main Jun 19, 2023
3 of 5 checks passed
@kkafar kkafar deleted the @kacperkapusciak/add-isNewBackTitleImplementation branch June 19, 2023 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants