8000 feat(iOS): Nearby transit direction grouping UX by EmmaSimon · Pull Request #893 · mbta/mobile_app · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(iOS): Nearby transit direction grouping UX #893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Apr 22, 2025

Conversation

EmmaSimon
Copy link
Contributor
@EmmaSimon EmmaSimon commented Apr 15, 2025

Summary

Ticket: Group by direction |  Barebones Nearby Transit UX

Group by direction in nearby.

Simulator Screenshot - iPhone 16 - 2025-04-18 at 16 29 14 Simulator Screenshot - iPhone 16 - 2025-04-18 at 16 29 28 Simulator Screenshot - iPhone 16 - 2025-04-18 at 16 28 40

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

Testing

  • Existing tests refactored to pass and moved to NearbyTransitViewLegacyTests
  • NearbyTransitViewTests replaces each of the legacy tests with new ones that use direction grouping, except for testLineGrouping, which was failing only in CI for unclear reasons, it was replaced with RouteCardDeparturesTests.testShowsBranchingHeadsigns
  • New basic unit tests added for new direction grouped RouteCard views, RouteCardTests, RouteCardStopHeaderTests, and RouteCardDeparturesTests.

@EmmaSimon EmmaSimon force-pushed the es-ios-initial-direction-grouping-ux branch from 11fafb1 to 37af9f7 Compare April 16, 2025 16:21
@EmmaSimon EmmaSimon force-pushed the es-ios-initial-direction-grouping-ux branch from 3f58363 to b15e28e Compare April 18, 2025 16:30
Also remove a test that was only failing in CI, with a more basic
replacement testShowsBranchingHeadsigns in RouteCardDeparturesTests.
@EmmaSimon EmmaSimon marked this pull request as ready for review April 18, 2025 20:38
@EmmaSimon EmmaSimon requested a review from a team as a code owner April 18, 2025 20:38
@EmmaSimon EmmaSimon requested a review from KaylaBrady April 18, 2025 20:38
Copy link
Collaborator
@KaylaBrady KaylaBrady left a comment

Choose a reason for hiding this comment

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

Nice work! Just a few small Qs

@@ -42,7 +42,8 @@ sealed class LeafFormat {
*/
val route: Route?,
val headsign: String,
val format: UpcomingFormat
val format: UpcomingFormat,
val id: String = "$headsign-$format"
Copy link
Collaborator

Choose a reason for hiding this comment

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

question (non-blocking): Is this required to be unique? I think it might not be in rare cases like bus bunching, where you could have two buses with the same headsign both arriving "Now"

Copy link
Contributor Author
@EmmaSimon EmmaSimon Apr 22, 2025

Choose a reason for hiding this comment

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

Ah, hmm, good point. This was really just to make the ForEach happy, so it would just give one of those "the ID X occurs multiple times within the collection, this will give undefined results" errors, which isn't ideal but generally doesn't seem to cause major problems. Maybe I could tack on a UUID?

8000

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added it!

@EmmaSimon EmmaSimon enabled auto-merge April 22, 2025 14:45
@EmmaSimon EmmaSimon added this pull request to the merge queue Apr 22, 2025
Merged via the queue into main with commit 776ed81 Apr 22, 2025
10 checks passed
@EmmaSimon EmmaSimon deleted the es-ios-initial-direction-grouping-ux branch April 22, 2025 16:34
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.

2 participants
0