Open hactar opened 2 months ago
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. š
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:
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.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 š
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."
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 :)
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
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.
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:
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.