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

Rework the NavigationHeader #21

Merged
merged 4 commits into from
Jan 21, 2016
Merged

Rework the NavigationHeader #21

merged 4 commits into from
Jan 21, 2016

Conversation

satya164
Copy link
Collaborator

Make <NavigationHeader /> work on both iOS and Android, and make it more customizable. Refer #20

iOS:

ios

Android:

android

@ericvicenti
Copy link
Owner

Sorry about the delay on this Satyajit! I'll merge it tonight

@satya164
Copy link
Collaborator Author

Cool. Thanks :)

@ericvicenti
Copy link
Owner

I landed #19 tonight, so it caused some thrash in the code

I've done most of the merging on this branch here: https://github.com/ericvicenti/navigation-rfc/tree/navheader

But there is still a bridge number error that shows up, as well as some small formatting issues. Would you mind taking a look?

@satya164
Copy link
Collaborator Author

@ericvicenti Hey. Will do ASAP. Thanks :)

@satya164
Copy link
Collaborator Author

Will be great if you can comment on the formatting issues :)

@satya164
Copy link
Collaborator Author

Merged master to my branch.

@satya164
Copy link
Collaborator Author

BTW I see this error on master (after running AsyncStorage.clear(), otherwise it wasn't displaying anything).

This only happens on iOS, not Android.

iphone 5 - iphone 5 - ios 9 2 13c75 simulator today at 5 33 39 pm

@ericvicenti
Copy link
Owner

I only noticed that issue after I patched in your header changes

@satya164
Copy link
Collaborator Author

@ericvicenti Hey. Seems that it only happens if Chrome debugger is connected. (Both in master and my branch). This not specifically related to my branch.

Here is a screencast confirming that it happens on master - https://www.youtube.com/watch?v=KdSdBDgXFIE

Sorry, I would've taken a hit at this, but I've no idea where it's coming from and why :(

ericvicenti added a commit that referenced this pull request Jan 21, 2016
@ericvicenti ericvicenti merged commit f0dd2cd into ericvicenti:master Jan 21, 2016
@ericvicenti
Copy link
Owner

Ah, in that case I'll merge your PR now! The redbox seems to be caused by the transform translateX in the NavigationCard.. hrmmm. It could be a bug in Animated, but more likely something going wrong with chrome debugging.

@satya164
Copy link
Collaborator Author

Awesome! 🎉

@satya164 satya164 deleted the navigation-header branch January 21, 2016 12:30
@satya164
Copy link
Collaborator Author

I'm also seeing the error on the master branch of RN with old Navigator code.

satya164 referenced this pull request in expo/react-native Feb 14, 2016
Summary:
A new API to unify internal navigation. Also addresses a highly-rated community 'pain': https://productpains.com/post/react-native/better-navigator-api-and-docs/

Offers the following improvements:

- Redux-style navigation logic is easy to reason about
- Navigation state can be easily saved and restored through refreshes
- Declarative navigation views can be implemented in native or JS
- Animations and gestures are isolated and now use the Animated library

public

Reviewed By: hedgerwang

Differential Revision: D2798048

fb-gh-sync-id: 88027ef9ead8a80afa38354252bc377455cc6dbb
ghost pushed a commit to facebook/react-native that referenced this pull request Mar 22, 2016
Summary:Add ability to specify custom left, right components, and title component. Style the `NavigationBar` according to the Platform.

Refer ericvicenti/navigation-rfc#21

cc ericvicenti
Closes #5971

Differential Revision: D3080601

Pulled By: ericvicenti

fb-gh-sync-id: 7b921cd36b4c2ec1edf6f52629f1f9890d272dfd
shipit-source-id: 7b921cd36b4c2ec1edf6f52629f1f9890d272dfd
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

2 participants