Copy navigation models to navigation core #84
Conversation
boldtrn
force-pushed
the
issue_37_local_models
branch
from
91fec3a
to
57b36f5
Compare
| @@ -225,7 +226,7 @@ class MockNavigationActivity : | |||
| this.accessToken(getString(R.string.mapbox_access_token)) | |||
| this.origin(origin) | |||
| this.destination(destination) | |||
| this.voiceUnits(DirectionsCriteria.METRIC) | |||
| this.voiceUnits(com.mapbox.services.android.navigation.v5.models.DirectionsCriteria.METRIC) | |||
There was a problem hiding this comment.
full qualifier here is not necessary, you forgot to remove the com.mapbox.api.directions.v5.models.DirectionsCriteria import in line 14. Replace that import with com.mapbox.services.android.navigation.v5.models.DirectionsCriteria
app/src/main/java/com/mapbox/services/android/navigation/testapp/MockNavigationActivity.kt
Show resolved
Hide resolved
app/src/main/java/com/mapbox/services/android/navigation/testapp/NavigationUIActivity.kt
Show resolved
Hide resolved
app/src/main/java/com/mapbox/services/android/navigation/testapp/NavigationUIActivity.kt
Outdated
Show resolved
Hide resolved
...n-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/listeners/RouteListener.java
Outdated
Show resolved
Hide resolved
...main/java/com/mapbox/services/android/navigation/v5/models/WalkingOptionsAdapterFactory.java
Show resolved
Hide resolved
...gation/src/test/java/com/mapbox/services/android/navigation/v5/utils/ToleranceUtilsTest.java
Show resolved
Hide resolved
.../src/main/java/com/mapbox/services/android/navigation/v5/routeprogress/RouteLegProgress.java
Outdated
Show resolved
Hide resolved
|
The code changes are looking good for me. I also understand the idea of this changes. But what I don't understand is the module architecture. The |
My first goal was to remove all dependencies to Mapbox APIs out of the core. So you can use the basic navigation without Mapbox. I think most serious apps will write their own UI as it is right now anyway, so apps can decide how they want to implement the route fetching and don't need to carry the unnecessary Mapbox weight in their apps. The ideal setup IMHO would be the further modularize the UI that it's possible to have use some of the UI elements in your own app. Then we could have a module for Mapbox routing, one for GraphHopper, one for Valhalla, one for... |
|
Okay, then we have the same thoughts. Thanks for explaining this! |
|
Thanks for all the reviews. I would kindly ask everyone to give this a good test in your apps as well, if nothing odd comes up, we can release a new major version in 1-2 weeks or so. |
|
I already tested the changed code base in our app, by review process. I don't see any issues with our implementation. ✅ |
Fixes #37.
This is a bigger change and would result in releasing a new major version due to several breaking changes. This is the foundation to finally loosening this repository from the Mapbox API and make it easier for anyone to include third party routers. So we could consider looking at #64 afterwards.
I would like to ask everyone to give this a good test, if everything works out, we can merge this soon.