mapbox / mapbox-navigation-ios

Turn-by-turn navigation logic and UI in Swift on iOS
https://docs.mapbox.com/ios/navigation/
Other
861 stars 314 forks source link

Hide MapboxDirections implementation details #169

Closed ericrwolfe closed 7 years ago

ericrwolfe commented 7 years ago

A developer implementing the drop in navigation SDK shouldn't need to fiddle with shape resolution (#165) or steps on a Route from MapboxDirections.

let options = RouteOptions(waypoints: [origin, destination], profileIdentifier: .automobileAvoidingTraffic)
options.routeShapeResolution = .full
options.includesSteps = true

Is there a way we can better abstract these?

Should each of those be enabled by default in MapboxDirections for developers to turn off? Or should we wrap route generation in a convenience initializer (https://github.com/mapbox/mapbox-navigation-ios/issues/125).

/cc @1ec5 @frederoni

1ec5 commented 7 years ago

Should each of those be enabled by default in MapboxDirections for developers to turn off?

Up to now, I’ve taken the position that MapboxDirections.swift should match the Directions API’s default behavior. However, I’m open to changing the client-side default to better match developer expectations. By comparison, MapKit includes steps and unsimplified geometry in all responses, with no option to omit it.

As far as I can tell, the Directions API’s default behavior was motivated by desktop needs, specifically optimizing route updates as the user drags the origin or destination pin on an overview map. This use case is unimportant on mobile platforms, other than for route preview screens in navigation applications. The more common use case requires the inclusion of steps (steps=true) and the full-resolution route shape (overview=full).

/cc @mapbox/directions

danpat commented 7 years ago

@1ec5 Denis wasn't one for explaining his reasoning, but yes, making the "drag the marker around" scenario fast was most likely the motiviation.

I don't see a problem defaulting to full resolution in the mobile SDK - the use-case is different. The API parameters aren't going to go away.

1ec5 commented 7 years ago

Fixed in #531.