Make 'Zoom in for stops' warning clickable to auto-zoom (#1062)#1063
Make 'Zoom in for stops' warning clickable to auto-zoom (#1062)#1063Vedd-Patel wants to merge 1 commit intoOneBusAway:mainfrom
Conversation
aaronbrethorst
left a comment
There was a problem hiding this comment.
Great idea turning the passive "Zoom in for stops" banner into an actionable tap target, Ved — this is the kind of small UX improvement that makes a real difference for users. The implementation approach is sound: dispatching from a single gesture recognizer based on state is clean. A couple of things to fix up before merging.
Critical Issues
None.
Important Issues
- Indentation regression in
mapRegionManagerShowZoomInStatus(MapViewController.swift:931-938): The method body is indented with 12 spaces and the closing brace with 8, but the rest of the file (and the method signature itself) uses standard 4-space indentation for method bodies. Compare with the adjacentmapRegionManager(_:didRemoveUserAnnotation:)at line 926. Please re-indent to match the file's convention.
Fit-and-Finish
-
Magic number
0.01(MapViewController.swift:211-212): The zoom span of 0.01 degrees is hardcoded. Consider extracting this to a named constant (e.g.,private static let zoomInForStopsSpan = 0.01) so the intent is clear and the value is easy to tune later. -
Unnecessary
self.prefix (MapViewController.swift:932):self.isShowingZoomWarning = showStatusdoesn't need theself.prefix — there's no naming ambiguity here, and the surrounding code in this file doesn't useself.outside of closures.
Strengths
- The dispatcher pattern in
handleMapStatusTapis a clean way to branch behavior while keeping the originaldidTapMapStatusintact. - Tracking
isShowingZoomWarningvia the existing delegate callback is the right approach — no new notification or KVO plumbing needed. - The video demo in the PR description is very helpful for understanding the UX change.
Recommended Action
Please fix the indentation issue (#1 above), then this is good to merge. The fit-and-finish items are optional but would improve the code.
Fixes
Fixes #1062
Overview
Currently, when a user zooms out too far on the main map, a passive "Zoom in for stops" warning appears at the top of the screen. To actually see the stops, the user is forced to manually pinch and adjust the map until they find the exact zoom level that triggers the map pins to render.
Proposed Behavior
To reduce UX friction and make map discovery more seamless, I have transformed this informational warning into an actionable trigger. When a user taps the "Zoom in for stops" text, the map now automatically animates and zooms into a 0.01 degree coordinate span. This guarantees that transit stops instantly become visible with a single tap, completely bypassing the need for manual gestures.
Changes Made
UITapGestureRecognizerto themapStatusViewinMapViewControllerto handle user interaction.didTapZoomInForStops(), which calculates the current map center and performs a programmatic zoom to a safe threshold where stops are guaranteed to load.isShowingZoomWarningboolean tracked via themapRegionManagerShowZoomInStatusdelegate method. This ensures that when the zoom warning is not present, standard taps on the status view still correctly trigger the expected location service alerts.Visual Proof
StopButton1.mov