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.

Sign up for GitHub

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

Jump to bottom

Fix camera binding issues #43

Merged
merged 2 commits into from
Aug 5, 2024
Merged

Fix camera binding issues #43

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5135853
Fix default cameraDisabled value
ianthetechie Aug 1, 2024
1e6f079
Flip automaticallyAdjustsContentInset to avoid contentInsets edit war
ianthetechie Aug 1, 2024
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
11 changes: 7 additions & 4 deletions Sources/MapLibreSwiftUI/MapView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import SwiftUI

public struct MapView<T: MapViewHostViewController>: UIViewControllerRepresentable {
public typealias UIViewControllerType = T
var cameraDisabled: Bool = true
var cameraDisabled: Bool = false
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Obvious change discussed on Slack.


@Binding var camera: MapViewCamera

Expand All @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Two point of note here:

  1. 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.
  2. 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.)


var unsafeMapViewControllerModifier: ((T) -> Void)?

Expand Down Expand Up @@ -103,13 +103,16 @@ public struct MapView<T: MapViewHostViewController>: UIViewControllerRepresentab

if cameraDisabled == false {
context.coordinator.updateCamera(mapView: uiViewController.mapView,
camera: $camera.wrappedValue,
camera: camera,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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?

mapViewController.mapView.contentInset = mapViewContentInset
}

// Assume all controls are hidden by default (so that an empty list returns a map with no controls)
mapViewController.mapView.logoView.isHidden = true
Expand Down
Loading