feat: put nearby transit in sheet #108
Conversation
BrandonTR
force-pushed
the
br-nearby-transit-sheet
branch
from
c318942 to
fac048e
Compare
There was a problem hiding this comment.
Looks good! Tried it out locally. This does seem tricky to test - potentially UITests would be appropriate for testing this, but we also don't have the foundation in place for writing those tests yet, so I think it is appropriate to hold off on writing an automated test for it.
Potentially one thing to look into - I'm noticing === AttributeGraph: cycle detected through attribute [number] in the logs when switching between detents.
| Replaces the use of the presentationDetents & presentationBackgroundInteraction modifiers | ||
| as they're only available in iOS 16+ for SwiftUI | ||
| **/ | ||
| func partialSheetDetents( |
There was a problem hiding this comment.
👍 cool approach of making this a modifier!
|
|
||
| controller.animateChanges { | ||
| controller.detents = detents.map(\.uiKitDetent) | ||
| controller.prefersScrollingExpandsWhenScrolledToEdge = true |
There was a problem hiding this comment.
suggestion (non-blocking): testing locally, it was a bit unexpected that scrolling made the sheet bigger. Since there isn't anything in the acceptance criteria about how to handle scrolling, may be worth sharing a video with product & design in our slack channel to get their input! I don't see that as needing to block merging of this PR though, if they do want to see any changes it could be addressed separately as a follow-up.
BrandonTR
force-pushed
the
br-nearby-transit-sheet
branch
from
5269dd1 to
5c577b8
Compare
Summary
Ticket: Put nearby transit in a sheet over the map
Putting the existing nearby transit view into a sheet. With SwiftUI's partial sheet implementations only being available in iOS 16+ we needed to backport some functionality from UIKit where it's been available since iOS 15. The only thing not available in iOS 15 is the custom detent option, we are left with just having
.mediumand.large.Testing
Mostly manual testing and accessibility audit. Not sure how to best test this change from an automated standpoint as it's mostly native API interfacing.