Skip to content
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

Ensure AnnotationManager's mapLoaded happens first #246

Merged
merged 5 commits into from
Apr 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@

Mapbox welcomes participation and contributions from everyone.

### Breaking changes ⚠️

- `AnnotationManager` no longer conforms to `Observer` and no longer has a `peer` ([#246](https://github.com/mapbox/mapbox-maps-ios/pull/246))
- `AnnotationSupportableMap` is now internal ([#246](https://github.com/mapbox/mapbox-maps-ios/pull/246))

### Bug fixes 🐞

- Fixes an issue that could cause issues with annotations including causing them to not be selectable ([#246](https://github.com/mapbox/mapbox-maps-ios/pull/246))

## 10.0.0-beta.16 - March 29, 2021

### Breaking changes ⚠️
Expand Down
28 changes: 10 additions & 18 deletions Sources/MapboxMaps/Annotations/AnnotationManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ import MapboxMapsFoundation

All annotations added with this class belong to a single source and style layer.
*/
public class AnnotationManager: Observer {

public var peer: MBXPeerWrapper?
public class AnnotationManager {

// MARK: - Public properties

Expand Down Expand Up @@ -130,7 +128,6 @@ public class AnnotationManager: Observer {

deinit {
self.tapGesture = nil
try! self.mapView?.observable?.unsubscribe(for: self, events: [MapEvents.mapLoaded])
}

/**
Expand All @@ -155,7 +152,15 @@ public class AnnotationManager: Observer {
userInteractionEnabled = true

configureTapGesture()
try! mapView.observable?.subscribe(for: self, events: [MapEvents.mapLoaded])
mapView.on(.mapLoaded) { [weak self] _ in
// Reset the annotation source and default layers.
guard let self = self else { return }
self.annotations = [:]
self.annotationSource = nil
self.symbolLayer = nil
self.lineLayer = nil
self.fillLayer = nil
}
}

internal func updateAnnotationOptions(with newOptions: AnnotationOptions) {
Expand Down Expand Up @@ -664,17 +669,4 @@ public class AnnotationManager: Observer {
// Updating the annotation failed.
case updateAnnotationFailed(Error?)
}

public func notify(for event: MapboxCoreMaps.Event) {
guard event.type == MapEvents.mapLoaded else {
return
}

// Reset the annotation source and default layers.
annotations = [:]
annotationSource = nil
symbolLayer = nil
lineLayer = nil
fillLayer = nil
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import MapboxMapsStyle
import MapboxMapsFoundation
#endif

public protocol AnnotationSupportableMap: UIView {
var observable: Observable? { get }
internal protocol AnnotationSupportableMap: UIView {
func visibleFeatures(in rect: CGRect,
styleLayers: Set<String>?,
filter: Expression?,
completion: @escaping (Result<[Feature], BaseMapView.QueryRenderedFeaturesError>) -> Void)
func on(_ eventType: MapEvents.EventKind, handler: @escaping (MapboxCoreMaps.Event) -> Void)
}

extension BaseMapView: AnnotationSupportableMap {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import CoreLocation
//swiftlint:disable explicit_acl explicit_top_level_acl
class AnnotationInteractionDelegateTests: XCTestCase {

var annotationSupportableMapMock: AnnotationSupportableMapMock!
var annotationSupportableStyleMock: AnnotationStyleDelegateMock!
var annotationSupportableMapMock: MockAnnotationSupportableMap!
var annotationSupportableStyleMock: MockAnnotationStyleDelegate!
var defaultCoordinate: CLLocationCoordinate2D!

var selectionExpectation: XCTestExpectation?
Expand All @@ -20,8 +20,8 @@ class AnnotationInteractionDelegateTests: XCTestCase {
var deselectionDelegateWasCalled: Bool = false

override func setUp() {
annotationSupportableMapMock = AnnotationSupportableMapMock()
annotationSupportableStyleMock = AnnotationStyleDelegateMock()
annotationSupportableMapMock = MockAnnotationSupportableMap()
annotationSupportableStyleMock = MockAnnotationStyleDelegate()
selectionExpectation = expectation(description: "didSelectAnnotation was called")
defaultCoordinate = CLLocationCoordinate2D(latitude: 0, longitude: 0)
}
Expand Down
30 changes: 12 additions & 18 deletions Tests/MapboxMapsTests/Annotations/AnnotationManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,30 @@ import XCTest
import CoreLocation
import Turf

#if canImport(MapboxMaps)
@testable import MapboxMaps
#else
@testable import MapboxMapsAnnotations
@testable import MapboxMapsFoundation
#endif

//swiftlint:disable explicit_acl explicit_top_level_acl
class AnnotationManagerTests: XCTestCase {
final class AnnotationManagerTests: XCTestCase {

var annotationSupportableMapMock: AnnotationSupportableMapMock!
var annotationSupportableStyleMock: AnnotationStyleDelegateMock!
var annotationSupportableMap: MockAnnotationSupportableMap!
var annotationSupportableStyle: MockAnnotationStyleDelegate!
var annotationManager: AnnotationManager!

var defaultCoordinate: CLLocationCoordinate2D!
let defaultCoordinate = CLLocationCoordinate2D(latitude: 0, longitude: 0)

override func setUp() {
// Given
annotationSupportableMapMock = AnnotationSupportableMapMock()
annotationSupportableStyleMock = AnnotationStyleDelegateMock()
annotationManager = AnnotationManager(for: annotationSupportableMapMock,
with: annotationSupportableStyleMock,
annotationSupportableMap = MockAnnotationSupportableMap()
annotationSupportableStyle = MockAnnotationStyleDelegate()
annotationManager = AnnotationManager(for: annotationSupportableMap,
with: annotationSupportableStyle,
options: AnnotationOptions())

defaultCoordinate = CLLocationCoordinate2D(latitude: 0, longitude: 0)
}

override func tearDown() {
annotationSupportableMapMock = nil
annotationSupportableStyleMock = nil
annotationSupportableMap = nil
annotationSupportableStyle = nil
annotationManager = nil
defaultCoordinate = nil
}

// MARK: - Test adding point annotation
Expand All @@ -60,6 +52,8 @@ class AnnotationManagerTests: XCTestCase {
XCTAssertNil(annotationManager.symbolLayer)
XCTAssertNil(annotationManager.lineLayer)
XCTAssertNil(annotationManager.fillLayer)
XCTAssertEqual(annotationSupportableMap.onStub.invocations.count, 1)
XCTAssertEqual(annotationSupportableMap.onStub.parameters.first?.eventType, .mapLoaded)
}

func testLayerIdentifiers() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
import UIKit

#if canImport(MapboxMaps)
@testable import MapboxMaps
#else
@testable import MapboxMapsAnnotations
@testable import MapboxMapsStyle
import MapboxCoreMaps
#endif

//swiftlint:disable explicit_acl explicit_top_level_acl
class AnnotationStyleDelegateMock: AnnotationStyleDelegate {
final class MockAnnotationStyleDelegate: AnnotationStyleDelegate {
//swiftlint:disable function_parameter_count
func setStyleImage(image: UIImage,
with identifier: String,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,9 @@
import UIKit
import CoreLocation
import Turf
import MapboxCoreMaps

#if canImport(MapboxMaps)
@testable import MapboxMaps
#else
@testable import MapboxMapsAnnotations
@testable import MapboxMapsFoundation
import MapboxMapsStyle
#endif

//swiftlint:disable explicit_acl explicit_top_level_acl
class AnnotationSupportableMapMock: UIView, AnnotationSupportableMap {

var observable: Observable? {
nil
}
final class MockAnnotationSupportableMap: UIView, AnnotationSupportableMap {

func visibleFeatures(in rect: CGRect,
styleLayers: Set<String>?,
Expand All @@ -27,4 +14,13 @@ class AnnotationSupportableMapMock: UIView, AnnotationSupportableMap {
let feature = Feature(Point.init(coord))
completion(.success([feature]))
}

struct OnParameters {
var eventType: MapEvents.EventKind
var handler: (Event) -> Void
}
let onStub = Stub<OnParameters, Void>()
func on(_ eventType: MapEvents.EventKind, handler: @escaping (Event) -> Void) {
return onStub.call(with: OnParameters(eventType: eventType, handler: handler))
}
}