Road name labeling functionality for CarPlay. #3527

MaximAlien · 2021-10-27T22:09:17Z

Closing #3480.

Road name labeling on CarPlay during free-drive:

Road name labeling on CarPlay during active-guidance:

MaximAlien · 2021-10-28T00:39:05Z

Sources/MapboxNavigation/NavigationMapView+RoadNameLabeling.swift

+    func labelCurrentRoadFeature(at location: CLLocation,
+                                 router: Router,
+                                 wayNameView: WayNameView,
+                                 roadNameFromStatus: String?) {


Querying implementation doesn't always work on CarPlay as expected, this leads to inconsistency with iOS. I'm also not sure that performing querying on iOS and CarPlay at the same time is the best approach. Though, I do not have any good ideas for handling cases like: CarPlay starts navigation without iOS app running etc.

Querying on both “screens” should be fine in principle, even simultaneously, but there could be a scaling issue going on. Maybe the map SDK’s querying functionality is confused about the fact that the CarPlay map has a different scale and aspect ratio.

MaximAlien · 2021-10-28T18:47:14Z

Sources/MapboxNavigation/CarPlayNavigationViewController.swift

@@ -22,12 +22,17 @@ open class CarPlayNavigationViewController: UIViewController {

     This view is hidden by default.
     */
-    public weak var compassView: CarPlayCompassView!


Not sure why we've marked these as weak before.

weak is the default attribute for IBOutlets and they are being used similarly here to keep the retain count at +1. view.addSubview(compassView) will retain the view. Strongly held would make the retain count +2, which is also fine in this case, even necessary if you want to remove the view and add it back after viewDidLoad.

mapbox-github-ci-issues-1 · 2021-11-09T20:31:02Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-1 · 2021-11-09T20:31:09Z

No breaking changes detected in MapboxNavigation

mapbox-github-ci-issues-3 · 2021-11-09T22:13:13Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-3 · 2021-11-09T22:13:18Z

No breaking changes detected in MapboxNavigation

MaximAlien · 2021-11-09T22:38:51Z

Sources/MapboxNavigation/UIImage.swift

+                          color: UIColor,
+                          font: UIFont,
+                          size: CGSize? = nil) -> UIImage {
+        let maxHeight = size?.height ?? font.lineHeight * 2


In case if size is set we'll use it to provide maximum height for the image. In this case this height is height of UILabel container.

mapbox-github-ci-issues-5 · 2021-11-10T00:14:58Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-5 · 2021-11-10T00:15:01Z

No breaking changes detected in MapboxNavigation

CHANGELOG.md

1ec5 · 2021-11-10T00:18:17Z

Sources/MapboxNavigation/WayNameView.swift

@@ -10,10 +10,43 @@ open class WayNameLabel: StylableLabel {}
 /// :nodoc:


If we expect the developer to use this class or any of its members, we should document it. Until now, these classes have largely been used in Style subclasses in a particular way, so they didn’t require much beyond an example in practice, though documentation is long overdue.

Created ticket for this in #3584.

Sources/MapboxNavigation/WayNameView.swift

1ec5 · 2021-11-10T00:27:21Z

Sources/MapboxNavigation/NavigationMapView+RoadNameLabeling.swift

+    func labelCurrentRoadFeature(at location: CLLocation,
+                                 router: Router,
+                                 wayNameView: WayNameView,
+                                 roadNameFromStatus: String?) {


Querying on both “screens” should be fine in principle, even simultaneously, but there could be a scaling issue going on. Maybe the map SDK’s querying functionality is confused about the fact that the CarPlay map has a different scale and aspect ratio.

Sources/MapboxNavigation/NavigationMapView+RoadNameLabeling.swift

mapbox-github-ci-issues-1 · 2021-11-11T02:20:31Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-1 · 2021-11-11T02:20:34Z

No breaking changes detected in MapboxNavigation

mapbox-github-ci-issues-5 · 2021-11-11T21:40:06Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-5 · 2021-11-11T21:40:10Z

Breaking Changes in MapboxNavigation

Breaking API Changes

`InstructionsBannerViewDelegate`

modified protocol: InstructionsBannerViewDelegate

Type of change: Inherited Types

From: [["key.name": AnyObject], ["key.name": UnimplementedLogging]]

To: [["key.name": VisualInstructionDelegate]]

mapbox-github-ci-issues-3 · 2021-11-12T00:13:04Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-3 · 2021-11-12T00:13:07Z

Breaking Changes in MapboxNavigation

Breaking API Changes

`InstructionsBannerViewDelegate`

modified protocol: InstructionsBannerViewDelegate

Type of change: Inherited Types

From: [["key.name": AnyObject], ["key.name": UnimplementedLogging]]

To: [["key.name": VisualInstructionDelegate]]

mapbox-github-ci-issues-2 · 2021-11-12T04:11:53Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-2 · 2021-11-12T04:11:57Z

Breaking Changes in MapboxNavigation

Breaking API Changes

`InstructionsBannerViewDelegate`

modified protocol: InstructionsBannerViewDelegate

Type of change: Inherited Types

From: [["key.name": AnyObject], ["key.name": UnimplementedLogging]]

To: [["key.name": VisualInstructionDelegate]]

mapbox-github-ci-issues-3 · 2021-11-12T04:22:37Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-3 · 2021-11-12T04:22:40Z

Breaking Changes in MapboxNavigation

Breaking API Changes

`InstructionsBannerViewDelegate`

modified protocol: InstructionsBannerViewDelegate

Type of change: Inherited Types

From: [["key.name": AnyObject], ["key.name": UnimplementedLogging]]

To: [["key.name": VisualInstructionDelegate]]

mapbox-github-ci-issues-5 · 2021-11-12T04:34:24Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-5 · 2021-11-12T04:34:26Z

Breaking Changes in MapboxNavigation

Breaking API Changes

`InstructionsBannerViewDelegate`

modified protocol: InstructionsBannerViewDelegate

Type of change: Inherited Types

From: [["key.name": AnyObject], ["key.name": UnimplementedLogging]]

To: [["key.name": VisualInstructionDelegate]]

S2Ler

My main concern is about duplicated code. The rest is just small comments about clarity.

S2Ler · 2021-11-12T06:27:33Z

Sources/MapboxNavigation/NavigationMapView+RoadNameLabeling.swift

+
+extension NavigationMapView {
+
+    func labelCurrentRoadFeature(at location: CLLocation,


Is this method a copy of the same code from OrnamentsController? If so, even if this code will diverge from the origin, it is a maintenance problem. I suggest extracting the code into a sensible function that can be customized in the future if needed (or already needed?).

The original code already has a bug that incorrectly hides road labels.

#2548 tracks consolidating and exposing the logic for labeling the current road name.

@S2Ler, NavigationMapView.labelCurrentRoadFeature(at:router:wayNameView:roadNameFromStatus:) is already reused by iOS and CarPlay. Can you elaborate regarding:

The original code already has a bug that incorrectly hides road labels.

WayNameView is hidden whenever road name info is missing and we were not able to query it. It'll be shown back whenever is such info is available once again.

NavigationMapView.labelCurrentRoadFeature(at:router:wayNameView:roadNameFromStatus:) is already reused by iOS and CarPlay.

I'm sorry, missed that part, that's cool that there is no duplication 👍

WayNameView is hidden whenever road name info is missing and we were not able to query it. It'll be shown back whenever is such info is available once again.

I haven't got a chance to create a ticket, but on the freeways, I've noticed that road name disappears even though it shouldn't.

@MaximAlien #3594 created a bug for this.

Thanks. Since querying depends on the zoom level we'll have to find more reliable ways of getting road name information. Hopefully in scope of #2548 we'll be able to fix that.

Sources/MapboxNavigation/NavigationViewportDataSource.swift

Sources/MapboxNavigation/WayNameView.swift

CHANGELOG.md

Sources/MapboxNavigation/WayNameView.swift

1ec5 · 2021-11-12T16:04:03Z

Sources/MapboxNavigation/NavigationMapView+RoadNameLabeling.swift

+
+extension NavigationMapView {
+
+    func labelCurrentRoadFeature(at location: CLLocation,


#2548 tracks consolidating and exposing the logic for labeling the current road name.

mapbox-github-ci-issues-4 · 2021-11-12T17:38:49Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-4 · 2021-11-12T17:38:52Z

No breaking changes detected in MapboxNavigation

mapbox-github-ci-issues-5 · 2021-11-12T18:40:09Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-5 · 2021-11-12T18:40:14Z

No breaking changes detected in MapboxNavigation

S2Ler · 2021-11-15T08:16:47Z

Sources/MapboxNavigation/NavigationMapView+RoadNameLabeling.swift

+
+extension NavigationMapView {
+
+    func labelCurrentRoadFeature(at location: CLLocation,


NavigationMapView.labelCurrentRoadFeature(at:router:wayNameView:roadNameFromStatus:) is already reused by iOS and CarPlay.

I'm sorry, missed that part, that's cool that there is no duplication 👍

S2Ler · 2021-11-15T08:17:45Z

Sources/MapboxNavigation/NavigationMapView+RoadNameLabeling.swift

+
+extension NavigationMapView {
+
+    func labelCurrentRoadFeature(at location: CLLocation,


WayNameView is hidden whenever road name info is missing and we were not able to query it. It'll be shown back whenever is such info is available once again.

I haven't got a chance to create a ticket, but on the freeways, I've noticed that road name disappears even though it shouldn't.

S2Ler · 2021-11-15T08:29:36Z

Sources/MapboxNavigation/NavigationMapView+RoadNameLabeling.swift

+
+extension NavigationMapView {
+
+    func labelCurrentRoadFeature(at location: CLLocation,


@MaximAlien #3594 created a bug for this.

Sources/MapboxNavigation/WayNameView.swift

mapbox-github-ci-issues-2 · 2021-11-15T21:41:23Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-2 · 2021-11-15T21:41:26Z

No breaking changes detected in MapboxNavigation

…e navigation. Add the ability to fully hide `WayNameView`, improved its documentation, added the ability to style `backgroundColor` and `borderWidth`.

mapbox-github-ci-issues-3 · 2021-11-15T22:12:42Z

No breaking changes detected in MapboxCoreNavigation

mapbox-github-ci-issues-3 · 2021-11-15T22:12:46Z

No breaking changes detected in MapboxNavigation

MaximAlien added UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. CarPlay Bugs, improvements and feature requests on Apple CarPlay labels Oct 27, 2021

MaximAlien added this to the v2.0.1 milestone Oct 27, 2021

MaximAlien requested a review from a team October 27, 2021 22:09

MaximAlien self-assigned this Oct 27, 2021

MaximAlien changed the base branch from main to maxim/3507-add-current-activity-on-car-play October 27, 2021 22:09

MaximAlien marked this pull request as draft October 27, 2021 22:09

MaximAlien linked an issue Oct 27, 2021 that may be closed by this pull request

Show current road name label on CarPlay #3480

Closed

MaximAlien commented Oct 28, 2021

View reviewed changes

MaximAlien changed the title ~~Road name label functionality for CarPlay.~~ Road name labeling functionality for CarPlay. Oct 28, 2021

MaximAlien commented Oct 28, 2021

View reviewed changes

Base automatically changed from maxim/3507-add-current-activity-on-car-play to main October 28, 2021 23:26

MaximAlien mentioned this pull request Oct 28, 2021

Add functionality, which allows to show WayNameView on CarPlay during active-guidance and free-drive. #3534

Closed

MaximAlien modified the milestones: v2.0.1, v2.1 Oct 29, 2021

MaximAlien force-pushed the maxim/3480-road-name-label-on-car-play branch from 53e768d to 6b3982b Compare November 9, 2021 20:19

MaximAlien marked this pull request as ready for review November 9, 2021 22:09

MaximAlien commented Nov 9, 2021

View reviewed changes

MaximAlien force-pushed the maxim/3480-road-name-label-on-car-play branch from 05e2902 to 2d67bfb Compare November 10, 2021 00:05

1ec5 reviewed Nov 10, 2021

View reviewed changes

MaximAlien force-pushed the maxim/3480-road-name-label-on-car-play branch from 2d67bfb to c3c192e Compare November 11, 2021 02:10

MaximAlien force-pushed the maxim/3480-road-name-label-on-car-play branch from c3c192e to a22b120 Compare November 11, 2021 21:30

MaximAlien force-pushed the maxim/3480-road-name-label-on-car-play branch from 1617b4d to 789b4cf Compare November 12, 2021 04:25

S2Ler reviewed Nov 12, 2021

View reviewed changes

1ec5 reviewed Nov 12, 2021

View reviewed changes

MaximAlien force-pushed the maxim/3480-road-name-label-on-car-play branch from 789b4cf to 9efbb76 Compare November 12, 2021 17:26

MaximAlien force-pushed the maxim/3480-road-name-label-on-car-play branch from 6e830a2 to 3063072 Compare November 12, 2021 18:28

S2Ler approved these changes Nov 15, 2021

View reviewed changes

Add ability to show WayNameView on CarPlay during passive and activ…

75a2c60

…e navigation. Add the ability to fully hide `WayNameView`, improved its documentation, added the ability to style `backgroundColor` and `borderWidth`.

MaximAlien force-pushed the maxim/3480-road-name-label-on-car-play branch from 9916c1d to 75a2c60 Compare November 15, 2021 21:59

Fix compilation warnings.

b04e653

MaximAlien merged commit 6241d6e into main Nov 15, 2021

MaximAlien deleted the maxim/3480-road-name-label-on-car-play branch November 15, 2021 22:21

		@@ -10,10 +10,43 @@ open class WayNameLabel: StylableLabel {}
		/// :nodoc:


		extension NavigationMapView {

		func labelCurrentRoadFeature(at location: CLLocation,

Road name labeling functionality for CarPlay. #3527

Road name labeling functionality for CarPlay. #3527

Conversation

MaximAlien commented Oct 27, 2021 • edited Loading

MaximAlien Oct 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frederoni Oct 29, 2021 • edited Loading

Choose a reason for hiding this comment

mapbox-github-ci-issues-1 bot commented Nov 9, 2021

mapbox-github-ci-issues-1 bot commented Nov 9, 2021

mapbox-github-ci-issues-3 bot commented Nov 9, 2021

mapbox-github-ci-issues-3 bot commented Nov 9, 2021

Choose a reason for hiding this comment

mapbox-github-ci-issues-5 bot commented Nov 10, 2021

mapbox-github-ci-issues-5 bot commented Nov 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mapbox-github-ci-issues-1 bot commented Nov 11, 2021

mapbox-github-ci-issues-1 bot commented Nov 11, 2021

mapbox-github-ci-issues-5 bot commented Nov 11, 2021

mapbox-github-ci-issues-5 bot commented Nov 11, 2021

Breaking Changes in MapboxNavigation

Breaking API Changes

InstructionsBannerViewDelegate

mapbox-github-ci-issues-3 bot commented Nov 12, 2021

mapbox-github-ci-issues-3 bot commented Nov 12, 2021

Breaking Changes in MapboxNavigation

Breaking API Changes

InstructionsBannerViewDelegate

mapbox-github-ci-issues-2 bot commented Nov 12, 2021

mapbox-github-ci-issues-2 bot commented Nov 12, 2021

Breaking Changes in MapboxNavigation

Breaking API Changes

InstructionsBannerViewDelegate

mapbox-github-ci-issues-3 bot commented Nov 12, 2021

mapbox-github-ci-issues-3 bot commented Nov 12, 2021

Breaking Changes in MapboxNavigation

Breaking API Changes

InstructionsBannerViewDelegate

mapbox-github-ci-issues-5 bot commented Nov 12, 2021

mapbox-github-ci-issues-5 bot commented Nov 12, 2021

Breaking Changes in MapboxNavigation

Breaking API Changes

InstructionsBannerViewDelegate

S2Ler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mapbox-github-ci-issues-4 bot commented Nov 12, 2021

mapbox-github-ci-issues-4 bot commented Nov 12, 2021

mapbox-github-ci-issues-5 bot commented Nov 12, 2021

mapbox-github-ci-issues-5 bot commented Nov 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mapbox-github-ci-issues-2 bot commented Nov 15, 2021

mapbox-github-ci-issues-2 bot commented Nov 15, 2021

mapbox-github-ci-issues-3 bot commented Nov 15, 2021

mapbox-github-ci-issues-3 bot commented Nov 15, 2021

MaximAlien commented Oct 27, 2021 •

edited

Loading

MaximAlien Oct 28, 2021 •

edited

Loading

frederoni Oct 29, 2021 •

edited

Loading

`InstructionsBannerViewDelegate`

`InstructionsBannerViewDelegate`

`InstructionsBannerViewDelegate`

`InstructionsBannerViewDelegate`

`InstructionsBannerViewDelegate`