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

CarPlay refactoring and bugfixing. #3219

Merged
merged 1 commit into from
Aug 20, 2021
Merged

Conversation

MaximAlien
Copy link
Contributor

@MaximAlien MaximAlien commented Aug 3, 2021

@MaximAlien MaximAlien added UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. CarPlay Bugs, improvements and feature requests on Apple CarPlay labels Aug 3, 2021
@MaximAlien MaximAlien self-assigned this Aug 3, 2021
@MaximAlien MaximAlien marked this pull request as draft August 4, 2021 00:00
@MaximAlien MaximAlien changed the title Car Play refactoring. CarPlay refactoring. Aug 4, 2021
@MaximAlien MaximAlien requested a review from a team August 4, 2021 00:50
@MaximAlien MaximAlien force-pushed the maxim/car-play-refactoring branch 4 times, most recently from 693f3ec to 884f728 Compare August 4, 2021 22:39
@MaximAlien MaximAlien changed the title CarPlay refactoring. CarPlay refactoring and bugfixing. Aug 5, 2021
@MaximAlien MaximAlien force-pushed the maxim/car-play-refactoring branch 5 times, most recently from ff3ffa9 to d08cce9 Compare August 10, 2021 17:44
@@ -306,10 +306,9 @@ extension CarPlayMapViewController: StyleManagerDelegate {
}

public func styleManager(_ styleManager: StyleManager, didApply style: Style) {
let styleURL = style.previewMapStyleURL
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes #3224. There should be an ability to change MapView style URL similarly to iOS.

Comment on lines 165 to 191
/**
Delegate method, which allows to observe when template presented by the `CarPlayManager` will
appear on the screen.
*/
func carPlayManager(_ carPlayManager: CarPlayManager,
templateWillAppear template: CPTemplate,
animated: Bool)

/**
Delegate method, which allows to observe when template presented by the `CarPlayManager` did
appear on the screen.
*/
func carPlayManager(_ carPlayManager: CarPlayManager,
templateDidAppear template: CPTemplate,
animated: Bool)

/**
Delegate method, which allows to observe when template presented by the `CarPlayManager` will
disappear from the screen.
*/
func carPlayManager(_ carPlayManager: CarPlayManager,
templateWillDisappear template: CPTemplate,
animated: Bool)

/**
Delegate method, which allows to observe when template presented by the `CarPlayManager` did
disappear from the screen.
*/
func carPlayManager(_ carPlayManager: CarPlayManager,
templateDidDisappear template: CPTemplate,
animated: Bool)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Delegate methods, which were added in scope of #3220.

@MaximAlien MaximAlien force-pushed the maxim/car-play-refactoring branch 7 times, most recently from cee1849 to 107969b Compare August 18, 2021 16:02
@MaximAlien MaximAlien marked this pull request as ready for review August 18, 2021 16:02
@MaximAlien MaximAlien force-pushed the maxim/car-play-refactoring branch from 107969b to 76f3ac2 Compare August 19, 2021 23:25
Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

Mostly documentation nits, but the removal of previewMapStyleURL usage would be a regression. Maybe I’m misunderstanding the impetus for that change though.

Sources/MapboxNavigation/CarPlayManager.swift Outdated Show resolved Hide resolved
Sources/MapboxNavigation/CarPlayManager.swift Outdated Show resolved Hide resolved
Sources/MapboxNavigation/CarPlayManager.swift Show resolved Hide resolved
Sources/MapboxNavigation/CarPlayManagerDelegate.swift Outdated Show resolved Hide resolved
Sources/MapboxNavigation/CarPlayMapViewController.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

Sorry, these suggestions didn’t take for some reason. Looks good overall.

Sources/MapboxNavigation/CarPlayManagerDelegate.swift Outdated Show resolved Hide resolved
Sources/MapboxNavigation/CarPlayManagerDelegate.swift Outdated Show resolved Hide resolved
Sources/MapboxNavigation/CarPlayManagerDelegate.swift Outdated Show resolved Hide resolved
…ack when template is being presented or dismissed. Improve documentation. Minor styling changes.
@MaximAlien MaximAlien force-pushed the maxim/car-play-refactoring branch from a8969d9 to 033d827 Compare August 20, 2021 19:11
@MaximAlien MaximAlien merged commit 4b337d8 into main Aug 20, 2021
@MaximAlien MaximAlien deleted the maxim/car-play-refactoring branch August 20, 2021 19:12
@chezzdev chezzdev mentioned this pull request Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CarPlay Bugs, improvements and feature requests on Apple CarPlay UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting CarPlayManager.interfaceController.delegate should not change internal CarPlayManager behavior.
2 participants