-
Notifications
You must be signed in to change notification settings - Fork 314
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
Custom Route shapes fix #2623
Custom Route shapes fix #2623
Conversation
…zing route and it's casing shape.
@@ -473,7 +473,8 @@ open class NavigationMapView: MGLMapView, UIGestureRecognizerDelegate { | |||
let allRoutesShape = navigationMapViewDelegate?.navigationMapView(self, shapeFor: routes) ?? shape(for: routes, legIndex: legIndex) | |||
let allRoutesSource = addAllRoutesSource(style, shape: allRoutesShape) | |||
|
|||
let mainRouteCasingShape = navigationMapViewDelegate?.navigationMapView(self, shapeFor: [mainRoute]) ?? shape(for: [mainRoute], legIndex: legIndex) | |||
let mainRouteCasingShape = navigationMapViewDelegate?.navigationMapView(self, simplifiedShapeFor: mainRoute) ?? shape(forCasingOf: mainRoute, legIndex: legIndex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should allRoutesShape
also call navigationMapView(_:simplifiedShapeFor:)
? What should call navigationMapView(_:shapeFor:)
? Previously, a developer would’ve implemented navigationMapView(_:shapeFor:)
to provide the shape data for the whole route (normally to be used as the casing) and navigationMapView(_:simplifiedShapeFor:)
to provide the shape data for the route broken up by arbitrary segments (normally to be colored by traffic congestion).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I found in the code, navigationMapView(_:shapeFor:)
is meant to provide route shapes, and navigationMapView(_:simplifiedShapeFor:)
meant to provide the casing shape. At least when I provided my custom Shapes like that they were colored as route and casing respectfully. Even if my custom shape for navigationMapView(_:shapeFor:)
had just one Polyline
it still was colored with congestion segments.
Doesn't it supposed to be like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I found in the code,
navigationMapView(_:shapeFor:)
is meant to provide route shapes, andnavigationMapView(_:simplifiedShapeFor:)
meant to provide the casing shape.
Yes, sorry, I got the two methods mixed up in #2623 (comment). If you provide only one shape in navigationMapView(_:shapeFor:)
, I don’t think there should be colored congestion segments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So... Is this convo resolved? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remember to add a changelog entry about restoring the ability to customize the route shape.
@1ec5, @Udumft After merging this PR to Before route line casing was visible: When I check We should also update changelog with newly added fix. |
Shame on me for missing this one :( but thank you for pointing that out! Fix submitted in mapbox/mapbox-navigation-ios/pull/2715 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Navigation on mapview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Authored
Fixes #2595
Restored calling
NavigationMapViewDelegate.navigationMapView(_:simplifiedShapeFor:)
method for shape customization and verified that custom shapes mechanism works.There is a discussion going on around these methods on how to improve the API and make customization easy and straightforward. Eventually we should remove these methods, but decided to leave them for now because there is no alternative ideas developed atm and some clients already use that API.