-
Notifications
You must be signed in to change notification settings - Fork 24
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 Mute button #294
IOS Mute button #294
Conversation
@@ -34,6 +34,7 @@ struct DemoNavigationView: View { | |||
|
|||
@State private var camera: MapViewCamera = .center(initialLocation.coordinate, zoom: 14) | |||
@State private var snappedCamera = true | |||
@State private var isNavigationActive = false // New state variable for navigation status |
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.
We actually have a pending change that adds this in #275 (https://github.com/stadiamaps/ferrostar/pull/275/files#r1778856304). I think we can remove this for now since it will soon be addressed at a more foundational level.
@@ -13,10 +13,10 @@ private let initialLocation = CLLocation(latitude: 37.332726, | |||
longitude: -122.031790) | |||
|
|||
struct DemoNavigationView: View { | |||
private let navigationDelegate = NavigationDelegate() | |||
private var navigationDelegate = NavigationDelegate() |
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.
Does this need to be a var
?
.overlay( | ||
|
||
HStack { | ||
|
||
Spacer() | ||
|
||
if isNavigationActive { | ||
|
||
MuteButton(isMuted: $spokenInstructionObserver.isMuted) | ||
|
||
} | ||
|
||
}, | ||
|
||
alignment: .topTrailing | ||
|
||
) |
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.
This works for the demo app, but doesn't do much for everyone building apps with the library components :)
Have a look at PortraitNavigationOverlayView
and LandscapeNavigationOverlayView
for how we handle this. We already built the whole grid layout and have ways to configure what's shown where, as well as overrides. Moving the mute button to the top trailing position there will also conveniently mean it's shown and hidden automatically in the future based on whether the user is navigating.
// | ||
// MuteButton.swift | ||
// Ferrostar Demo | ||
// | ||
// Created by Marek Sabol on 08/10/2024. | ||
// | ||
|
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 mute button isn't specific to the demo app but is reusable, so it should move to the
FerrostarSwiftUI
module. All similar controls are under FerrostarSwiftUI > Views > Controls. - We remove the automatic Xcode header comments.
.resizable() | ||
.aspectRatio(contentMode: .fit) | ||
.frame(width: 18, height: 18) | ||
.padding() | ||
.foregroundColor(.black) | ||
.background(Color.white) | ||
.clipShape(Circle()) | ||
.shadow(radius: 10) | ||
} | ||
.padding(.trailing, 18) // Right | ||
.padding(.top, 112) |
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.
We can probably remove most of these after integrating into the overlays. See the other controls for examlpes.
I've opened new PR for adjusments Ios mute fix after discussion #296 |
Hi, I've developed it based on our discussion, hopefully you will be satisfied with the implementation :). In case there will be some changes needed, let me know :).