Additional Android functionality by FeralAI · Pull Request #61 · wix/react-native-navigation

FeralAI · 2016-06-09T16:22:04Z

I started looking into your library recently and was excited by the prospect of a single package to drive native navigation, but was a little discouraged when I saw a good portion of the example application wasn't working on Android. Given that and a little interest in RN modules I started poking around the project trying to get things working, with the result being this PR. Here's what I have so far:

Added Android screen API for:
- popToRoot
- resetTo
- setTitle
- setTabBadge
- switchToTab
- toggleNavBar
- toggleTabs
Fixed modal close after 1.0.9 update (missing super call)
Fixed error when popping, previously allowed popping past last screen on the stack
Refactored example app so screens aren't platform-specific

A couple things I was having issues with (or just don't know enough about yet):

ActionBar show/hide animation: I was able to animate the toolbar content using .animate(), but I'm not sure why the ActionBar container isn't animating (at least on my devices).
Modal background is transparent: I've added a bg color for the inital modal in the example so the text doesn't overlap, but I haven't dug into this issue far enough to know the cause.

This mostly gets the example app working on par with the iOS version, except lightbox which isn't currently implemented.

…ionStyle later)

…k on the three main tabs

# Conflicts: # android/app/src/main/java/com/reactnativenavigation/activities/SingleScreenActivity.java # android/app/src/main/java/com/reactnativenavigation/modal/RnnModal.java

…ionStyle, is called is all derived methods

FeralAI · 2016-06-11T21:33:59Z

Looks like there's some bugs with this currently. I'm working on getting this integrated with an app I'm currently working on, so I'll be updating over the next few days.

…p/react-native-navigation into more-android-updates

FeralAI · 2016-06-13T05:44:26Z

Didn't mean to merge that last set of changes, but the good news is there's a basic menu implementation now. I know there's a lot of code here and it needs some cleanup, but hopefully it can be useful to get more Android features going. I've recorded a demo of the example project running against our branch here: https://youtu.be/KWcZgAyZXJw. I'm going to be integrating our branch into a project I'm currently working on, so it will start getting battle-tested over the next few weeks.

guyca · 2016-06-13T07:08:34Z

@jskuby Thanks a lot for the PR. I'm starting to review it, but it will take me some time to go over such a large PR.

guyca · 2016-06-13T09:54:21Z

+        boolean hide = params.getBoolean(KEY_HIDDEN);
+        boolean animated = params.getBoolean(KEY_ANIMATED);
+        if (hide) {
+            mToolbar.hideToolbar(animated);


hideToolbar and showToolbar don't animate the transition since they only change the View's visibility to GONE or VISIBLE. I'll fix it after we merge.

MattDavies · 2016-06-14T14:42:28Z

I would love to see some of this get merged in. Perhaps we can break the PR up into a few chunks - e.g. pull the bug fixes out into separate PRs so we can test and merge them separately with confidence?

Worried the merge conflicts will build up pretty quickly on a PR this size as development continues on master. I'm happy to help with the splitting if you'd like :)

adamski · 2016-06-14T14:49:27Z

+1 on splitting it up, great work here :)

guyca · 2016-06-14T16:34:39Z

@jskuby, @MattDavies has a good point. I tried reviewing the PR before other tasks I have but it wasn't possible.

Today I refactored the logic related to setting/updating navigation style after screen change in daee13f - this commit alone results in a pretty massive conflict.

I suggest we split the PR as follows:

Fixes and enhancements to example project
setNavigationTitle
Hide/Show tabs in BottomTabActivity
setTabBadge
Toolbar toggle
popToRoot
resetTo
ScreenStack addView/RemoveView issue
Drawer - If it's ok with you, I would like us to merge this feature in last since the review process might take some time.

If you guys are interested, we can open a channel in discord so we have a more efficient form of communication.

guyca · 2016-06-15T12:56:36Z

@jskuby I'm going to start cherry picking some of your commits. Hope it's ok with you.

FeralAI · 2016-06-22T13:04:57Z

Sorry to disappear for a while there. Looks like some of the functionality has been integrated over the last week. If there's anything else you would like broken into separate PRs I can probably spare some time a few nights this week or this weekend to help move things along.

guyca · 2016-06-22T13:16:18Z

@jskuby You got us worried for a while. I did in fact cherry pick most of your commits (Thanks a lot!)
I didn't have time to rebase + cherry pick the drawer commits.
If you could submit a separate PR for the drawer it would be great!
Thanks

FeralAI · 2016-06-22T13:23:05Z

That's great to hear @guyca! I'll get on the drawer PR as soon as possible.

# Conflicts: # android/app/src/main/java/com/reactnativenavigation/activities/BaseReactActivity.java # android/app/src/main/java/com/reactnativenavigation/activities/BottomTabActivity.java # android/app/src/main/java/com/reactnativenavigation/activities/SingleScreenActivity.java # android/app/src/main/java/com/reactnativenavigation/activities/TabActivity.java # android/app/src/main/java/com/reactnativenavigation/core/objects/Screen.java # android/app/src/main/java/com/reactnativenavigation/modal/RnnModal.java # android/app/src/main/java/com/reactnativenavigation/modules/RctActivityModule.java # android/app/src/main/java/com/reactnativenavigation/views/RnnToolBar.java # android/app/src/main/java/com/reactnativenavigation/views/ScreenStack.java # src/platformSpecific.android.js

FeralAI · 2016-06-24T03:27:37Z

I made some progress by merging main master with my master branch. At this point, I think we close this PR and I'll do some cleanup and open separate requests for the remaining updates, which are mainly the drawer implementation and some updates to the example app.

guyca · 2016-06-24T05:08:24Z

Sounds great @jskuby. Thanks!

Jason Skuby added 26 commits June 8, 2016 10:06

Update base activity name to get example running

1d9e0d6

Update toolbar along with style update (may want to rename setNavigat…

28f3cc7

…ionStyle later)

Add basic setNavigationTitle method

128d2f7

Make the modal work and presentable on Android

2682147

Fix Android modal not closing on first try

c4433d1

Wire up tab show/hide and tab selection

9d8c0d2

Wire up Android setTabBadge

2e48348

Add toolbar toggle, not sure if animation is working

714b31e

Consolidate example code since everything but lightbox should now wor…

005d1e5

…k on the three main tabs

Fix circular reference error parsing navigatorEventSubscription

229fd5d

Implement popToRoot

4515e13

Implement resetTo

be6c8e2

Merge branch 'master' of https://github.com/wix/react-native-navigation

496c2a9

# Conflicts: # android/app/src/main/java/com/reactnativenavigation/activities/SingleScreenActivity.java # android/app/src/main/java/com/reactnativenavigation/modal/RnnModal.java

Use update styles

05b511c

Remove setNavigationStyle since updatesStyles, which calls setNavigat…

cb48fd5

…ionStyle, is called is all derived methods

Cleanup

3d0b6cd

Fix boolean test

2a18c6b

Revert drawer flag

89a9ea4

Update remaining alert references

692e2e9

Restore super call for modal back

1a7129f

Restore update

1da6343

Update tab title and persist title on tab change

efa84a2

Make sure addView is called before removeView to get fade transition

7f3d9ba

Restore toolbar updates

8d34eac

Fix reference

c79c92c

Fix reference

f5964a4

Jason Skuby added 3 commits June 11, 2016 16:49

Fix navbar bg color and text color

e2f3bb0

Add left drawer for Android

7ef6dbf

Merge branch 'more-android-updates' of https://github.com/the-mx-grou…

5c0de57

…p/react-native-navigation into more-android-updates

Jason Skuby added 3 commits June 12, 2016 17:46

Fix drawer appearance

b5fdfe7

Fix tint color for native drawer icon

f97f170

Fix custom menu icon not displaying

2e1f84e

guyca self-assigned this Jun 13, 2016

guyca reviewed Jun 13, 2016
View reviewed changes

guyca force-pushed the master branch from 8f131f4 to daee13f Compare June 14, 2016 11:52

Jason Skuby added 2 commits June 23, 2016 22:04

Set default nav arrow color to black

dc74c1c

guyca closed this Jun 24, 2016

aminosman mentioned this pull request Dec 1, 2019

[Android] App crash after react-native-webview upgrade [Solved] #5702

Closed

Conversation

FeralAI commented Jun 9, 2016

Uh oh!

FeralAI commented Jun 11, 2016

Uh oh!

FeralAI commented Jun 13, 2016

Uh oh!

guyca commented Jun 13, 2016

Uh oh!

guyca Jun 13, 2016

Choose a reason for hiding this comment

Uh oh!

MattDavies commented Jun 14, 2016

Uh oh!

adamski commented Jun 14, 2016

Uh oh!

guyca commented Jun 14, 2016

Uh oh!

guyca commented Jun 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FeralAI commented Jun 22, 2016

Uh oh!

guyca commented Jun 22, 2016

Uh oh!

FeralAI commented Jun 22, 2016

Uh oh!

FeralAI commented Jun 24, 2016

Uh oh!

guyca commented Jun 24, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

guyca commented Jun 15, 2016 •

edited

Loading