Fix camera binding issues #43
Conversation
|
Would love a review from @Patrick-Kladek and/or @hactar on this, since it does make a few changes you probably should know about. |
| @@ -5,7 +5,7 @@ import SwiftUI | |||
|
|
|||
| public struct MapView<T: MapViewHostViewController>: UIViewControllerRepresentable { | |||
| public typealias UIViewControllerType = T | |||
| var cameraDisabled: Bool = true | |||
| var cameraDisabled: Bool = false | |||
There was a problem hiding this comment.
Obvious change discussed on Slack.
| @@ -18,7 +18,7 @@ public struct MapView<T: MapViewHostViewController>: UIViewControllerRepresentab | |||
| var onStyleLoaded: ((MLNStyle) -> Void)? | |||
| var onViewPortChanged: ((MapViewPort) -> Void)? | |||
|
|
|||
| public var mapViewContentInset: UIEdgeInsets = .zero | |||
| var mapViewContentInset: UIEdgeInsets? = .zero | |||
There was a problem hiding this comment.
Two point of note here:
- I apologise if I've forgotten a reason that this was supposed to be public, but I don't see an obvious reason so removed that modifier.
- I made the insets optional because I think that it's quite possible that someone (maybe @Patrick-Kladek and @hactar?) actually DO want the default behavior! (More in a future comment at the use site.)
| @@ -103,13 +103,16 @@ public struct MapView<T: MapViewHostViewController>: UIViewControllerRepresentab | |||
|
|
|||
| if cameraDisabled == false { | |||
| context.coordinator.updateCamera(mapView: uiViewController.mapView, | |||
| camera: $camera.wrappedValue, | |||
| camera: camera, | |||
There was a problem hiding this comment.
I sometimes forget the details of binding syntactic sugar, but I am pretty sure this is equivalent.
| animated: isStyleLoaded) | ||
| } | ||
| } | ||
|
|
||
| @MainActor private func applyModifiers(_ mapViewController: T, runUnsafe: Bool) { | ||
| mapViewController.mapView.contentInset = mapViewContentInset | ||
| if let mapViewContentInset { | ||
| mapViewController.mapView.automaticallyAdjustsContentInset = false |
There was a problem hiding this comment.
So, here's where things get a bit funny. We HAVE to disable automatic inset adjustment here, but as initially submitted, this PR does NOT have a solution for opting back in once you've disabled it. It feels like the API for doing so would be a bit clunky.
For example, we could add a modifier which sets this explicitly, but then it's going to sometimes conflict with this and so it won't take effect.
A second approach might be to model content insets as an enum. I think that works.... Is it really a binary choice of full manual or full auto?
This fixes two issues with camera handling introduced in #39.
MLNMapViewwithin aUIViewControllercreates an "edit war" if you are trying to set content insets, which all navigation use cases will need to do. This was pretty hard to foresee, but the docs on thecontentInsetsproperty do mention the behavior.