Skip to content

feat(favorites): Group by stop UI part 1#1835

Merged
KaylaBrady merged 10 commits into
mainfrom
kb-group-by-stop-ui
Jul 7, 2026
Merged

feat(favorites): Group by stop UI part 1#1835
KaylaBrady merged 10 commits into
mainfrom
kb-group-by-stop-ui

Conversation

@KaylaBrady

@KaylaBrady KaylaBrady commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

Ticket: Group favorites by stop | Favorites View UI

What is this PR for?

  • Adjusts padding on the stop icon
  • Adjusts formatting of route pill w/ warning icon

Not included:

  • This doesn't adjust the placement of the warning icon for branched routes; that will involve some data structure changes, so I'll look into that in a follow-up
  • Outstanding question to design about whether we want to change anything to accommodate longer bus pills

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 in alphabetical order
- [ ] Expensive calculations are run in withContext(Dispatchers.Default) where possible (ideally in shared code)

Testing

What testing have you done?

  • Ran locally in iOS & android
platform group by stop group by route
ios image image
android image image

@KaylaBrady KaylaBrady requested a review from a team as a code owner July 2, 2026 19:52
@KaylaBrady KaylaBrady requested a review from thedecaftony July 2, 2026 19:52
@KaylaBrady KaylaBrady force-pushed the kb-group-by-stop-ui branch from 43f3376 to 38005e3 Compare July 2, 2026 20:08
Row(horizontalArrangement = Arrangement.spacedBy(-8.dp)) {
RoutePill(pillDecoration.route, line = null, RoutePillType.FlexCompact)

predictions.warningAlert?.let { warningAlert ->

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick: It would reduce the code a bit to have this on a constant and put it here and on the else block

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll address this in the follow-up PR where I need to make this section a bit more reusable anyway!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Actually ended up including in this one

@thedecaftony thedecaftony left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now favorites will be group by stop, are we planning to give the pill and text the same padding space so they look more align with each other?

if (route.id.idText.startsWith("Green-")) {
when (type) {
Type.Fixed -> Content.Text(route.longName.replace("Green Line ", "GL "))
Type.Fixed,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Note: This "GL B" rounded fixed-width pill was never actually used anywhere because we'd instead use the "GL" fixed-width line pill representing the whole line instead of just one GL route.

With these changes, the unused "GL B" rounded fixed-width pill is effectively replaced by the "B" circle pill.

@KaylaBrady

Copy link
Copy Markdown
Collaborator Author

Now favorites will be group by stop, are we planning to give the pill and text the same padding space so they look more align with each other?

@thedecaftony yes, based on this thread with design! I just pushed a change for that, would you mind re-reviewing this PR? I updated the description with images after this change.

@KaylaBrady KaylaBrady requested a review from thedecaftony July 6, 2026 18:42
@KaylaBrady KaylaBrady force-pushed the kb-group-by-stop-ui branch from 00e34c0 to 1ab84e3 Compare July 6, 2026 19:24
@KaylaBrady KaylaBrady enabled auto-merge July 7, 2026 13:49
@KaylaBrady KaylaBrady added this pull request to the merge queue Jul 7, 2026
Merged via the queue into main with commit a81c84f Jul 7, 2026
12 checks passed
@KaylaBrady KaylaBrady deleted the kb-group-by-stop-ui branch July 7, 2026 20:58
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