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

[NavigatorIOS] Fix pop method to properly refresh the navigatorBar #1989

Closed
wants to merge 1 commit into from
Closed

Conversation

jaysalvat
Copy link
Contributor

When using nativator.pop() the navigatorBar doesn't refresh properly if we change navigationBarHidden.

Current result:

When navigator.pop() is called, the navbar doesn't show up, even avec navigationBarHidden set back to true. See between Statusbar and Movie poster in the example below.

bad

// Not actually updating the indices yet until we get the native
// `onNavigationComplete`.
updatingAllIndicesAtOrBeyond: null,

https://github.com/facebook/react-native/blob/master/Libraries/Components/Navigation/NavigatorIOS.ios.js#L510-L512

Expected result (after fix)

The View need to be refreshed in pop method anyway.

updatingAllIndicesAtOrBeyond: this.state.requestedTopOfStack - n,

Now, navbar appears properly.

good

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jul 14, 2015
@sahrens
Copy link
Contributor

sahrens commented Jul 15, 2015

@ericvicenti, @ide: Any idea if this will cause problems? The previous code with the comment was pretty clear....

@sahrens
Copy link
Contributor

sahrens commented Jul 15, 2015

@ide: just mark as accepted when you think this is good to go and I'll import it.

@ide
Copy link
Contributor

ide commented Jul 15, 2015

Reading through, this won't cause correctness issues I believe but it may affect performance. Will need to dig in deeper since I'm not very familiar with this code.

@jaysalvat
Copy link
Contributor Author

Performance is important, but 'm sure we can't afford loosing navbar on pop().

@fender
Copy link

fender commented Nov 25, 2015

+1

I've noticed this bug recently too.

@facebook-github-bot
Copy link
Contributor

@jaysalvat updated the pull request.

@brentvatne
Copy link
Collaborator

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/972167456166907/int_phab to review.

@ghost ghost closed this in e62471b Jan 3, 2016
FalseLobster pushed a commit to FalseLobster/react-native that referenced this pull request Dec 19, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants