Usage as non navigation map

[hactar]

In the hudhud app, we would like to use the provided Views for showing maps that are not being navigated. The idea behind this is that you start an app in non navigation mode, like apple maps does, and then only later switch to navigation mode by tapping a button - now ferrostar core takes over camera controls and map modifications.

The main changes are:

• adding a mapViewModifiersWhenNotNavigating closure where developers can attach map view modifiers to be applied while not navigating, such as touch controls for map features
• wrapping elements such as navigation ui, insets and camera controls in navigationState.isNavigating checks to ensure they only appear/do their thing while navigating
• I've also added a few convenience extensions for some of the UniFFi stuff.

While I have made the effort to not "clash" with navigation mode by making the mapview modifiers either/or, in a future step we would for example like to add touch controls to navigation itself, being able to tap a gas station or similar, and doing things based on this. My current solution does not cover this case.

I've marked a few things as comments that I'd love to see improved, but wasn't able to figure out cleanly, maybe someone has an idea.

[hactar]

I wish we wouldn't have to type-erase here, but I didn't find a way to get this to work with it returning some View or a MapView as this closure may contain both MapView modifiers such as onMapTapGesture, but also View modifiers such as onTapGesture.

[hactar]

I chose a very simple definition of isNavigating - I'm not sure about this though. Is the .idle tripState navigation for example?

[ianthetechie]

Yeah, this is correct.

[michaelkirk]

Is there a reason this is a draft? Are you waiting for something in particular?

[michaelkirk]

Seems like a weird entry with your local machine details.

[hactar]

Thanks, yes I was referencing my local copy so that I could edit it directly in Xcode. Will fix in next commit.

[michaelkirk]

Why did this become public?

[hactar]

I needed access to this to help me with debugging, could be turned to private again, though one thing that might be useful is making this a public var, so that one can switch between the simulated location provider and the real location provider if needed (either for debugging, or for tunnel simulation, unless ferrostar core is planning to handle that internally.

[ianthetechie]

I'm open to making this public so that it would be easier to switch (though I personally have a hard time thinking this will get any use outside debug builds, but I also know I lack imagination sometimes; call me out an whatever I'm missing :D).

The main reason it's private right now is that it's not actually safe to just go resetting it at will. It would need setter logic to ensure that everything below is aware and subscribes to updates correctly.

[hactar]

I think 99% of the cases will be debug cases. That said, I do think its an important debug case:

• exposing it as read only gives devs important information on why ferrostar might be behaving how its behaving (where does ferrostar think it is, what accuracy does it have, etc)
• we have a debug menu where in maplibre navigation, we switch between the simulated and real location manager. We use this frequently as switching between real locations and simulated locations is very powerful, we do not need to rebuild (or have access to xcode location simulating) to quickly test both modes, depending on our situation (working in the office, on the go, on the go but stationary). So a set-able version would be great - and I think we might need to be able to do this anyway for continuing navigation while accuracy is bad like when users are in a tunnel?

[ianthetechie]

Those debug use cases make sense.

I think we might need to be able to do this anyway for continuing navigation while accuracy is bad like when users are in a tunnel?

You might be righte about this... Some use cases will be better solved with a custom LocationProvider (ex: if you have wheel sensors, sophisticated inertial processing, etc.). I haven't yet examined the existing approaches closely enough to see how these work.

One approach would be the developer using the simulated provider directly and replacing it as you're implying here. This would actually be pretty straightforward; it has some shortcomings right now like not being able to very easily set the speed, but those are easy to address. Another could be a "fallback" configuration where the dev doesn't have to reset the location provider directly but rather provides a fallback designed to simulate progress + parameters on when to switch back and forth, what speed to simulate progress at, how long (distance) to simulate progress, decay in speed as you get past some threshold, etc.

This is all pretty half-baked, but the idea behind that all would be to let you do something like "my router tells me that this tunnel is 3km long. My average speed over 30 seconds leading up to the tunnel has been 70kph with minimal variance, so we'll simulate progress at 68kph and hopefully we get a smooth transition." And at the end of the tunnel idk just stop or slow way down? And kick back over to "live" location once we get a location update from CoreLocation with an accuracy of better than 50m.

TL;DR though, yes, switching should be possible, both directly and maybe indirectly ;)

[michaelkirk]

Will this be called when the user calls ferrostarCore.startNavigation(route: route)?

[hactar]

Nope - unless start navigation is called before the style completes. But this call here should only be seen as a fallback, something else should set the camera to navigation camera, like startNavigation.

[Archdoog]

Idea for this concept generally. Could we extend the NavigationMapView with navigations state event view modifier(s) (e.g. func navigationDidStart(onStart: () -> Void)). Allowing the developer to apply modifications to the map view's configuration when certain navigationState events occur? This might be a bit overkill, but seems like it would first allow us to do default behaviors like setting the navigationCamera one time when navigation starts as well as let the developer modify the view's behavior when certain nav event occur? It may also be quite scalable as an approach.

[ianthetechie]

We might not actually need this if we go with your suggestion (which I'll elaborate on in my review comments), since the modifiers get applied any time state changes ;)

[hactar]

While we might not need it, it might still be convenient? one can already achieve this by a .onChange(ferrostar.state) but this would be easier to use and understand for people starting with the framework?

[ianthetechie]

Yeah, fair point ;) I'm open to draft implementations from either of you if you think it's helpful. I originally was super enthusiastic when @Archdoog described it on our call but then realized it might not be a hard requirement with the other proposed changes.

[michaelkirk]

Personally, I'd actually never like to show the zoom buttons, and rely only on pinch/tap to zoom in all cases.

I wonder if a delegate query would be a better choice

(or maybe init parameter is more conventional in SwiftUI? I'm still pretty new to SwiftUI and not confident on the conventions)

[hactar]

I agree, I'm not a fan of zoom buttons either, but I wanted to keep it as close as possible to the current setup first :)

[Archdoog]

All for that. The only reason I added the zoom buttons in the first place was to include them in the available UI components and to provide a simple verification for zoom behavior while navigating.

Long term goal is the existing configurable static zoom as well as a fancier dynamic zoom per activity. E.g. zooms out when as route annotation speed increases. We're almost there on the "current annotation" state, so this probably won't be hard shortly.

This can become as fancy as we want as long as various use cases are supported 👍

[ianthetechie]

Yeah, this SHOULD be configurable. Thanks for pointing it out!

On zoom or no zoom, that's a decision that app developers can make. I don't like the screen real estate, but OTOH, 80% of the time pinch zoom makes the camera stop following me (in every nav app I can think of), and that's ALMOST never what I wanted, so I can appreciate the button option ;)

[hactar]

Is there a reason this is a draft? Are you waiting for something in particular?

While it is fully functional, it's meant for the discussion here: https://osmus.slack.com/archives/C0553U433FX/p1726827237335429 - this approach may be different to what is intended by the makers. Also I'd want to add documentation etc before this is merged. 😊

[ianthetechie]

Thanks a bunch for opening this draft @hactar! This is super helpful and let us see what you're thinking a lot better.

I've had a day or two to mull over this and discussed on our weekly call with @Archdoog yesterday.

North Star

The problem with the current design of some Ferrostar higher-level views (ex: DynamicallyOrientingNavigationView) is that it's difficult to make any modifications to the map view. This is an unintended side effect of the current design. We would like to preserve the ability for devs to get at this.

The specific use case of changing the behavior when the map is not in a "navigating" state is important, but I think we can find a general solution that covers this use case.

Thoughts on draft solution

First, I recognize this is a draft, so take all comments as such :)

The current draft addresses the problem with a new closure that modifies the map view when not navigating. This has at least two unfortunate side-effects. The first is type erasure, which you already noted in your comments. Second is the fact that we now have to use escaping closures. This makes it very hard to reason about. I guess a third would be that now we have a bunch of internal if/else logic which could be difficult to maintain.

Proposal for discussion

I think that exposing the same modifiers that the map view has on these higher level views would give you the flexibility you need while addressing the concerns with the current draft. This way you can just chain them naturally as if you had access to the map view directly.

Your code can then conditionally apply modifiers to the view without indirect callbacks. The conveniences you introduce in this PR, which are based off navigation state (something your view would be "observing" already) should offer an elegant solution for doing this conditionally.

I think this approach solves the type erasure problem and provides a more ergonomic interface. There are some implementation complexities, but let's defer discussing those until we decide if we like the idea first :)

[ianthetechie]

Yeah, this SHOULD be configurable. Thanks for pointing it out!

On zoom or no zoom, that's a decision that app developers can make. I don't like the screen real estate, but OTOH, 80% of the time pinch zoom makes the camera stop following me (in every nav app I can think of), and that's ALMOST never what I wanted, so I can appreciate the button option ;)

[ianthetechie]

I'm open to making this public so that it would be easier to switch (though I personally have a hard time thinking this will get any use outside debug builds, but I also know I lack imagination sometimes; call me out an whatever I'm missing :D).

The main reason it's private right now is that it's not actually safe to just go resetting it at will. It would need setter logic to ensure that everything below is aware and subscribes to updates correctly.

[ianthetechie]

We might not actually need this if we go with your suggestion (which I'll elaborate on in my review comments), since the modifiers get applied any time state changes ;)

[ianthetechie]

Yeah, this is correct.

[hactar]

Proposal for discussion

I think that exposing the same modifiers that the map view has on these higher level views would give you the flexibility you need while addressing the concerns with the current draft. This way you can just chain them naturally as if you had access to the map view directly.

Your code can then conditionally apply modifiers to the view without indirect callbacks. The conveniences you introduce in this PR, which are based off navigation state (something your view would be "observing" already) should offer an elegant solution for doing this conditionally.

I think this approach solves the type erasure problem and provides a more ergonomic interface. There are some implementation complexities, but let's defer discussing those until we decide if we like the idea first :)

Thanks for the reviews and the thoughts.

So if I understand you correctly you propose a solution such as

DynamicallyOrientingNavigationView(...)
.onMapTapGesture { features in
   // this code will run while not navigating...
}

So when I initially started coding on this, this was my first call too, I just tried attaching those typical modifiers to DynamicallyOrientingNavigationView and was sad that that didn't work. So yes, what you describe would be the most "natural" way of doing this for me too, instead of jumping through that closure that I added - I would like that solution. That said, for me a few questions remain unanswered:

• If these modifiers are only applied while isNavigating == false, how are developers supposed to customize behavior during navigation? Like let's say we want to make gas stations during navigation tappable, and when tapped the gas station is added as a waypoint to the currently running navigation. (my solution also doesn't currently cover this)
• I didn't test if it might actually be already enough to just attach non MapView view modifiers directly to DynamicallyOrientingNavigationView (like drag gesture detectors, safe area insets, etc), but if its not, we'd not only need to "pass through" MapView modifiers, but view modifiers themselves.
• And then finally, yes the "implementation complexities", am I missing something or doesn't that mean we'd have to manually add all these map view modifiers to DynamicallyOrientingNavigationView and others, always ensuring that when we add a new modifier to maplibre dsl, we also add it to here too? Sounds unfun to maintain 😅

[ianthetechie]

If these modifiers are only applied while isNavigating == false, how are developers supposed to customize behavior during navigation?

Actually, I'd propose having them ALWAYS run. Sorry if I didn't make that clear. You can do the if navigationState.isNavigating { ... } check yourself for maximum flexibility.

If we decide that it's also necessary to have a "disable all non-essential modifiers" or something, we can figure that out. I'm hopeful that it won't be needed since this view SHOLUD be pretty unobtrusive, but I think that's a fairly clean way to offer a bypass in case some modifiers internal to the view end up causing problems.

... but if its not, we'd not only need to "pass through" MapView modifiers, but view modifiers themselves

Yeah, I think that's right. Swift is not Haskell so I can't just make an fmap over this with some type system magic :P The idea (credit to @Archdoog, not me) is to make it so you could call any map view modifier on the DynamicallyOrientingNavigationView.

And then finally, yes the "implementation complexities" ...

😅 I have a few ideas involving some (TBD) mix of protocols and macros... I think the next step if we think this is roughly the right direction is to just hand code up one or two modifiers that you'd actually like to use in your app (can be committed to this branch). Then we can see if we missed some fatal flaw or ergonomic annoyance. Then if that's feeling alright for you, we can generalize it and worry about those "implementation complexities."

[hactar]

Problem is even our implementation already uses unsafeMapViewControllerModifier, onTapGesture, expandClustersOnTapping, onStyleLoaded, onLongPressMapGesture from the MapView modifiers, and also: gesture from the some view modifiers - and as far as I see each one will need to be added as modifiers to both DynamicallyOrientingNavigationView and NavigationMapView? Or am I missing something? 😅

While we're discussing, as we need to make progress, we're running this solution for now (which again, isn't meant as a "lets do it this way please", but we have to progress with the rest of the app too :) ). I have updated the solution based on our discussion:

mapViewModifiersWhileNotNavigating(view) has been changed to mapViewModifiers(view, isNavigating). The modifiers are now always applied, but developers can use isNavigating to apply conditions within the closure when to apply which modifier.

I've also merged in upstream. Thanks for the pulley for the instructions @michaelkirk :)

[hactar]

Further update: because of the AnyView wrapper, the whole UIViewController for MapView was being recreated, causing makeUIViewController to be called multiple times. As far as I understand, UIViewControllerRepresentable need to written in a way that they are recreate-able at any time , but our current implementation cannot be recreated: the recreated version has a lat long camera of 0,0 and shows the whole map, before potentially reapplying a camera with an animation.

I didn't explore fixing this, instead I've turned the mapViewModifier to return a MapView, so that when isNavigating changes, SwiftUI understands that this is still the same MapView.

[ianthetechie]

and as far as I see each one will need to be added as modifiers to both DynamicallyOrientingNavigationView and NavigationMapView? Or am I missing something? 😅

Nope, you're not missing anything. I think some mix of protocols (to enforce that we don't accidentally forget something) and/or macros (to automate the boilerplate) will be required to do this cleanly. Unless Swift evolved some more type system improvements that I missed :/

While we're discussing, as we need to make progress, we're running this solution for now (which again, isn't meant as a "lets do it this way please", but we have to progress with the rest of the app too :) ). I have updated the solution based on our discussion:

Understood; please do keep trying things and I'll see if I can get some time to explore my crazy ideas this week. We'll get there eventually, and in the meantime you need to make progress so all cool.