Upgrade Mapbox SDK 2.0 #66
Conversation
|
@joe307bad Any idea on how to run these changes and see the outcome ? |
|
Thanks @JorgeQuevedoC, this looks pretty straight forward. I am going to find some time in the next few days to devise a testing strategy. Feel free to share any ideas here. |
|
I think that the best idea, as you also mentioned, is to create some examples within the same repo |
|
Yep! Here is a good example an example app inside a react native package. We should work on adding a folder like this in this package. |
|
Hey @joe307bad, I have just updated the PR with the Examples folder already set up!
|
|
This is really good work @JorgeQuevedoC, thanks for putting this together. I am going to find some time today to run this locally but the screenshot definitely goes a long way. A small request, can we move the example app back one directory so its in |
|
@joe307bad Sure thing! I will push as soon as it is done and tested |
|
Hi @joe307bad, Changes are already push into the PR with just 1 example folder |
|
Yo @JorgeQuevedoC! Im going to be helping out a bit with this library too when I have free time. Thanks for taking the initiative to make a PR. A couple things on this PR:
|
|
Hi @gciluffo, great feedback and glad to see more people involved on this. Tomorrow I will start reviewing the first 3 bullets; but, I am not very familiar to the process to update Android SDK. I would like to be involved but some help would be much appreciated |
|
@JorgeQuevedoC No worries, I can take a look at upgrading the Android SDK. |
|
@gciluffo I think we are ready on iOS side. Please correct me if wrong and let me know if something else is required |
|
@JorgeQuevedoC I think we just need to update this to version 11. But other than that everything else looks good. I am currently working on upgrading the SDK for Android on your fork. It looks like theres some compatibility issues with using Gradle 7 which is what the example project is using. So hopefully will have that working along with the Mapbox SDK upgrade before the week is over. |
|
@gciluffo I don't get where should I update the version |
|
@JorgeQuevedoC I should have a PR for the android upgrade within a couple days, I will merge it into your branch here. One thing I noticed when testing the iOS changes is that nothing happens when tapping the X to cancel navigation. The event from the swift code propagates up to React Native properly but the navigation doesnt stop when tapping it. |
|
@gciluffo those are great news. I didn't change that functionality but I can review it. So it must execute the function passed through props and after that stop the navigation no matter what, right? |
|
@JorgeQuevedoC Correct. It looks like its an outstanding issue #34. However it seems like it existed well before we upgraded iOS SDK. So no worries if you dont want to address in this PR. |
|
@gciluffo I can address it. Probably I can push that change tomorrow! |
|
@JorgeQuevedoC Lets just address it in another PR. I didnt implement cancelling a route in the android PR I submitted to your fork. That way the functionality between android and ios are the same for now. Also its not clear what the best approach to cancelling the route is either because in this implementation a user cannot create another route once they cancel it. The route is created entirely from the code once the component is initialized. Sorry for the back and forth. |
|
@gciluffo understood. Let's address it later then |
|
@JorgeQuevedoC Are you able to take a look at the PR I created for your fork? This PR doesnt have permissions to allow maintainers to push to it. |
…roid-sdk Feature/gciluffo/upgrade android sdk
|
@gciluffo let's give it a final review |
|
@JorgeQuevedoC Are able to check the |
|
@gciluffo it was checked
|
|
@JorgeQuevedoC I am still having permission issues pushing to your fork directly. I just went ahead and opening another pull request for your fork if you can a look at it. And then I think we should be good to go. JorgeQuevedoC#2 |
…roid-sdk Update styling for android view to allow more screen real estate
|
@gciluffo done! |
|
@JorgeQuevedoC Thanks! Just doing more testing and I noticed that the languages are mixed up in iOS in the example. I was expecting them to be the same language from whatever the system is using. Any ideas on this? This is on iOS simulator and my local language is set to english.
|
|
@gciluffo tbh I don't know what is causing that; but it is there since I started using the package. It just happens when simulation is on |
|
This is epic @JorgeQuevedoC and @gciluffo. 🎉 Thanks for the work on this. |
|
Hey guys, thanks for all your efforts for updating to v2. I just wanted to know if there is any status on when this update will be published on npm ? because the npm's version is still 1.1.0 |
|
Hey @adoniscoder we are currently trying to get access back to the npm repository so we can publish this. The previous developers who maintained this are no longer working here. If you need to consume the package now you can just reference like this in package.json: |
This PR upgrades the Mapbox Navigation SDK for iOS to 2.1.0 and 2.0.0 for Android.
The Android Mapbox Navigation SDK for version 2.X.X no longer supports the drop in UI as mentioned here: https://docs.mapbox.com/android/navigation/guides/migrate-to-v2/#navigationview-was-removed. So this PR implements a fully customizable UI for Android.
Everything will function the same except the prop endOfRouteFeedback will not work Android for now. If we want to support the feedback functionality we will need to create our own UI for this as this was removed from the drop in UI Mapbox supported for Android in previous versions.