From 2e6e0088a7adc82cb301df7fffce367cb5eb03f7 Mon Sep 17 00:00:00 2001 From: Emma Simon Date: Thu, 11 Apr 2024 16:57:50 -0400 Subject: [PATCH] fix: Load correct nearby location on launch When the app was started with location permissions already granted, nearby transit was instead loading the default camera location, rather that the phone's current GPS location. The NearbyTransitLocationProvider was implemented in a silly way where it was effectively static, and on app startup, `currentLocation` and `viewportProvider.viewport` were both changing and running their `onChange` at the same time, so the current location was getting overridden by the default camera state. --- .../NearbyTransitLocationProvider.swift | 54 ++++++++- .../NearbyTransit/NearbyTransitPageView.swift | 40 +++---- .../NearbyTransit/NearbyTransitView.swift | 10 +- .../Views/NearbyTransitViewTests.swift | 103 ++++-------------- 4 files changed, 92 insertions(+), 115 deletions(-) diff --git a/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitLocationProvider.swift b/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitLocationProvider.swift index 4f573c249..dbe8bf4be 100644 --- a/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitLocationProvider.swift +++ b/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitLocationProvider.swift @@ -9,9 +9,9 @@ import CoreLocation class NearbyTransitLocationProvider: ObservableObject { - let currentLocation: CLLocationCoordinate2D? - let cameraLocation: CLLocationCoordinate2D - let isFollowing: Bool + var currentLocation: CLLocationCoordinate2D? + var cameraLocation: CLLocationCoordinate2D + var isFollowing: Bool @Published var location: CLLocationCoordinate2D @@ -20,6 +20,52 @@ class NearbyTransitLocationProvider: ObservableObject { self.cameraLocation = cameraLocation self.isFollowing = isFollowing - location = if isFollowing, currentLocation != nil { currentLocation! } else { cameraLocation } + location = Self.resolveLocation(currentLocation, cameraLocation, isFollowing) + handleFollowingCamera() + } + + static func resolveLocation( + _ currentLocation: CLLocationCoordinate2D?, + _ cameraLocation: CLLocationCoordinate2D, + _ isFollowing: Bool + ) -> CLLocationCoordinate2D { + if isFollowing, currentLocation != nil { currentLocation! } else { cameraLocation } + } + + func updateCurrentLocation(_ newLocation: CLLocationCoordinate2D?) { + currentLocation = newLocation + handleFollowingCamera() + updateLocation() + } + + func updateCameraLocation(_ newLocation: CLLocationCoordinate2D) { + if cameraLocation.isRoughlyEqualTo(newLocation) { + return + } + cameraLocation = newLocation + updateLocation() + } + + func updateIsFollowing(_ newFollowing: Bool, withCameraLocation newLocation: CLLocationCoordinate2D? = nil) { + isFollowing = newFollowing + if newLocation != nil { + cameraLocation = newLocation! + } + handleFollowingCamera() + updateLocation() + } + + private func updateLocation() { + location = Self.resolveLocation(currentLocation, cameraLocation, isFollowing) + } + + /* + Reset camera location to current location when viewport is following. + This prevents loading stale camera locations when the viewport stops following. + */ + private func handleFollowingCamera() { + if isFollowing, currentLocation != nil { + cameraLocation = currentLocation! + } } } diff --git a/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitPageView.swift b/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitPageView.swift index 26ff60a01..01f2dff5e 100644 --- a/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitPageView.swift +++ b/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitPageView.swift @@ -14,7 +14,7 @@ import SwiftUI @_spi(Experimental) import MapboxMaps struct NearbyTransitPageView: View { - let currentLocation: CLLocationCoordinate2D? + var currentLocation: CLLocationCoordinate2D? @ObservedObject var globalFetcher: GlobalFetcher @ObservedObject var nearbyFetcher: NearbyFetcher @ObservedObject var scheduleFetcher: ScheduleFetcher @@ -23,7 +23,9 @@ struct NearbyTransitPageView: View { @ObservedObject var alertsFetcher: AlertsFetcher @State var cancellables: [AnyCancellable] - @State var locationProvider: NearbyTransitLocationProvider + @StateObject var locationProvider: NearbyTransitLocationProvider + + let inspection = Inspection() init( currentLocation: CLLocationCoordinate2D?, @@ -43,16 +45,16 @@ struct NearbyTransitPageView: View { self.alertsFetcher = alertsFetcher cancellables = .init() - locationProvider = .init( + _locationProvider = StateObject(wrappedValue: .init( currentLocation: currentLocation, cameraLocation: viewportProvider.cameraState.center, isFollowing: viewportProvider.viewport.isFollowing - ) + )) } var body: some View { NearbyTransitView( - locationProvider: locationProvider, + location: locationProvider.location, globalFetcher: globalFetcher, nearbyFetcher: nearbyFetcher, scheduleFetcher: scheduleFetcher, @@ -64,36 +66,20 @@ struct NearbyTransitPageView: View { viewportProvider.$cameraState .debounce(for: .seconds(0.5), scheduler: DispatchQueue.main) .sink { newCameraState in - let shouldUpdateLocation = !viewportProvider.viewport.isFollowing && - !locationProvider.location.isRoughlyEqualTo(newCameraState.center) - if shouldUpdateLocation { - locationProvider = .init( - currentLocation: currentLocation, - cameraLocation: newCameraState.center, - isFollowing: viewportProvider.viewport.isFollowing - ) - } + locationProvider.updateCameraLocation(newCameraState.center) } ) } .onChange(of: currentLocation) { newLocation in - let shouldUpdateLocation = viewportProvider.viewport.isFollowing - && !locationProvider.location.isRoughlyEqualTo(newLocation) - if shouldUpdateLocation { - locationProvider = .init( - currentLocation: newLocation, - cameraLocation: viewportProvider.cameraState.center, - isFollowing: viewportProvider.viewport.isFollowing - ) - } + locationProvider.updateCurrentLocation(newLocation) } .onChange(of: viewportProvider.viewport) { newViewport in - locationProvider = .init( - currentLocation: currentLocation, - cameraLocation: viewportProvider.cameraState.center, - isFollowing: newViewport.isFollowing + locationProvider.updateIsFollowing( + newViewport.isFollowing, + withCameraLocation: viewportProvider.cameraState.center ) } + .onReceive(inspection.notice) { inspection.visit(self, $0) } .navigationTitle("Nearby Transit") .navigationBarTitleDisplayMode(.inline) } diff --git a/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitView.swift b/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitView.swift index 8d5cc2ee0..cc962e1ba 100644 --- a/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitView.swift +++ b/iosApp/iosApp/Pages/NearbyTransit/NearbyTransitView.swift @@ -15,7 +15,7 @@ import SwiftUI struct NearbyTransitView: View { @Environment(\.scenePhase) private var scenePhase - @ObservedObject var locationProvider: NearbyTransitLocationProvider + var location: CLLocationCoordinate2D @ObservedObject var globalFetcher: GlobalFetcher @ObservedObject var nearbyFetcher: NearbyFetcher @ObservedObject var scheduleFetcher: ScheduleFetcher @@ -44,14 +44,14 @@ struct NearbyTransitView: View { } } .onAppear { - getNearby(location: locationProvider.location) + getNearby(location: location) joinPredictions() didAppear?(self) } .onChange(of: globalFetcher.response) { _ in - getNearby(location: locationProvider.location) + getNearby(location: location) } - .onChange(of: locationProvider.location) { newLocation in + .onChange(of: location) { newLocation in getNearby(location: newLocation) } .onChange(of: nearbyFetcher.nearbyByRouteAndStop) { _ in @@ -76,7 +76,7 @@ struct NearbyTransitView: View { } .replaceWhen(nearbyFetcher.errorText) { errorText in IconCard(iconName: "network.slash", details: errorText) - .refreshable(nearbyFetcher.loading) { getNearby(location: locationProvider.location) } + .refreshable(nearbyFetcher.loading) { getNearby(location: location) } } } diff --git a/iosApp/iosAppTests/Views/NearbyTransitViewTests.swift b/iosApp/iosAppTests/Views/NearbyTransitViewTests.swift index a7fc2e61b..21ca3056a 100644 --- a/iosApp/iosAppTests/Views/NearbyTransitViewTests.swift +++ b/iosApp/iosAppTests/Views/NearbyTransitViewTests.swift @@ -25,11 +25,7 @@ final class NearbyTransitViewTests: XCTestCase { func testPending() throws { let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: nil, - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: ViewportProvider.defaultCenter, globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: NearbyFetcher(backend: IdleBackend()), scheduleFetcher: .init(backend: IdleBackend()), @@ -63,11 +59,7 @@ final class NearbyTransitViewTests: XCTestCase { let getNearbyExpectation = expectation(description: "getNearby") var sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: FakeGlobalFetcher(), nearbyFetcher: FakeNearbyFetcher(getNearbyExpectation: getNearbyExpectation), scheduleFetcher: .init(backend: IdleBackend()), @@ -154,11 +146,7 @@ final class NearbyTransitViewTests: XCTestCase { func testRoutePatternsGroupedByRouteAndStop() throws { let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: Route52NearbyFetcher(), scheduleFetcher: .init(backend: IdleBackend()), @@ -240,11 +228,7 @@ final class NearbyTransitViewTests: XCTestCase { } let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: Route52NearbyFetcher(), scheduleFetcher: FakeScheduleFetcher(objects), @@ -319,11 +303,7 @@ final class NearbyTransitViewTests: XCTestCase { let testFormatter = DateFormatter() testFormatter.timeStyle = .short let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: Route52NearbyFetcher(), scheduleFetcher: .init(backend: IdleBackend()), @@ -379,11 +359,7 @@ final class NearbyTransitViewTests: XCTestCase { let nearbyFetcher = Route52NearbyFetcher() let predictionsFetcher = FakePredictionsFetcher(sawmillAtWalshExpectation: sawmillAtWalshExpectation, lechmereExpectation: lechmereExpectation) let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: nearbyFetcher, scheduleFetcher: .init(backend: IdleBackend()), @@ -410,11 +386,7 @@ final class NearbyTransitViewTests: XCTestCase { let predictionsFetcher = PredictionsFetcher(socket: MockSocket()) let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: Route52NearbyFetcher(), scheduleFetcher: .init(backend: IdleBackend()), @@ -471,11 +443,7 @@ final class NearbyTransitViewTests: XCTestCase { let nearbyFetcher = Route52NearbyFetcher() let predictionsFetcher = FakePredictionsFetcher(joinExpectation: joinExpectation, leaveExpectation: leaveExpectation) let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: nearbyFetcher, scheduleFetcher: .init(backend: IdleBackend()), @@ -516,11 +484,7 @@ final class NearbyTransitViewTests: XCTestCase { let nearbyFetcher = Route52NearbyFetcher() let predictionsFetcher = FakePredictionsFetcher(joinExpectation: joinExpectation, leaveExpectation: leaveExpectation) let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: nearbyFetcher, scheduleFetcher: .init(backend: IdleBackend()), @@ -564,11 +528,7 @@ final class NearbyTransitViewTests: XCTestCase { let nearbyFetcher = Route52NearbyFetcher() let predictionsFetcher = FakePredictionsFetcher(joinExpectation: joinExpectation, leaveExpectation: leaveExpectation) let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: nearbyFetcher, scheduleFetcher: .init(backend: IdleBackend()), @@ -595,11 +555,7 @@ final class NearbyTransitViewTests: XCTestCase { } let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: FakeNearbyFetcher(), scheduleFetcher: .init(backend: IdleBackend()), @@ -625,11 +581,7 @@ final class NearbyTransitViewTests: XCTestCase { } let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: FakeNearbyFetcher(), scheduleFetcher: .init(backend: IdleBackend()), @@ -657,40 +609,37 @@ final class NearbyTransitViewTests: XCTestCase { let getNearbyExpectation = expectation(description: "getNearby") getNearbyExpectation.expectedFulfillmentCount = 2 - let fakeFetcher = FakeNearbyFetcher(getNearbyExpectation: getNearbyExpectation) + let nearbyFetcher = FakeNearbyFetcher(getNearbyExpectation: getNearbyExpectation) let currentLocation = CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78) - let locationProvider: NearbyTransitLocationProvider = .init( - currentLocation: currentLocation, - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ) let globalFetcher = GlobalFetcher(backend: IdleBackend()) globalFetcher.response = .init(patternIdsByStop: [:], routes: [:], routePatterns: [:], stops: [:], trips: [:]) - let sut = NearbyTransitView( - locationProvider: locationProvider, + var sut = NearbyTransitPageView( + currentLocation: currentLocation, globalFetcher: globalFetcher, - nearbyFetcher: fakeFetcher, + nearbyFetcher: nearbyFetcher, scheduleFetcher: .init(backend: IdleBackend()), predictionsFetcher: .init(socket: MockSocket()), + viewportProvider: .init(viewport: .followPuck(zoom: ViewportProvider.defaultZoom)), alertsFetcher: .init(socket: MockSocket()) ) let newLocation = CLLocationCoordinate2D(latitude: 0.0, longitude: 0.0) + let appearancePublisher = PassthroughSubject() let hasAppeared = sut.inspection.inspect(after: 0.2) { view in XCTAssertEqual(try view.actualView().nearbyFetcher.loadedLocation, currentLocation) + try view.actualView().locationProvider.updateCurrentLocation(newLocation) + appearancePublisher.send(true) } - let hasChangedLocation = sut.inspection.inspect(onReceive: locationProvider.$location.dropFirst()) { view in - XCTAssertEqual(try view.actualView().locationProvider.location, newLocation) + let hasChangedLocation = sut.inspection.inspect(onReceive: appearancePublisher, after: 0.2) { view in + XCTAssertEqual(try view.actualView().nearbyFetcher.loadedLocation, newLocation) } ViewHosting.host(view: sut) - wait(for: [hasAppeared], timeout: 3) - locationProvider.location = newLocation - wait(for: [getNearbyExpectation, hasChangedLocation], timeout: 3) + wait(for: [hasAppeared, getNearbyExpectation, hasChangedLocation], timeout: 3) } func testLocationProviderResolvesProperly() { @@ -724,11 +673,7 @@ final class NearbyTransitViewTests: XCTestCase { alertsFetcher.alerts = AlertsStreamDataResponse(objects: objects) let sut = NearbyTransitView( - locationProvider: .init( - currentLocation: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), - cameraLocation: ViewportProvider.defaultCenter, - isFollowing: true - ), + location: CLLocationCoordinate2D(latitude: 12.34, longitude: -56.78), globalFetcher: .init(backend: IdleBackend()), nearbyFetcher: Route52NearbyFetcher(), scheduleFetcher: scheduleFetcher,