feat: Update stop details navigation to match spec (#586)

* feat: Update stop details navigation to match spec

* test: Add tests for stop details nav behavior

* test: Remove unfiltered back button tests

* fix: Move check into nav stack extension

iosApp/iosApp/Pages/StopDetails/StopDetailsFilteredView.swift

@@ -181,7 +181,7 @@ struct StopDetailsFilteredView: View {
                 stop: stop,
                 pinned: pinned,
                 onPin: toggledPinnedRoute,
-                onClose: { nearbyVM.navigationStack.removeAll() }
+                onClose: { nearbyVM.goBack() }
             )
             if nearbyVM.showDebugMessages {
                 DebugView {

iosApp/iosApp/Pages/StopDetails/StopDetailsUnfilteredView.swift

@@ -66,7 +66,6 @@ struct StopDetailsUnfilteredView: View {
                 VStack(spacing: 16) {
                     SheetHeader(
                         title: stop?.name ?? "Invalid Stop",
-                        onBack: nearbyVM.navigationStack.count > 1 ? { nearbyVM.goBack() } : nil,
                         onClose: { nearbyVM.navigationStack.removeAll() }
                     )
                     if nearbyVM.showDebugMessages {

iosApp/iosApp/Utils/SheetNavigationStackEntry.swift

@@ -101,8 +101,16 @@ extension [SheetNavigationStackEntry] {
             if case let .legacyStopDetails(stop, _) = self.last {
                 _ = self.popLast()
                 self.append(.legacyStopDetails(stop, newValue))
-            } else if case let .stopDetails(stopId: stopId, stopFilter: _, tripFilter: _) = self.last {
-                _ = self.popLast()
+            } else if case let .stopDetails(
+                stopId: stopId,
+                stopFilter: lastFilter,
+                tripFilter: _
+            ) = self.last {
+                // When the stop filter changes, we want a new entry to be added (i.e. no pop) only when
+                // you're on the unfiltered (lastFilter == nil) page, but if there is already a filter,
+                // the entry with the old filter should be popped and replaced with the new value.
+                if lastFilter != nil { _ = self.popLast() }
+                if self.shouldSkipStopFilterUpdate(newStop: stopId, newFilter: newValue) { return }
                 self.append(.stopDetails(stopId: stopId, stopFilter: newValue, tripFilter: nil))
             }
         }
@@ -151,4 +159,18 @@ extension [SheetNavigationStackEntry] {
     func lastSafe() -> SheetNavigationStackEntry {
         self.last ?? .nearby
     }
+
+    func shouldSkipStopFilterUpdate(newStop: String, newFilter: StopDetailsFilter?) -> Bool {
+        // If the new filter is nil and there is already a nil filter in the stack for the same stop ID,
+        // we don't want a duplicate unfiltered entry, so skip appending a new one
+        if case let .stopDetails(
+            stopId: lastStop,
+            stopFilter: lastFilter,
+            tripFilter: _
+        ) = self.last {
+            lastStop == newStop && newFilter == nil && lastFilter == nil
+        } else {
+            false
+        }
+    }
 }

iosApp/iosApp/ViewModels/NearbyViewModel.swift

@@ -116,14 +116,27 @@ class NearbyViewModel: ObservableObject {
             )
             pushNavEntry(.stopDetails(stopId: stopId, stopFilter: stopFilter, tripFilter: tripFilter))
         } else if case let .legacyStopDetails(targetStop, _) = entry,
-                  case let .legacyStopDetails(currentStop, _) = navigationStack.last,
-                  targetStop == currentStop {
+                  case let .legacyStopDetails(lastStop, _) = navigationStack.last,
+                  targetStop == lastStop {
             _ = navigationStack.popLast()
             navigationStack.append(entry)
-        } else if case let .stopDetails(stopId: targetStop, stopFilter: _, tripFilter: _) = entry,
-                  case let .stopDetails(stopId: currentStop, stopFilter: _, tripFilter: _) = navigationStack.last,
-                  targetStop == currentStop {
-            _ = navigationStack.popLast()
+        } else if
+            case let .stopDetails(
+                stopId: targetStop,
+                stopFilter: newFilter,
+                tripFilter: _
+            ) = entry,
+            case let .stopDetails(
+                stopId: lastStop,
+                stopFilter: lastFilter,
+                tripFilter: _
+            ) = navigationStack.last,
+            targetStop == lastStop {
+            // When the stop filter changes, we want a new entry to be added (i.e. no pop) only when
+            // you're on the unfiltered (lastFilter == nil) page, but if there is already a filter,
+            // the entry with the old filter should be popped and replaced with the new value.
+            if lastFilter != nil { _ = navigationStack.popLast() }
+            if navigationStack.shouldSkipStopFilterUpdate(newStop: targetStop, newFilter: newFilter) { return }
             navigationStack.append(entry)
         } else {
             navigationStack.append(entry)

iosApp/iosAppTests/Pages/StopDetails/StopDetailsPageTests.swift

@@ -115,51 +115,6 @@ final class StopDetailsPageTests: XCTestCase {
         wait(for: [exp], timeout: 30)
     }
 
-    func testBackButton() throws {
-        let objects = ObjectCollectionBuilder()
-        let stop = objects.stop { _ in }
-
-        class FakeNearbyVM: NearbyViewModel {
-            let backExp: XCTestExpectation
-            init(_ backExp: XCTestExpectation) {
-                self.backExp = backExp
-                super.init(combinedStopAndTrip: true)
-            }
-
-            override func goBack() {
-                backExp.fulfill()
-            }
-        }
-
-        let backExp = XCTestExpectation(description: "goBack called")
-        let nearbyVM = FakeNearbyVM(backExp)
-        nearbyVM.navigationStack = [
-            .stopDetails(stopId: stop.id, stopFilter: nil, tripFilter: nil),
-            .stopDetails(stopId: stop.id, stopFilter: nil, tripFilter: nil),
-            .stopDetails(stopId: stop.id, stopFilter: nil, tripFilter: nil),
-        ]
-
-        let stopDetailsVM = StopDetailsViewModel(
-            predictionsRepository: MockPredictionsRepository(),
-            schedulesRepository: MockScheduleRepository()
-        )
-
-        let sut = StopDetailsPage(
-            stopId: stop.id,
-            stopFilter: nil,
-            tripFilter: nil,
-            errorBannerVM: .init(),
-            nearbyVM: nearbyVM,
-            mapVM: .init(),
-            stopDetailsVM: stopDetailsVM,
-            viewportProvider: .init()
-        )
-
-        try sut.inspect().find(viewWithAccessibilityLabel: "Back").button().tap()
-
-        wait(for: [backExp], timeout: 2)
-    }
-
     func testCloseButton() throws {
         let objects = ObjectCollectionBuilder()
         let stop = objects.stop { _ in }

iosApp/iosAppTests/Pages/StopDetails/StopDetailsViewTests.swift

@@ -145,60 +145,6 @@ final class StopDetailsViewTests: XCTestCase {
         XCTAssert(nearbyVM.navigationStack.isEmpty)
     }
 
-    func testBackButtonGoesBack() throws {
-        let objects = ObjectCollectionBuilder()
-        let stop = objects.stop { _ in }
-
-        let initialNavStack: [SheetNavigationStackEntry] = [
-            .stopDetails(stopId: stop.id, stopFilter: nil, tripFilter: nil),
-            .tripDetails(tripId: "", vehicleId: "", target: nil, routeId: "", directionId: 0),
-            .stopDetails(stopId: stop.id, stopFilter: nil, tripFilter: nil),
-        ]
-        let nearbyVM: NearbyViewModel = .init(navigationStack: initialNavStack)
-        let sut = StopDetailsView(
-            stopId: stop.id,
-            stopFilter: nil,
-            tripFilter: nil,
-            setStopFilter: { _ in },
-            setTripFilter: { _ in },
-            departures: nil,
-            now: Date.now,
-            errorBannerVM: .init(),
-            nearbyVM: nearbyVM,
-            mapVM: .init(),
-            stopDetailsVM: .init()
-        )
-
-        ViewHosting.host(view: sut)
-        try? sut.inspect().find(viewWithAccessibilityLabel: "Back").button().tap()
-        XCTAssertEqual(initialNavStack.dropLast(), nearbyVM.navigationStack)
-    }
-
-    func testBackButtonHiddenWhenNearbyIsBack() throws {
-        let objects = ObjectCollectionBuilder()
-        let stop = objects.stop { _ in }
-
-        let nearbyVM: NearbyViewModel = .init(navigationStack: [
-            .stopDetails(stopId: stop.id, stopFilter: nil, tripFilter: nil),
-        ])
-        let sut = StopDetailsView(
-            stopId: stop.id,
-            stopFilter: nil,
-            tripFilter: nil,
-            setStopFilter: { _ in },
-            setTripFilter: { _ in },
-            departures: nil,
-            now: Date.now,
-            errorBannerVM: .init(),
-            nearbyVM: nearbyVM,
-            mapVM: .init(),
-            stopDetailsVM: .init()
-        )
-
-        ViewHosting.host(view: sut)
-        XCTAssertNil(try? sut.inspect().find(viewWithAccessibilityLabel: "Back"))
-    }
-
     func testDebugModeNotShownByDefault() throws {
         let objects = ObjectCollectionBuilder()
         let stop = objects.stop { stop in

iosApp/iosAppTests/Utils/SheetNavigationStackEntryTests.swift

@@ -21,7 +21,7 @@ final class SheetNavigationStackEntryTests: XCTestCase {
         XCTAssertEqual(stack, [])
     }
 
-    func testLastFilterShallow() throws {
+    func testLastFilterShallowLegacy() throws {
         let stop = ObjectCollectionBuilder.Single.shared.stop { _ in }
         var stack: [SheetNavigationStackEntry] = [.legacyStopDetails(stop, nil)]
 
@@ -38,7 +38,7 @@ final class SheetNavigationStackEntryTests: XCTestCase {
         XCTAssertEqual(stack.lastStopDetailsFilter, nil)
     }
 
-    func testLastFilterDeep() throws {
+    func testLastFilterDeepLegacy() throws {
         let stop = ObjectCollectionBuilder.Single.shared.stop { _ in }
         let otherStop = ObjectCollectionBuilder.Single.shared.stop { _ in }
         let previousEntries: [SheetNavigationStackEntry] = [
@@ -67,7 +67,7 @@ final class SheetNavigationStackEntryTests: XCTestCase {
         XCTAssertEqual(stack.lastStopDetailsFilter, nil)
     }
 
-    func testLastFilterNotTop() throws {
+    func testLastFilterNotTopLegacy() throws {
         let stop = ObjectCollectionBuilder.Single.shared.stop { _ in }
         var stack: [SheetNavigationStackEntry] = [
             .legacyStopDetails(stop, .init(routeId: "A", directionId: 1)),
@@ -108,7 +108,7 @@ final class SheetNavigationStackEntryTests: XCTestCase {
         XCTAssertEqual(alertEntry.coverItemIdentifiable()!.id, "0")
     }
 
-    func testNavStackLastStop() throws {
+    func testNavStackLastStopLegacy() throws {
         let stop = ObjectCollectionBuilder.Single.shared.stop { _ in }
         let stopEntry: SheetNavigationStackEntry = .legacyStopDetails(stop, .init(routeId: "A", directionId: 1))
         let tripEntry: SheetNavigationStackEntry = .tripDetails(
@@ -166,4 +166,37 @@ final class SheetNavigationStackEntryTests: XCTestCase {
         stack.append(tripEntry)
         XCTAssertEqual(stop.id, stack.lastStopId)
     }
+
+    func testLastFilterSet() throws {
+        let stop = ObjectCollectionBuilder.Single.shared.stop { _ in }
+        var stack: [SheetNavigationStackEntry] = [.stopDetails(stopId: stop.id, stopFilter: nil, tripFilter: nil)]
+
+        let filter1 = StopDetailsFilter(routeId: "A", directionId: 1)
+        stack.lastStopDetailsFilter = filter1
+
+        XCTAssertEqual([
+            .stopDetails(stopId: stop.id, stopFilter: nil, tripFilter: nil),
+            .stopDetails(stopId: stop.id, stopFilter: filter1, tripFilter: nil),
+        ], stack)
+        XCTAssertEqual(filter1, stack.lastStopDetailsFilter)
+
+        let filter2 = StopDetailsFilter(routeId: "A", directionId: 0)
+        stack.lastStopDetailsFilter = filter2
+
+        XCTAssertEqual([
+            .stopDetails(stopId: stop.id, stopFilter: nil, tripFilter: nil),
+            .stopDetails(stopId: stop.id, stopFilter: filter2, tripFilter: nil),
+        ], stack)
+        XCTAssertEqual(filter2, stack.lastStopDetailsFilter)
+
+        stack.lastStopDetailsFilter = nil
+
+        XCTAssertEqual([.stopDetails(stopId: stop.id, stopFilter: nil, tripFilter: nil)], stack)
+        XCTAssertEqual(nil, stack.lastStopDetailsFilter)
+
+        stack.lastStopDetailsFilter = nil
+
+        XCTAssertEqual([.stopDetails(stopId: stop.id, stopFilter: nil, tripFilter: nil)], stack)
+        XCTAssertEqual(nil, stack.lastStopDetailsFilter)
+    }
 }