Add buildings extrusion example. #71
Conversation
There was a problem hiding this comment.
Found some not critical points but also I could not build the project after clean checkout. pod install --repo-update modifies Navigation=Examples.xcodeproj/project.pbxproj and the build itself fails with Pods-Navigation-Examples-artifacts.sh file missing in Scripts Build Phase.
| return routes?.first | ||
| } | ||
| set { | ||
| guard let selected = newValue else { routes?.remove(at: 0); return } |
There was a problem hiding this comment.
This creates unexpected behavior:
self.currentRoute = nil
if self.current.route == nil { // will be true if there were at least 2 `routes`
// ...
I suggest
| guard let selected = newValue else { routes?.remove(at: 0); return } | |
| guard let selected = newValue else { routes = nil; return } |
There was a problem hiding this comment.
Latest update should resolve this issue too.
| func changeToNightStyle() { | ||
| mapView?.style?.transition = MGLTransition(duration: 1.0, delay: 1.0) | ||
| mapView?.styleURL = MGLStyle.navigationNightStyleURL | ||
| } |
There was a problem hiding this comment.
Maybe something like toggleDayNightStyles() would be more functional?
There was a problem hiding this comment.
Good idea, added toggle between Day and Night styles.
| // MARK: - MGLMapViewDelegate methods | ||
|
|
||
| func mapView(_ mapView: MGLMapView, didFinishLoading style: MGLStyle) { | ||
| removeRoutes() |
There was a problem hiding this comment.
Why do we want to remove routes when style is updated?
Also, this case creates inconsistency: removeRoutes() cleans routes display on the map, but it does not clean the routes array, which remains populated and thus startNavigation() will still run. I suggest calling routes = nil instead if we need to clean routes on style change.
There was a problem hiding this comment.
This was mainly done to have the ability to see whether dynamic changes in style cause any issues with routes, waypoints etc. For now such changes cause issue described here. I'll remove this delegate method once we have fix for it, on the other had I'll keep the ability to change styles. I think with new update inconsistency with storing routes data should be fixed.
|
|
||
| var routes: [Route]? { | ||
| didSet { | ||
| guard let routes = routes, let current = routes.first else { mapView?.removeRoutes(); return } |
There was a problem hiding this comment.
I think it's better to call removeRoutes() instead
| guard let routes = routes, let current = routes.first else { mapView?.removeRoutes(); return } | |
| guard let routes = routes, let current = routes.first else { removeRoutes(); return } |
There was a problem hiding this comment.
Yes, I think it was handled in most recent update.
Just did clean clone like this: and able to build the project. Build job is also passing on CircleCI. Maybe it's related to your local repo state? I can also see that |
…hanges from master.
… current route state.
| let actions: [(String, UIAlertAction.Style, ActionHandler?)] = [ | ||
| ("Start Navigation", .default, startNavigation), | ||
| ("Toggle Day/Night Style", .default, toggleDayNightStyle), | ||
| ("Unhighlight Buildings", .default, unhighlightBuildings), | ||
| ("Remove Routes", .default, removeRoutes), | ||
| ("Cancel", .cancel, nil) | ||
| ] |
There was a problem hiding this comment.
This example is fairly unusual in that it combines several different actions, but it is more straightforward this way than multiple examples that overlap in various ways.
Closing #70.
Depends on work being done in #2535.