feat(iOS): recenter on vehicle from new stop details #580
Conversation
| .navigationStack.lastSafe(), | ||
| let selectedVehicle = mapVM.selectedVehicle, let globalData = mapVM.globalData, | ||
| let stop = nearbyVM.getTargetStop(global: globalData), let routeId = stopFilter?.routeId, | ||
| let route = globalData.routes[routeId] else { return nil } |
There was a problem hiding this comment.
This doesn't work for GL because its filter "routeId" is actually the line ID. I think you could use the one from the vehicle instead?
There was a problem hiding this comment.
I thought about using the vehicle's route ID but decided not to because the vehicle's route may not actually be the currently selected route. This was silly, though, because buses will not spontaneously become trains. However, if the vehicle does not have a route assigned at all, it'd be nice if this button kept working; I'm not sure what conditions would cause that to happen, but there appear to currently be zero vehicles with no route, so it should be rare enough that we can get away with falling back to the stop filter route ID.
There was a problem hiding this comment.
Yeah I wonder if it's possible for the vehicles to have a null route when coming from a garage or being in between routes. I ran into basically the same issue with the trip details card and created TripRouteAccents for just the general display related components of a route, maybe it's worth splitting that out into its own file and using it here too, though it seems much less risky in this case that someone will get confused and attempt to use the route for something that requires that it be the correct trip route.
| }.frame(maxWidth: .infinity, alignment: .topTrailing) | ||
| } | ||
| if !searchObserver.isSearching, !viewportProvider.viewport.isOverview, | ||
| let (routeType, selectedVehicle, stop) = recenterOnVehicleButtonInfo() { |
There was a problem hiding this comment.
This causes some weird behavior when you tap between single route+direction stops on the map, when the viewport is moving to the new stop overview, the vehicle recenter button flickers in for a second. Not certain why, but I think it might be when the old selected vehicle is still set but the viewport is set to the position of the new stop, before the auto trip filter has loaded and set the new vehicle, which sets the viewport to overview once it changes. You might be able to include a check that selectedVehicle.id == tripFilter.vehicleId?
There was a problem hiding this comment.
That appears to work. Good catch.
Summary
Ticket: Combined Stop + Trip Details: Recenter on vehicle button
Moving the map's global data into the MapViewModel felt a little bit weird, but making the ContentView load its own global data would've felt weirder. I think the Android approach of putting these buttons structurally within the map is nicer, but I don't want to refactor iOS to match that in this PR.
iOS
android
Testing
Manually validated that the button uses the subway/bus/CR icon for subway/bus/CR routes (since ferries don't get realtime info right now, I can't test that, but presumably it'd work if that changed). No unit test for the vehicle recenter button, since there's no unit test for the self recenter button.