Skip to content
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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions apple/DemoApp/Demo/DemoNavigationView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Contributor

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?

// NOTE: This is probably not ideal but works for demo purposes.
// This causes a thread performance checker warning log.
private let spokenInstructionObserver = AVSpeechSpokenInstructionObserver(isMuted: false)
@State private var spokenInstructionObserver = AVSpeechSpokenInstructionObserver(isMuted: false)

private var locationProvider: LocationProviding
@ObservedObject private var ferrostarCore: FerrostarCore
Expand All @@ -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
Copy link
Contributor

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.


init() {
let simulated = SimulatedLocationProvider(location: initialLocation)
Expand Down Expand Up @@ -121,6 +122,7 @@ struct DemoNavigationView: View {
isFetchingRoutes = true
try await startNavigation()
isFetchingRoutes = false
isNavigationActive = true // Set to true when navigation starts
} catch {
isFetchingRoutes = false
errorMessage = "\(error.localizedDescription)"
Expand Down Expand Up @@ -148,6 +150,23 @@ struct DemoNavigationView: View {
.task {
await getRoutes()
}
.overlay(

HStack {

Spacer()

if isNavigationActive {

MuteButton(isMuted: $spokenInstructionObserver.isMuted)

}

},

alignment: .topTrailing

)
Comment on lines +153 to +169
Copy link
Contributor

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.

}
}

Expand Down Expand Up @@ -209,6 +228,7 @@ struct DemoNavigationView: View {
ferrostarCore.stopNavigation()
camera = .center(initialLocation.coordinate, zoom: 14)
allowAutoLock()
isNavigationActive = false
}

var locationLabel: String {
Expand Down
34 changes: 34 additions & 0 deletions apple/DemoApp/Demo/MuteButtonView.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//
// MuteButton.swift
// Ferrostar Demo
//
// Created by Marek Sabol on 08/10/2024.
//

Comment on lines +1 to +7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 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.
  2. We remove the automatic Xcode header comments.

import SwiftUI

struct MuteButton: View {
@Binding var isMuted: Bool

var body: some View {
Button(action: {
isMuted.toggle()
}) {
Image(systemName: isMuted ? "speaker.slash.fill" : "speaker.2.fill")
.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)
Comment on lines +18 to +28
Copy link
Contributor

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.

}
}

#Preview {
MuteButton(isMuted: .constant(false))
}
Loading