-
Notifications
You must be signed in to change notification settings - Fork 31
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
iOS Route Overview #330
base: main
Are you sure you want to change the base?
iOS Route Overview #330
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import FerrostarCore | ||
import MapLibre | ||
import MapLibreSwiftUI | ||
|
||
extension NavigationState { | ||
var routeOverviewCamera: MapViewCamera? { | ||
guard let firstCoordinate = routeGeometry.first else { | ||
return nil | ||
} | ||
|
||
let initial = MLNCoordinateBounds( | ||
sw: firstCoordinate.clLocationCoordinate2D, | ||
ne: firstCoordinate.clLocationCoordinate2D | ||
) | ||
let bounds = routeGeometry.reduce(initial) { acc, coord in | ||
MLNCoordinateBounds( | ||
sw: CLLocationCoordinate2D(latitude: min(acc.sw.latitude, coord.lat), longitude: min( | ||
acc.sw.longitude, | ||
coord.lng | ||
)), | ||
ne: CLLocationCoordinate2D( | ||
latitude: max(acc.ne.latitude, coord.lat), | ||
longitude: max(acc.ne.longitude, coord.lng) | ||
) | ||
) | ||
} | ||
|
||
return MapViewCamera.boundingBox(bounds, edgePadding: UIEdgeInsets(top: 20, left: 100, bottom: 20, right: 100)) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ public struct DynamicallyOrientingNavigationView: View, CustomizableNavigatingIn | |
public var topCenter: (() -> AnyView)? | ||
public var topTrailing: (() -> AnyView)? | ||
public var midLeading: (() -> AnyView)? | ||
public var bottomLeading: (() -> AnyView)? | ||
public var bottomTrailing: (() -> AnyView)? | ||
|
||
let isMuted: Bool | ||
|
@@ -106,8 +107,13 @@ public struct DynamicallyOrientingNavigationView: View, CustomizableNavigatingIn | |
showZoom: true, | ||
onZoomIn: { camera.incrementZoom(by: 1) }, | ||
onZoomOut: { camera.incrementZoom(by: -1) }, | ||
showCentering: !camera.isTrackingUserLocationWithCourse, | ||
onCenter: { camera = navigationCamera }, | ||
cameraControlState: camera.isTrackingUserLocationWithCourse ? .showRouteOverview { | ||
if let overviewCamera = navigationState?.routeOverviewCamera { | ||
camera = overviewCamera | ||
} | ||
} : .showRecenter { // TODO: Third case when not navigating! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TBD the best way to express this logic given the limits of view code; probably in an extension / helper function. |
||
camera = navigationCamera | ||
}, | ||
onTapExit: onTapExit, | ||
currentRoadNameView: currentRoadNameView | ||
) | ||
|
@@ -117,6 +123,8 @@ public struct DynamicallyOrientingNavigationView: View, CustomizableNavigatingIn | |
topTrailing?() | ||
} midLeading: { | ||
midLeading?() | ||
} bottomLeading: { | ||
bottomLeading?() | ||
Comment on lines
+126
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Undo this change; recenter will stay in lower left corner. |
||
} bottomTrailing: { | ||
bottomTrailing?() | ||
}.complementSafeAreaInsets(parentGeometry: geometry, minimumInsets: minimumSafeAreaInsets) | ||
|
@@ -131,8 +139,13 @@ public struct DynamicallyOrientingNavigationView: View, CustomizableNavigatingIn | |
showZoom: true, | ||
onZoomIn: { camera.incrementZoom(by: 1) }, | ||
onZoomOut: { camera.incrementZoom(by: -1) }, | ||
showCentering: !camera.isTrackingUserLocationWithCourse, | ||
onCenter: { camera = navigationCamera }, | ||
cameraControlState: camera.isTrackingUserLocationWithCourse ? .showRouteOverview { | ||
if let overviewCamera = navigationState?.routeOverviewCamera { | ||
camera = overviewCamera | ||
} | ||
} : .showRecenter { // TODO: Third case when not navigating! | ||
camera = navigationCamera | ||
}, | ||
onTapExit: onTapExit, | ||
currentRoadNameView: currentRoadNameView | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,23 @@ | ||
import FerrostarCore | ||
import SwiftUI | ||
|
||
/// Determines which camera control is shown in the navigation inner grid view. | ||
public enum CameraControlState { | ||
/// Don't show any camera control. | ||
case hidden | ||
|
||
/// Shows the recenter button. | ||
/// | ||
/// The action is responsible for resetting the camera | ||
/// to a state that follows the user. | ||
case showRecenter(() -> Void) | ||
|
||
/// Shows the route overview button. | ||
/// | ||
/// The action is responsible for transitioning the camera to an overview of the route. | ||
case showRouteOverview(() -> Void) | ||
} | ||
Comment on lines
+4
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the first significant design decision. I think that an enum makes the camera control states significantly cleaner to express. We can add more later if necessary, but the common ones are here. |
||
|
||
/// When navigation is underway, we use this standardized grid view with pre-defined metadata and interactions. | ||
/// This is the default UI and can be customized to some extent. If you need more customization, | ||
/// use the ``InnerGridView``. | ||
|
@@ -14,8 +31,7 @@ public struct NavigatingInnerGridView: View, CustomizableNavigatingInnerGridView | |
let onZoomIn: () -> Void | ||
let onZoomOut: () -> Void | ||
|
||
let showCentering: Bool | ||
let onCenter: () -> Void | ||
let cameraControlState: CameraControlState | ||
|
||
let showMute: Bool | ||
let isMuted: Bool | ||
|
@@ -26,6 +42,7 @@ public struct NavigatingInnerGridView: View, CustomizableNavigatingInnerGridView | |
public var topCenter: (() -> AnyView)? | ||
public var topTrailing: (() -> AnyView)? | ||
public var midLeading: (() -> AnyView)? | ||
public var bottomLeading: (() -> AnyView)? | ||
public var bottomTrailing: (() -> AnyView)? | ||
|
||
/// The default navigation inner grid view. | ||
|
@@ -42,12 +59,7 @@ public struct NavigatingInnerGridView: View, CustomizableNavigatingInnerGridView | |
/// - showZoom: Whether to show the provided zoom control or not. | ||
/// - onZoomIn: The on zoom in tapped action. This should be used to zoom the user in one increment. | ||
/// - onZoomOut: The on zoom out tapped action. This should be used to zoom the user out one increment. | ||
/// - showCentering: Whether to show the centering control. This is typically determined by the Map's centering | ||
/// state. | ||
/// - onCenter: The action that occurs when the user taps the centering control (to re-center the map on the | ||
/// user). | ||
/// - showMute: Whether to show the provided mute toggle or not. | ||
/// - spokenInstructionObserver: The spoken instruction observer (for driving mute button state). | ||
/// - cameraControlState: Which camera control to show (and its respective action). | ||
public init( | ||
speedLimit: Measurement<UnitSpeed>? = nil, | ||
speedLimitStyle: SpeedLimitView.SignageStyle? = nil, | ||
|
@@ -57,8 +69,7 @@ public struct NavigatingInnerGridView: View, CustomizableNavigatingInnerGridView | |
showZoom: Bool = false, | ||
onZoomIn: @escaping () -> Void = {}, | ||
onZoomOut: @escaping () -> Void = {}, | ||
showCentering: Bool = false, | ||
onCenter: @escaping () -> Void = {} | ||
cameraControlState: CameraControlState = .hidden | ||
) { | ||
self.speedLimit = speedLimit | ||
self.speedLimitStyle = speedLimitStyle | ||
|
@@ -68,8 +79,7 @@ public struct NavigatingInnerGridView: View, CustomizableNavigatingInnerGridView | |
self.showZoom = showZoom | ||
self.onZoomIn = onZoomIn | ||
self.onZoomOut = onZoomOut | ||
self.showCentering = showCentering | ||
self.onCenter = onCenter | ||
self.cameraControlState = cameraControlState | ||
} | ||
|
||
public var body: some View { | ||
|
@@ -86,6 +96,29 @@ public struct NavigatingInnerGridView: View, CustomizableNavigatingInnerGridView | |
}, | ||
topCenter: { topCenter?() }, | ||
topTrailing: { | ||
// TODO: Extract to a separate view? | ||
switch cameraControlState { | ||
case .hidden: | ||
// Nothing | ||
EmptyView() | ||
case let .showRecenter(onCenter): | ||
NavigationUIButton(action: onCenter) { | ||
Image(systemName: "location.north.fill") | ||
.resizable() | ||
.aspectRatio(contentMode: .fit) | ||
.frame(width: 18, height: 18) | ||
} | ||
.shadow(radius: 8) | ||
case let .showRouteOverview(onRouteOverview): | ||
NavigationUIButton(action: onRouteOverview) { | ||
Image(systemName: "point.bottomleft.forward.to.point.topright.scurvepath") | ||
.resizable() | ||
.aspectRatio(contentMode: .fit) | ||
.frame(width: 18, height: 18) | ||
} | ||
.shadow(radius: 8) | ||
} | ||
Comment on lines
+99
to
+120
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The important thing to note here is that we have moved the recenter button to be in the top corner. The logic behind that is that we probably only ever care about having one camera button visible at a time in my opinion (differing opinions welcome :D). So the camera button slot will either be a recenter button, a route overview button, or hidden, depending on state driven higher up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Discussed; we will move the recenter button to the bottom left corner to keep with convention. |
||
|
||
if showMute { | ||
MuteUIButton(isMuted: isMuted, action: onMute) | ||
.shadow(radius: 8) | ||
|
@@ -104,19 +137,7 @@ public struct NavigatingInnerGridView: View, CustomizableNavigatingInnerGridView | |
Spacer() | ||
} | ||
}, | ||
bottomLeading: { | ||
if showCentering { | ||
NavigationUIButton(action: onCenter) { | ||
Image(systemName: "location.north.fill") | ||
.resizable() | ||
.aspectRatio(contentMode: .fit) | ||
.frame(width: 18, height: 18) | ||
} | ||
.shadow(radius: 8) | ||
} else { | ||
Spacer() | ||
} | ||
}, | ||
bottomLeading: { bottomLeading?() }, | ||
bottomCenter: { | ||
// This view does not allow center content to prevent overlaying the puck. | ||
Spacer() | ||
|
Unchanged files with check annotations Beta
extension FerrostarCore: LocationManagingDelegate { | ||
@MainActor | ||
public func locationManager(_: LocationProviding, didUpdateLocations locations: [UserLocation]) { | ||
Check warning on line 378 in apple/Sources/FerrostarCore/FerrostarCore.swift
|
||
guard let location = locations.last, | ||
let state = state?.tripState, | ||
let newState = navigationController?.updateUserLocation(location: location, state: state) |
} | ||
queue.async { | ||
self.synthesizer.speak(utterance) | ||
Check warning on line 41 in apple/Sources/FerrostarCore/Speech/SpokenInstructionObserver.swift
|
||
} | ||
} | ||
await fulfillment(of: [routeDeviationCallbackExp], timeout: 1.0) | ||
await fulfillment(of: [loadedAltRoutesExp], timeout: 1.0) | ||
XCTAssert(core.state?.isCalculatingNewRoute == false, "Expected to no longer be calculating a new route") | ||
Check failure on line 398 in apple/Tests/FerrostarCoreTests/FerrostarCoreTests.swift
|
||
} | ||
// TODO: Various location services failure modes (need special mocks to simulate these) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The padding values are silly because the implementation in the MapLibre SwiftUI DSL seems to be broken.