-
Notifications
You must be signed in to change notification settings - Fork 12
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: make Android example app runnable #23
base: master
Are you sure you want to change the base?
Conversation
Prior to this change, attempts to build the Android example app failed with errors like 'Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.6.0, expected version is 1.1.15.' See this thread for a similar problem and the source of the solution: react-native-webview/react-native-webview#2578
Prior to this change, the 'yarn example android' command would fail because the gradlew file in the android example app was not an executable.
@mdole hi, we actually released a new version of the example recently, you may want to check that one out |
Hi @viaskal-sift! I tried deleting and re-cloning the repo and had the same problem as before. That makes sense to me - assuming this is the change you mean (since that is the only PR merged since this one was created that I can see), the change was only to the test setup and would not affect running the app. |
@mdole okay, I see, worth double checking things. Thanks for raising this! |
Thanks @viaskal-sift! While you're here: the Android part of the wrapper (not the example app, but the wrapper itself) also does not appear to be correctly sending events to Sift. I've flagged this with our rep, also sharing here in case this hasn't been reported to you yet. I'm happy to open an issue with it if that would be helpful. I recorded a video demo showing the problem here (3 videos sounds like a lot, but it's 10 min total 😄): Feel free to reach out if I can provide more info or you have questions! |
@mdole yes, we received this information already and currently working on it so no need to open it here in Github. The problem looks a bit trickier than we originally expected, but our goal is to release the fix later this week (optimistic scenario) or early next week (realistic/pessimistic scenario). |
Oh that's great to hear! Thanks for the update and for looking into it 🙂 and I'm glad to hear that my videos helped! |
@mdole sorry for being silent for some time. The problem was trickier than we originally thought, but we are about to provide you an example of the integration code that would help to address the problem of not having events for Android case. One thing I would like to double check - what is the navigation lib you're using in your app? Cause in the example we gonna provide we rely on |
Hey @viaskal-sift, all good! Yes, we use |
Hi @mdole ! I am reaching you about the issue you mentioned here - #23 (comment). We've forwarded the answer some time ago but seems we didn't hear back for some time, so I decided to reach you directly here Eventually, we can provide the workaround. We have a branch GitHub - SanoopC/sift-react-native at screen_navigation_example with the example of the integration code that should fix the problem. To reiterate - the problem we observed with missing events from Android flow in ReactNative was caused by the limitations of the events propagation between React-Native and Android. The proposed fix is not in the SDK code itself, but in the way we integrate the SDK into an app, thus, so no need to release a new version of the code for now @mdole The instructions can be found on the branch’s README (Track Screen Navigation section) , so please try this one and let us know if it is working for you. Worth mentioning we've got a feedback from other customer that it was working for them so there is high chance it will address your problem. Looking forward to hearing back from your side! |
Hey @viaskal-sift ! Thanks for touching base.
Oh, how did you send it? I don't recall receiving anything! Sorry if I missed it.
We tried this out, and it works fine :) that said, it's still relying on manual calls rather than the queueing system the native Android SDK and iOS SDKs have, which automatically upload events after a certain number of events or a certain amount of time. So I hope that Sift will still eventually ship a "real" fix as part of the Thanks again for your work on this! |
@mdole Thanks a lot for the quick answer!
I have an impression we were unfortunate enough to lose some communication steps between our engineering and your team since we communicated the workaround for some already
We will try to find a better solution, but unfortunately we have no prospects at the moment of a quick fix since there are some limitations for events propagation between React Native and Android that look like rather a blocker to us at the moment :( |
All good, thanks @viaskal-sift :) appreciate the info and the context! Also, AFAIK the issue that I originally opened this PR to address (the Android example app setup not working out-of-the-box) is still happening - not a high priority or anything, but is this mergeable? |
sure, I will push the team to verify this sooner, sorry for the delay |
Tried to set up the Android example app and ran into a couple problems. First, the
./gradlew
file was not an executable, fixed with achmod +x
(second commit in this PR). Second, the build would fail with errors like:Specifying the Kotlin version fixes it. Credit to: react-native-webview/react-native-webview#2578
It's possible that this issue is specific to my setup, but I did have a colleague try the setup and he had the exact same issues. So maybe a fix is necessary!
Let me know if I missed things 🙂