-
Notifications
You must be signed in to change notification settings - Fork 326
Add intersection signals along current route. #4185
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
Conversation
01c8afe to
b16f29b
Compare
| if style.image(withId: ImageIdentifier.trafficSignalDay) == nil, | ||
| let trafficSignlaDay = Bundle.mapboxNavigation.image(named: "TrafficSignalDay") { | ||
| try style.addImage(trafficSignlaDay, id: ImageIdentifier.trafficSignalDay) | ||
| } |
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.
It would be awesome if the current Style subclass could define these images right alongside Style.mapStyle.
| let layerPosition = layerPosition(for: layerIdentifier) | ||
| try style.addPersistentLayer(shapeLayer, layerPosition: layerPosition) |
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.
Looks like the intersection annotation layer is on top of the maneuver arrow layer. That makes logical sense and maintains the illusion of these traffic lights and signs standing upright. However, the maneuver arrow is an important element to be obscuring with an icon.
Will the maneuver arrow be prominent enough to overcome this overlap? Or should be consider a visual mitigation, such as offsetting the traffic light at a maneuver point?
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.
I think we could solve this issue by having a smaller icon or a longer maneuver arrow, from a UX design perspective. Because it's easy to understand by having the traffic signal icons place above the route line, which means that the user will drive through them.
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.
Yes, Google Maps takes a similar approach of elongating the maneuver arrow. That would also make the arrow more intuitive at large intersections: #3589. Apple Maps no longer displays maneuver arrows at all. Both applications display the traffic control device icon and highlight the intersection callout. #2928 would be a major usability improvement, making the maneuver arrow redundant, but maybe we could ticket out a simpler tweak to the maneuver arrow in the meantime.
8c8550d to
eb941e2
Compare
Breaking Changes in MapboxCoreNavigationBreaking API Changes
|
1 similar comment
Breaking Changes in MapboxCoreNavigationBreaking API Changes
|
9aed2c3 to
e94914b
Compare
e94914b to
e3aa974
Compare
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.
This is looking great! There’s some potential follow-up work detailed below, but from an API standpoint the feature is good to go.
| let layerPosition = layerPosition(for: layerIdentifier) | ||
| try style.addPersistentLayer(shapeLayer, layerPosition: layerPosition) |
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.
Yes, Google Maps takes a similar approach of elongating the maneuver arrow. That would also make the arrow more intuitive at large intersections: #3589. Apple Maps no longer displays maneuver arrows at all. Both applications display the traffic control device icon and highlight the intersection callout. #2928 would be a major usability improvement, making the maneuver arrow redundant, but maybe we could ticket out a simpler tweak to the maneuver arrow in the meantime.
...gation/Resources/Assets.xcassets/RoadIntersections/TrafficSignalNight.imageset/Contents.json
Show resolved
Hide resolved
e3aa974 to
0dfa87b
Compare
Description
This Pr is to fix #3843 by annotating intersection signals along the current route.
Implementation
NavigationViewController.annotatesIntersectionsAlongRouteandCarPlayNavigationViewController.annotatesIntersectionsAlongRouteto annotate intersections on the current route step during active navigation.NavigationViewControllerandCarPlayNavigationViewController, the signal icons from bundle resource will be added to the MapViewstylefor intersection signals in current style type. And then update the signal layers.RouteProgressupdated or reroute event happened, intersections on the current step and the first intersection on the upcoming step will be annotated on map. When user passed an intersection, the intersection signals will be removed from the source feature and disappear.Screenshots or Gifs