Skip to content

Conversation

@EmmaSimon
Copy link
Contributor

@EmmaSimon EmmaSimon commented Sep 10, 2025

Summary

Ticket: Move StopDetailsViewModel to shared code, follow up to #1273

This rips out the iOS specific StopDetailsViewModel and replaces it with the shared stop and trip details VMs. I pulled out some VM specific handling out into modifiers, and had to rearrange a lot of the state and params in stop details, but functionality should be exactly the same.

iOS
- [ ] If you added any user-facing strings on iOS, are they included in Localizable.xcstrings?
- [ ] Add temporary machine translations, marked "Needs Review"

Testing

Deleted iOS VM tests that are now covered by shared tests, and a few irrelevant tests that were testing VM functionality from the views. Got all existing tests passing with the new mock VMs

@EmmaSimon EmmaSimon requested a review from a team as a code owner September 10, 2025 19:14
.onChange(of: nearbyVM.alerts) { _ in
handleGlobalMapDataChange(now: now)
}
.onChange(of: nearbyVM.routeCardData) { _ in
Copy link
Contributor Author

@EmmaSimon EmmaSimon Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, nearbyVM.routeCardData was being set with the nearby route card data, but also, kinda inappropriately, with the stop details rcd. Since the map only cares about the stop details rcd, it was just doing this nav filtering to distinguish them. Now, this uses the molecule RouteCardDataViewModel directly, which is only updated with stop details rcd (the name isn't the best tbh). I could probably remove the stop details nav check now, not sure if it's really adding any additional safety, unless we start updating the RouteCardDataViewModel also with nearby rcd.

}
.task {
for await model in stopDetailsVM.models {
alertSummaries = model.alertSummaries as? [String: AlertSummary?] ?? [:]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

model.alertSummaries has a type of [String: Any], which was slightly awkward to work around.

@EmmaSimon EmmaSimon force-pushed the es-stop-details-vm-ios branch from 7f3a02b to 339caf7 Compare September 10, 2025 20:08
Copy link
Member

@boringcactus boringcactus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@EmmaSimon EmmaSimon force-pushed the es-stop-details-vm-ios branch from ec1f8db to 77fca68 Compare September 11, 2025 19:48
@EmmaSimon EmmaSimon enabled auto-merge September 11, 2025 19:56
@EmmaSimon EmmaSimon added this pull request to the merge queue Sep 11, 2025
Merged via the queue into main with commit 383a70e Sep 11, 2025
11 checks passed
@EmmaSimon EmmaSimon deleted the es-stop-details-vm-ios branch September 11, 2025 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants