feat: reset nearby transit scroll position #116
Conversation
| @@ -63,6 +65,7 @@ struct NearbyTransitView: View { | |||
| leavePredictions() | |||
| } else if newPhase == .active { | |||
| joinPredictions() | |||
| scrollToTop() | |||
There was a problem hiding this comment.
It seems that this will scrollToTop whether or not the list of nearby routes has changed. Based on the AC, we only need to reset the scroll position if the list of routes has changed. That way, if I scroll to the bottom of the list to see the redline, background the app to check my texts quickly, then come back to the app, it will still be scrolled to the bottom (unless the list of routes has been reordered/changed).
Can this function call depend on the list of routes changing rather than the location + backgrounding?
| @@ -21,31 +21,27 @@ struct NearbyTransitView: View { | |||
| @ObservedObject var scheduleFetcher: ScheduleFetcher | |||
| @ObservedObject var predictionsFetcher: PredictionsFetcher | |||
| @ObservedObject var alertsFetcher: AlertsFetcher | |||
| @State var nearbyWithRealtimeInfo: [StopAssociatedRoute]? | |||
There was a problem hiding this comment.
question (non-blocking): For my own learning, why move this to state?
There was a problem hiding this comment.
Before we were doing this association with every render of the view and if we store the value this way we don't need to re-run the association with realtime info every render, only when one of the onChange/onReceive modifiers triggers it. Just a small optimization
| Note: using the `.top` anchor does not actually bring you to the top | ||
| of the view as expected but using this custom UnitPoint does | ||
| */ | ||
| proxy.scrollTo(id, anchor: UnitPoint(x: 0, y: -1)) |
There was a problem hiding this comment.
question (non-blocking): I notice that when the sheet is at the small detent and I pan the map, I sometimes get stuck scrolled to the top item with the route out of view. Is there a straightforward fix? I don't think fixing that needs to block merging this PR, but maybe worth a check with product about fixing as a follow-up if it seems non-trivial to do.
https://github.com/mbta/mobile_app/assets/31781298/a6ad3535-4e86-43ba-9ed1-c6824fff70f8
There was a problem hiding this comment.
Very strange that this is happening only for the small detent. I tried a few things to see if I can mitigate this but I think this is ultimately a bug with SwiftUI. I enabled the grabber on the sheet for now so users won't get stuck. Gonna ping the team channel about this
BrandonTR
force-pushed
the
br-nearby-transit-scroll-reset
branch
4 times, most recently
from
3d04cd1 to
9636b92
Compare
BrandonTR
force-pushed
the
br-nearby-transit-scroll-reset
branch
from
9636b92 to
df7fe98
Compare
Summary
Ticket: Nearby Transit scroll position shouldn't always persist
Reset the nearby transit scroll position when the map is panned or the app returns from the background. The solution isn't entirely ideal. I would've liked to use a
PassthroughSubjectso that we don't have to clear a state after it is consumed but sinceNearbyTransitViewis re-initialized several times a second the.onReceivemodifier was not getting the ID from the publisher. There was also an issue where the.topanchor was not actually scrolling to the top of the view but a customUnitPointdoes.Testing
Added
ViewInspectorunit tests to ensurescrollPositionis being set when expected.