fix(Android): reset nearby transit state on pan/recenter #635
Conversation
| composable<SheetRoutes.NearbyTransit> { | ||
| // for ViewModel reasons, must be within the `composable` to be the same instance | ||
| val nearbyViewModel: NearbyTransitViewModel = koinViewModel() |
There was a problem hiding this comment.
Originally, I had nearbyViewModel: NearbyTransitViewModel = koinViewModel() as an argument to NearbyTransitPage, but since navigation states can own ViewModels, calling koinViewModel() outside this composable creates a different instance than the one that's created as an argument in NearbyTransitView, but calling it here gives the same instance. This was absolutely miserable to try to debug.
There was a problem hiding this comment.
That is tricky. To clarify my own understanding, that would be an issue even if we weren't using koin and were instantiating the view models directly, is that right? I haven't been using koinViewModel regularly, and wondering about differences / benefits we get with it aside from test defaults.
There was a problem hiding this comment.
I believe that this is inherent to the Android ViewModel subsystem and not specific to Koin's integration with it. My understanding is that using koinViewModel lets us pass repositories via Koin into the ViewModel without having to define a custom ViewModelProvider.Factory or deal with CreationExtras.
androidApp/src/main/java/com/mbta/tid/mbta_app/android/phoenix/PhoenixSocketWrapper.kt
Show resolved
Hide resolved
androidApp/src/main/java/com/mbta/tid/mbta_app/android/state/subscribeToPredictions.kt
Outdated
Show resolved
Hide resolved
| // TODO reset view model | ||
| nearbyViewModel.reset() |
There was a problem hiding this comment.
Love a good TODO -> done
# Conflicts: # androidApp/src/main/java/com/mbta/tid/mbta_app/android/nearbyTransit/NearbyTransitView.kt
Summary
Ticket: 🤖 | Nearby | Reset state when panning or recentering
I thought this would be easy, and if I knew one more thing about ViewModels it would've been easy, but I did not, so this was really difficult.
iOS
[ ] If you added any user-facing strings on iOS, are they included in Localizable.xcstrings?[ ] Add temporary machine translations, marked "Needs Review"android
[ ] All user-facing strings added to strings resource[ ] Expensive calculations are run inwithContext(Dispatchers.Default)where possibleTesting
Manually verified that the ViewModel gets reset and the stale data does not flash in for a frame or two.