-
-
Notifications
You must be signed in to change notification settings - Fork 576
refactor(iOS): limit scope of shouldUseCustomBackBarButtonItem
flag
#2867
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
refactor(iOS): limit scope of shouldUseCustomBackBarButtonItem
flag
#2867
Conversation
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 don't think this works.
Consider scenario: config.disableBackButtonMenu == true && config.isBackTitleVisible == false
You won't use custom back button then -> you will break the behaviour of disableBackButtonMenu
prop. We can't merge this.
But it didn't work before these changes too. When As far as I remember this is what we decided in #2800, the backTitleVisible works as a kill switch. |
shouldUseCustomBackBarButtonItem
flag
shouldUseCustomBackBarButtonItem
flagshouldUseCustomBackBarButtonItem
flag
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 think we're good. Thanks!
Description / Changes
This is continuation of #2860 that proposes small refactor around back button logic. This area is very confusing and in my opinion this is due to flag
shouldUseCustomBackBarButtonItem
that breaks the flow of other if's and causes other props to not work. Not we contained this flag inif (visible)
which currently is the only case when backButtonItem might be used after the changes from #2800.Test code and steps to reproduce
see
FabricExample/e2e/issuesTests/Test2809.e2e.ts
andapps/src/tests/Test2809/index.tsx
Checklist