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

Add support for heading calibration alert #1509

Merged
merged 8 commits into from
Aug 10, 2022

Conversation

evil159
Copy link
Contributor

@evil159 evil159 commented Aug 3, 2022

This PR adds a method to location manager's delegate protocol to provide the ability to display the heading calibration alert on top of the current window immediately.

Public API changes

  • LocationPermissionsDelegate.locationManagerShouldDisplayHeadingCalibration(_:) -> Bool added

Pull request checklist:

  • Describe the changes in this PR, especially public API changes.
  • Write tests for all new functionality. If tests were not written, please explain why.
  • Add documentation comments for any added or updated public APIs.
  • Add any new public, top-level symbols to the Jazzy config's custom_categories (scripts/doc-generation/.jazzy.yaml)
  • Add a changelog entry to to bottom of the relevant section (typically the ## main heading near the top).

PRs must be submitted under the terms of our Contributor License Agreement CLA.

@evil159 evil159 requested a review from a team as a code owner August 3, 2022 11:57
@evil159
Copy link
Contributor Author

evil159 commented Aug 3, 2022

  • Test this by making the heading calibration alert to appear

OdNairy
OdNairy previously requested changes Aug 3, 2022
Sources/MapboxMaps/Location/LocationManager.swift Outdated Show resolved Hide resolved
@evil159 evil159 requested a review from OdNairy August 8, 2022 11:09
@@ -40,3 +40,7 @@ internal class EmptyLocationProviderDelegate: LocationProviderDelegate {
func locationProvider(_ provider: LocationProvider, didUpdateLocations locations: [CLLocation]) {}
func locationProviderDidChangeAuthorization(_ provider: LocationProvider) {}
}

internal protocol CalibratingLocationProviderDelegate: LocationProviderDelegate {
func locationProviderShouldDisplayHeadingCalibration(_ locationProvider: LocationProvider) -> Bool
Copy link
Contributor

Choose a reason for hiding this comment

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

as discussed, we should have this as part of public LocationProviderDelegate so custom LocationProvider can have one possible channel of communicating back to LocationManager to consult whether or not should calibration alert be displayed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding another method to a public protocol would be a breaking change and we need to verify that methods with default implementation are dispatched correctly before using those.

@evil159 evil159 requested a review from maios August 10, 2022 13:32
@evil159 evil159 enabled auto-merge (squash) August 10, 2022 13:47
@evil159 evil159 dismissed OdNairy’s stale review August 10, 2022 13:48

Dismissed as the requested changes are done.

@evil159 evil159 merged commit 717cf39 into main Aug 10, 2022
@evil159 evil159 deleted the rl/MAPSIOS-259-heading-calibration-alert branch August 10, 2022 13:48
mapbox-github-ci-writer-public-1 bot pushed a commit that referenced this pull request May 3, 2023
…) (#1509)

* [CarPlay] Get associated scene of a window in CarPlay dashboard.
- When a CarPlay application is displaying with a dashboard, a window in the dashboard scene will not have an associated windowScene, instead we need to check if the notification's scene if it contains the map's window and pause display link accordingly.
mapbox-github-ci-writer-public-1 bot pushed a commit that referenced this pull request May 5, 2023
* [CarPlay] Get associated scene of a window in CarPlay dashboard. (#1503) (#1509)

* [CarPlay] Get associated scene of a window in CarPlay dashboard.
- When a CarPlay application is displaying with a dashboard, a window in the dashboard scene will not have an associated windowScene, instead we need to check if the notification's scene if it contains the map's window and pause display link accordingly.

* [v10] Expose ModelLayer api (#1513)

* Prepare release v10.13.1 (#1515)

* Expose Style/addMode(modelId:modelUri:) as experimental) (#1521)
OdNairy pushed a commit that referenced this pull request Aug 22, 2023
…) (#1509)

* [CarPlay] Get associated scene of a window in CarPlay dashboard.
- When a CarPlay application is displaying with a dashboard, a window in the dashboard scene will not have an associated windowScene, instead we need to check if the notification's scene if it contains the map's window and pause display link accordingly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants