From f020fe07548e2ecb093cf791b09f2f7ed140b404 Mon Sep 17 00:00:00 2001 From: Jill Cardamon Date: Thu, 17 Nov 2022 11:50:56 -0500 Subject: [PATCH 1/2] Make destination annotation methods generic and change identifiers. --- .../MapboxNavigation/NavigationMapView.swift | 39 +++++++++++++++---- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/Sources/MapboxNavigation/NavigationMapView.swift b/Sources/MapboxNavigation/NavigationMapView.swift index 176affafa05..bd3fb974b52 100755 --- a/Sources/MapboxNavigation/NavigationMapView.swift +++ b/Sources/MapboxNavigation/NavigationMapView.swift @@ -1335,14 +1335,30 @@ open class NavigationMapView: UIView { if let lastLeg = route.legs.last, let destinationCoordinate = lastLeg.destination?.coordinate { - addDestinationAnnotation(destinationCoordinate) + addDestinationAnnotation(destinationCoordinate) { [weak self] in + guard self != nil else { return } + } } } - func addDestinationAnnotation(_ coordinate: CLLocationCoordinate2D, - identifier: String = NavigationMapView.AnnotationIdentifier.finalDestinationAnnotation) { + /** + Adds a final destination annotation to the map. + + - parameter coordinate: Coordinate which represents the annotation location. + - parameter identifier: String to uniquely identify the destination annotation. Defaults to `nil` and a default identifier will be provided. + - parameter styleLoaded: An escaping closure to be executed when the `MapView` style has finished loading. + */ + public func addDestinationAnnotation(_ coordinate: CLLocationCoordinate2D, + identifier: String? = nil, + styleLoaded: @escaping () -> Void) { + let identifier = identifier ?? String("finalDestinationAnnotation_\(finalDestinationAnnotations.count)") var destinationAnnotation = PointAnnotation(id: identifier, coordinate: coordinate) destinationAnnotation.image = .init(image: .defaultMarkerImage, name: ImageIdentifier.markerImage) + + mapView.mapboxMap.onNext(event: .styleLoaded) { [weak self] _ in + guard self != nil else { return } + styleLoaded() + } // If `PointAnnotationManager` is available - add `PointAnnotation`, if not - remember it // and add it only after fully loading `MapView` style. @@ -1356,12 +1372,19 @@ open class NavigationMapView: UIView { } } - func removeDestinationAnnotation(_ identifier: String = NavigationMapView.AnnotationIdentifier.finalDestinationAnnotation) { - let remainingAnnotations = pointAnnotationManager?.annotations.filter { - $0.id != identifier + /** + Removes a final destination annotation to the map. + + - parameter identifier: String to uniquely identify the destination annotation to be removed. Defaults to `nil` and removes all destination annotations. + */ + public func removeDestinationAnnotation(_ identifier: String? = nil) { + if let identifier { + finalDestinationAnnotations.removeAll(where: { $0.id == identifier }) + pointAnnotationManager?.annotations.removeAll(where: { $0.id == identifier }) + } else { + finalDestinationAnnotations.removeAll(where: { $0.id.contains("finalDestinationAnnotation") }) + pointAnnotationManager?.annotations.removeAll(where: { $0.id.contains("finalDestinationAnnotation") }) } - - pointAnnotationManager?.annotations = remainingAnnotations ?? [] } /** From 790cbade3fc89355a44355aad1b08499067ebd4a Mon Sep 17 00:00:00 2001 From: Nastassia Makaranka Date: Wed, 8 Mar 2023 17:46:02 +0100 Subject: [PATCH 2/2] Add tests, update changelog and formatting for final destination --- CHANGELOG.md | 1 + .../xcshareddata/swiftpm/Package.resolved | 16 +-- .../MapboxNavigation/NavigationMapView.swift | 53 ++++----- .../NavigationMapViewTests.swift | 102 +++++++++++++++++- 4 files changed, 132 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b560af6a1e8..28e2cec58ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ ### Map * Fixed an issue when `NavigationMapView` would not display a map, when using CarPlay and custom tile storage location. ([#4399](https://github.com/mapbox/mapbox-navigation-ios/pull/4399)) +* Added `NavigationMapView.addDestinationAnnotation(_:identifier:styleLoaded:)`, `NavigationMapView.removeDestinationAnnotation(_:)` to present and remove the final destination annotation on a NavigationMapView. ([#4253](https://github.com/mapbox/mapbox-navigation-ios/pull/4253)) ### Camera diff --git a/MapboxNavigation-SPM.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/MapboxNavigation-SPM.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 0e112854dee..2ec650992b6 100644 --- a/MapboxNavigation-SPM.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/MapboxNavigation-SPM.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -24,8 +24,8 @@ "repositoryURL": "https://github.com/mapbox/mapbox-common-ios.git", "state": { "branch": null, - "revision": "185d791e7478cc5da7cc2b4756ecd0d0bd387eac", - "version": "23.4.0-beta.1" + "revision": "d38abed1cfca3843308d478450c3d8b07a797c5a", + "version": "23.4.0-rc.1" } }, { @@ -33,8 +33,8 @@ "repositoryURL": "https://github.com/mapbox/mapbox-core-maps-ios.git", "state": { "branch": null, - "revision": "81b929488b5bbc467df4cf294ceffa0ae98ef0e0", - "version": "10.12.0-beta.1" + "revision": "82706a65bf7a51818d943c91f014d17588b0f65e", + "version": "10.12.0-rc.1" } }, { @@ -60,8 +60,8 @@ "repositoryURL": "https://github.com/mapbox/mapbox-maps-ios.git", "state": { "branch": null, - "revision": "246fa104e4eec0fb913c036cb0505723e26a9a2e", - "version": "10.12.0-beta.1" + "revision": "e47d5d6f2b55b0decd9d60b47b3c9c9b6c726fa3", + "version": "10.12.0-rc.1" } }, { @@ -123,8 +123,8 @@ "repositoryURL": "https://github.com/apple/swift-argument-parser", "state": { "branch": null, - "revision": "fee6933f37fde9a5e12a1e4aeaa93fe60116ff2a", - "version": "1.2.2" + "revision": "fddd1c00396eed152c45a46bea9f47b98e59301d", + "version": "1.2.0" } }, { diff --git a/Sources/MapboxNavigation/NavigationMapView.swift b/Sources/MapboxNavigation/NavigationMapView.swift index bd3fb974b52..7af111cd493 100755 --- a/Sources/MapboxNavigation/NavigationMapView.swift +++ b/Sources/MapboxNavigation/NavigationMapView.swift @@ -1273,7 +1273,7 @@ open class NavigationMapView: UIView { final destination `PointAnnotation` will be stored in this property and added to the `MapView` later on. */ - var finalDestinationAnnotations: [PointAnnotation] = [] + private(set) var finalDestinationAnnotations: [PointAnnotation] = [] /** Adds the route waypoints to the map given the current leg index. Previous waypoints for completed legs will be omitted. @@ -1335,31 +1335,25 @@ open class NavigationMapView: UIView { if let lastLeg = route.legs.last, let destinationCoordinate = lastLeg.destination?.coordinate { - addDestinationAnnotation(destinationCoordinate) { [weak self] in - guard self != nil else { return } - } + addDestinationAnnotation(destinationCoordinate, identifier: AnnotationIdentifier.finalDestinationAnnotation) } } - + /** - Adds a final destination annotation to the map. - + Adds a final destination annotation to the map. The annotation will be added only after fully loading `MapView` style. In such case + final destination will be stored and added to the `MapView` later on. `delegate` will be notified about the change via + `NavigationMapViewDelegate.navigationMapView(_:didAdd:pointAnnotationManager:)` method. + - parameter coordinate: Coordinate which represents the annotation location. - parameter identifier: String to uniquely identify the destination annotation. Defaults to `nil` and a default identifier will be provided. - - parameter styleLoaded: An escaping closure to be executed when the `MapView` style has finished loading. */ public func addDestinationAnnotation(_ coordinate: CLLocationCoordinate2D, - identifier: String? = nil, - styleLoaded: @escaping () -> Void) { - let identifier = identifier ?? String("finalDestinationAnnotation_\(finalDestinationAnnotations.count)") + identifier: String? = nil) { + let count = pointAnnotationManager?.annotations.count ?? finalDestinationAnnotations.count + let identifier = identifier ?? "\(AnnotationIdentifier.finalDestinationAnnotation)_\(count)" var destinationAnnotation = PointAnnotation(id: identifier, coordinate: coordinate) destinationAnnotation.image = .init(image: .defaultMarkerImage, name: ImageIdentifier.markerImage) - - mapView.mapboxMap.onNext(event: .styleLoaded) { [weak self] _ in - guard self != nil else { return } - styleLoaded() - } - + // If `PointAnnotationManager` is available - add `PointAnnotation`, if not - remember it // and add it only after fully loading `MapView` style. if let pointAnnotationManager = pointAnnotationManager { @@ -1373,18 +1367,20 @@ open class NavigationMapView: UIView { } /** - Removes a final destination annotation to the map. - - - parameter identifier: String to uniquely identify the destination annotation to be removed. Defaults to `nil` and removes all destination annotations. + Removes a final destination annotation from the map. + + - parameter identifier: String to uniquely identify the destination annotation to be removed. Defaults to `nil` and removes all destination annotations with non-custom identifiers. */ - public func removeDestinationAnnotation(_ identifier: String? = nil) { - if let identifier { - finalDestinationAnnotations.removeAll(where: { $0.id == identifier }) - pointAnnotationManager?.annotations.removeAll(where: { $0.id == identifier }) + public func removeDestinationAnnotation(identifier: String? = nil) { + let filter: (PointAnnotation) -> Bool + if let identifier = identifier { + filter = { $0.id == identifier } } else { - finalDestinationAnnotations.removeAll(where: { $0.id.contains("finalDestinationAnnotation") }) - pointAnnotationManager?.annotations.removeAll(where: { $0.id.contains("finalDestinationAnnotation") }) + filter = { $0.id.contains(AnnotationIdentifier.finalDestinationAnnotation) } } + + finalDestinationAnnotations.removeAll(where: filter) + pointAnnotationManager?.annotations.removeAll(where: filter) } /** @@ -1456,11 +1452,6 @@ open class NavigationMapView: UIView { */ public var pointAnnotationManager: PointAnnotationManager? - func annotationsToRemove() -> [Annotation] { - let identifier = NavigationMapView.AnnotationIdentifier.finalDestinationAnnotation - return pointAnnotationManager?.annotations.filter({ $0.id == identifier }) ?? [] - } - // MARK: Map Rendering and Observing var routes: [Route]? diff --git a/Tests/MapboxNavigationTests/NavigationMapViewTests.swift b/Tests/MapboxNavigationTests/NavigationMapViewTests.swift index cd46c5879c8..ff3bf45d1c7 100644 --- a/Tests/MapboxNavigationTests/NavigationMapViewTests.swift +++ b/Tests/MapboxNavigationTests/NavigationMapViewTests.swift @@ -2,7 +2,7 @@ import XCTest import MapboxDirections import TestHelper import Turf -import MapboxMaps +@testable import MapboxMaps @testable import MapboxNavigation @testable import MapboxCoreNavigation @@ -13,6 +13,7 @@ class NavigationMapViewTests: TestCase { ])) var navigationMapView: NavigationMapView! var navigator: CoreNavigator! + var pointAnnotationManager: PointAnnotationManager! let options: NavigationRouteOptions = .init(coordinates: [ CLLocationCoordinate2D(latitude: 40.311012, longitude: -112.47926), @@ -22,11 +23,34 @@ class NavigationMapViewTests: TestCase { let route = response.routes!.first! return route }() + + private let coordinate1 = CLLocationCoordinate2D(latitude: 40.311012, longitude: -112.47926) + private let coordinate2 = CLLocationCoordinate2D(latitude: 30.176322, longitude: -102.806108) + private let finalDestinationAnnotationTestPrefix = "MapboxNavigation-MapboxNavigation-resources_finalDestinationAnnotation" + + private final class DisplayLinkCoordinatorSpy: DisplayLinkCoordinator { + func add(_ participant: DisplayLinkParticipant) {} + func remove(_ participant: DisplayLinkParticipant) {} + } + + private final class AnnotationImagesManagerSpy: AnnotationImagesManagerProtocol { + func addImage(_ image: UIImage, id: String, sdf: Bool, contentInsets: UIEdgeInsets) {} + func removeImage(_ imageName: String) {} + func register(imagesConsumer: MapboxMaps.AnnotationImagesConsumer) {} + func unregister(imagesConsumer: MapboxMaps.AnnotationImagesConsumer) {} + } override func setUp() { navigator = Navigator.shared super.setUp() navigationMapView = NavigationMapView(frame: CGRect(origin: .zero, size: .iPhone6Plus)) + let mapboxMap = navigationMapView.mapView.mapboxMap! + pointAnnotationManager = PointAnnotationManager(id: "", + style: mapboxMap.style, + layerPosition: nil, + displayLinkCoordinator: DisplayLinkCoordinatorSpy(), + imagesManager: AnnotationImagesManagerSpy(), + offsetPointCalculator: OffsetPointCalculator(mapboxMap: mapboxMap)) } override func tearDown() { @@ -587,6 +611,82 @@ class NavigationMapViewTests: TestCase { XCTAssertFalse(style.layerExists(withId: NavigationMapView.LayerIdentifier.intersectionAnnotationsLayer)) } + func testAddFinalDestinationWithDefaultIdentifierIfStyleNotLoaded() { + XCTAssertTrue(navigationMapView.finalDestinationAnnotations.isEmpty) + XCTAssertTrue(pointAnnotationManager.annotations.isEmpty) + + navigationMapView.addDestinationAnnotation(coordinate1) + XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 1) + XCTAssertEqual(navigationMapView.finalDestinationAnnotations[0].id, "\(finalDestinationAnnotationTestPrefix)_0") + XCTAssertTrue(pointAnnotationManager.annotations.isEmpty) + + navigationMapView.addDestinationAnnotation(coordinate2) + XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 2) + XCTAssertEqual(navigationMapView.finalDestinationAnnotations[1].id, "\(finalDestinationAnnotationTestPrefix)_1") + XCTAssertTrue(pointAnnotationManager.annotations.isEmpty) + } + + func testAddFinalDestinationWithCustomIdentifierIfStyleNotLoaded() { + navigationMapView.addDestinationAnnotation(coordinate1, identifier: "custom") + XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 1) + XCTAssertEqual(navigationMapView.finalDestinationAnnotations[0].id, "custom") + } + + func testRemovesFinalDestinationIfStyleNotLoaded() { + navigationMapView.addDestinationAnnotation(coordinate1) + navigationMapView.addDestinationAnnotation(coordinate2) + + navigationMapView.removeDestinationAnnotation(identifier: "custom") + XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 2) + + navigationMapView.addDestinationAnnotation(coordinate2, identifier: "custom") + XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 3) + navigationMapView.removeDestinationAnnotation(identifier: "custom") + XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 2) + + navigationMapView.removeDestinationAnnotation() + XCTAssertEqual(navigationMapView.finalDestinationAnnotations.count, 0) + } + + func testAddFinalDestinationWithDefaultIdentifierIfStyleLoaded() { + navigationMapView.pointAnnotationManager = pointAnnotationManager + + navigationMapView.addDestinationAnnotation(coordinate1) + XCTAssertTrue(navigationMapView.finalDestinationAnnotations.isEmpty) + XCTAssertEqual(pointAnnotationManager.annotations.count, 1) + XCTAssertEqual(pointAnnotationManager.annotations[0].id, "\(finalDestinationAnnotationTestPrefix)_0") + + navigationMapView.addDestinationAnnotation(coordinate2) + XCTAssertTrue(navigationMapView.finalDestinationAnnotations.isEmpty) + XCTAssertEqual(pointAnnotationManager.annotations.count, 2) + XCTAssertEqual(pointAnnotationManager.annotations[1].id, "\(finalDestinationAnnotationTestPrefix)_1") + } + + func testAddFinalDestinationWithCustomIdentifierIfStyleLoaded() { + navigationMapView.pointAnnotationManager = pointAnnotationManager + + navigationMapView.addDestinationAnnotation(coordinate1, identifier: "custom") + XCTAssertEqual(pointAnnotationManager.annotations.count, 1) + XCTAssertEqual(pointAnnotationManager.annotations[0].id, "custom") + } + + func testRemovesFinalDestinationIfStyleLoaded() { + navigationMapView.pointAnnotationManager = pointAnnotationManager + navigationMapView.addDestinationAnnotation(coordinate1) + navigationMapView.addDestinationAnnotation(coordinate2) + + navigationMapView.removeDestinationAnnotation(identifier: "custom") + XCTAssertEqual(pointAnnotationManager.annotations.count, 2) + + navigationMapView.addDestinationAnnotation(coordinate2, identifier: "custom") + XCTAssertEqual(pointAnnotationManager.annotations.count, 3) + navigationMapView.removeDestinationAnnotation(identifier: "custom") + XCTAssertEqual(pointAnnotationManager.annotations.count, 2) + + navigationMapView.removeDestinationAnnotation() + XCTAssertEqual(pointAnnotationManager.annotations.count, 0) + } + private func configureIntersections() { let style = navigationMapView.mapView.mapboxMap.style var source = GeoJSONSource()