Start & Stop Navigation in existing Map #54
Conversation
|
Tests are passing now |
|
The plan was to get this merged by Monday but was delayed because of #58. Now the week nearly passed and I can't see any progress. Can we please get this merged today, I don't see any blockers or risks as we have a working 3.0 release. |
There was a problem hiding this comment.
@Patrick-Kladek, my understanding was that we first merge #58 and then #54.
I am fine with merging this PR first, I haven't seen any issues when running the code or when reviewing it.
|
No worries, I can understand. I am currently reviewing this repo with priority already. Reviews take their time if you do them seriously, so It's not always possible to do something on the same day. Feel free to merge this PR or wait on #58. I am fine with either way. Maybe @michaelkirk would like to chime in as well, I think he is in the US timezone, so maybe let's give him the opportunity to give this one more check as well? |
|
LGTM - I am OK with merging this before #58. Since the end of route logic is also changing with this PR, I'll probably want to re-evaluate how #58 gets merged anyway. To set future expectations, it will not be uncommon for me to take more than 2 days to respond to activity on Github (especially when one is a holiday for me, or the weekend). One way to speed up the review process in the future will be to try hard to have changes be as focused as possible on the critical changes. For my own project, I am running a fork that has my own PR's merged. It requires a bit of extra upkeep / rebase work, but it allows me to work at my own pace without feeling "held back" too much by the review process. |
|
Alright, we have 3 approvals, I'm gonna do the honors. Thanks @Patrick-Kladek for the contributions and iterations. And thanks for all coding and reviewing contributions, I know this wasn't a small change ❤️ |
After a tough rebase on top of maplibre#54, this branch doesn't work. The EORVC is not presented. My understanding is that I'm the only one currently working on the repository that even uses this VC, so maybe it's a waste of time to try to upstream my efforts there anyway.
…c view Note this was cherry-picked, and no longer works due to change in maplibre#54
…c view Note this was cherry-picked, and no longer works due to change in maplibre#54
FIXES maplibre#57 - by rewriting EORVC as programmatic VC rather than storyboard. FIXES maplibre#62 - Omitted the "Rate this route" feedback while rewriting the EORVC. Note this was cherry-picked, and no longer works due to change in maplibre#54
Where possible, the particulars of presentation were copied from b790f96 (aka before maplibre#54).
Where possible, the particulars of presentation were copied from b790f96 (aka before maplibre#54).
Prior to maplibre#54, the NavigationViewController could only be used modally. With maplibre#54, you can switch between "map mode" and "navigation mode". To do so, an animation was introduced for the sake of smoothly transitioning from "map mode" to "navigation mode". However, if you are still using the NavigationViewController modally, this animation might interfere with other things you were doing - like setting the camera.
Prior to maplibre#54, the NavigationViewController could only be used modally. With maplibre#54, you can switch between "map mode" and "navigation mode". To do so, an animation was introduced for the sake of smoothly transitioning from "map mode" to "navigation mode". However, if you are still using the NavigationViewController modally, this animation might interfere with other things you were doing - like setting the camera.
FIXES maplibre#57 - by rewriting EORVC as programmatic VC rather than storyboard. FIXES maplibre#62 - Omitted the "Rate this route" feedback while rewriting the EORVC. Note this was cherry-picked, and no longer works due to change in maplibre#54
Where possible, the particulars of presentation were copied from b790f96 (aka before maplibre#54).
Description
A long wanted feature was to interact with a map and later on start a navigation without presenting another map on top. This PR finally allows this. In retrospect it was quite simple, In
NavigationViewControllerthe route property is now optional and there are additional functions added to start & stop the navigation.Open Tasks
Infos for Reviewer
In the Example, the route calculation is done with Mapbox, users need a mapbox account and add the api-key in Secrets.xcconfig file. This file ignores changes, so the api-key should not leak into the git repo.
A nice side effect of this Task is, that we to got rid of some implicitly unwrapped optionals.
Breaking Changes
navigationViewControllerDidFinish()delegate method. TheEndOfRouteViewControlleris still bundled in the project.