Add tutorial code from mapbox.com/help #127

captainbarbosa · 2017-11-21T02:49:40Z

Adds the code used for iOS tutorials from mapbox.com/help/tutorials into the DocsCode target. This will be used for testing code snippets before being used in documentation on mapbox.com/help.

friedbunny · 2017-11-21T16:40:00Z

Podfile

@@ -1,9 +1,10 @@
-platform :ios, '8.0'
+platform :ios, '9.0'


We’ve been trying to keep this at the same minimum deployment target as the SDK, as one more check that we’re not breaking promised backwards compatibility. I’d like to keep it at 8, unless there’s absolutely no way not to.

friedbunny · 2017-11-21T16:40:31Z

Podfile

 use_frameworks!

 def shared_pods
  #pod 'Mapbox-iOS-SDK', '~> 3.6.3'
  pod 'Mapbox-iOS-SDK-symbols', :podspec => 'https://raw.githubusercontent.com/mapbox/mapbox-gl-native/ios-v3.6.4/platform/ios/Mapbox-iOS-SDK-symbols.podspec'
+  pod 'MapboxNavigation', '~> 0.10'


This is apparently problematic — tests are failing because CocoaPods is trying to install Mapbox.framework twice.

shared_pods is installed across all targets, while MapboxNavigation is only needed for DocsCode.

friedbunny · 2017-11-21T16:42:26Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.swift

@@ -0,0 +1,128 @@
+// #-code-snippet: navigation dependencies-swift
+import Mapbox
+import MapboxCoreNavigation


Is it necessary to import all four of these? At least MapboxCoreNavigation should be able to be implicitly imported.

Looks likes it is required from testing it out 😐

captainbarbosa · 2017-11-21T22:42:01Z

It seems like the build is failing because the DocsCode target is using Swift 4. I'll need to wait until mapbox/mapbox-navigation-ios#663 lands and then update the dependency here - once that is done this should pass on BitRise.

Edit magic comment placement syntax for DDS tutorial

friedbunny · 2017-11-27T20:55:11Z

DocsCode/DDSCircleLayerTutorial/DDSCircleLayerTutorialViewController.swift

+
+        // #-code-snippet: dds-circle add-style-layer-swift
+        layer.circleColor = MGLStyleValue<UIColor>(interpolationMode: .interval,
+        sourceStops: stops,


Nit: indent these lines to make it clear that they are children of L52. Up to you whether or not to align the arguments or not — I don’t think Swift usually does, while ObjC does.

friedbunny · 2017-11-27T20:58:19Z

DocsCode/DDSCircleLayerTutorial/DDSCircleLayerTutorialViewController.m

+    // #-end-code-snippet: dds-circle add-vector-source-objc
+
+    // #-code-snippet: dds-circle add-circle-layer-objc
+    MGLCircleStyleLayer *layer = [[MGLCircleStyleLayer alloc] initWithIdentifier: @"tree-style" source:source];


Nit: ObjC method calls don’t typically have a space before the parameter. E.g., initWithIdentifier:@"tree-style”.

There are other instances of unstylish spaces — it would be good to be consistent with this.

friedbunny · 2017-11-27T21:02:25Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.m

+                 toDestination :(CLLocationCoordinate2D)destination
+                     completion:(void(^)(MBRoute *_Nullable route, NSError *_Nullable error))completion {
+
+    MBWaypoint *originWaypoint = [[MBWaypoint alloc] initWithCoordinate:origin coordinateAccuracy:-1 name:@"Start"];


What does coordinateAccuracy:-1 do? Is -1 the correct value/type? Is this worth explaining to the reader?

friedbunny · 2017-11-27T21:03:38Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.m

+    MBNavigationRouteOptions *options = [[MBNavigationRouteOptions alloc] initWithWaypoints:@[originWaypoint, destinationWaypoint] profileIdentifier:MBDirectionsProfileIdentifierAutomobileAvoidingTraffic];
+
+    // Generate the route object and draw it on the map
+    NSURLSessionDataTask *task = [[MBDirections sharedDirections] calculateDirectionsWithOptions:options completionHandler:^(NSArray<MBWaypoint *> * _Nullable waypoints, NSArray<MBRoute *> * _Nullable routes, NSError * _Nullable error){


This is a very long line — can it be broken into something easier to read? The _Nullable annotations could be removed, though maybe they’re useful for a first-timer.

friedbunny · 2017-11-27T21:07:27Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.swift

+        mapView.addAnnotation(annotation)
+
+        // Calcuate the route from the user's location to the set destination
+        calculateRoute(from: (mapView.userLocation!.coordinate), to: annotation.coordinate) { [unowned self] (route, error) in


There’s no use of self in this callback, so [unowned self] should be unnecessary.

friedbunny · 2017-11-27T21:09:30Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.swift

+        let options = NavigationRouteOptions(waypoints: [origin, destination], profileIdentifier: .automobileAvoidingTraffic)
+
+        // Generate the route object and draw it on the map
+        _ = Directions.shared.calculate(options) { (waypoints, routes, error) in


self is used in this closure — should it be unowned or weak (or stay strong)?

:squints: Looks like we missed this one.

friedbunny · 2017-11-27T21:12:01Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.swift

+
+        // Generate the route object and draw it on the map
+        _ = Directions.shared.calculate(options) { (waypoints, routes, error) in
+            guard let route = routes?.first else { return }


You could skip creating this route variable and directly assign to self.directionsRoute here.

friedbunny · 2017-11-27T21:13:23Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.swift

+
+            // Customize the route line color and width
+            let lineStyle = MGLLineStyleLayer(identifier: "route-style", source: source)
+            lineStyle.lineColor = MGLStyleValue(rawValue:  #colorLiteral(red: 0.1897518039, green: 0.3010634184, blue: 0.7994888425, alpha: 1))


Nit: looks like there’s an extra space before #colorLiteral(.

friedbunny · 2017-11-27T21:17:02Z

Podfile

@@ -10,7 +10,9 @@ target 'Examples' do
 end

 target 'DocsCode' do
-  shared_pods


We might as well remove shared_pods entirely, now that it’s only used for the Examples target.

Probably should also switch the Examples target to use the non-symbols flavor of our SDK, that way we’re not downloading two different versions for this project.

Thanks. 👍

friedbunny · 2017-11-28T00:23:23Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.swift

+        let options = NavigationRouteOptions(waypoints: [origin, destination], profileIdentifier: .automobileAvoidingTraffic)
+
+        // Generate the route object and draw it on the map
+        _ = Directions.shared.calculate(options) { (waypoints, routes, error) in


:squints: Looks like we missed this one.

friedbunny · 2017-11-28T00:24:19Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.m

+
+    // Generate the route object and draw it on the map
+    NSURLSessionDataTask *task = [[MBDirections sharedDirections] calculateDirectionsWithOptions:options completionHandler:^(
+            NSArray<MBWaypoint *> *waypoints,


These look one indent too far, judging by what follows.

friedbunny · 2017-11-28T00:26:07Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.m

+                            if (error != nil) {
+                                printf("Error calculating route");
+                            }
+                        }];


This closing }]; should align with its opening on L71 ([self calc...).

friedbunny · 2017-11-28T00:26:31Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.m

+                     toDestination:annotation.coordinate
+                        completion:^(MBRoute * _Nullable route, NSError * _Nullable error) {
+                            if (error != nil) {
+                                printf("Error calculating route");


Use NSLog() in ObjC.

Also, since we have the error object, let’s print that, too. (Like this.)

friedbunny · 2017-11-28T00:28:59Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.m

+
+// #-code-snippet: navigation calculate-route-objc
+-(void)calculateRoutefromOrigin:(CLLocationCoordinate2D)origin
+                 toDestination :(CLLocationCoordinate2D)destination


Nit: extra space before this colon.

friedbunny · 2017-11-28T00:31:14Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.swift

+        // Calcuate the route from the user's location to the set destination
+        calculateRoute(from: (mapView.userLocation!.coordinate), to: annotation.coordinate) { (route, error) in
+            if error != nil {
+                // Print an error message


The ObjC version doesn’t have this comment — it’s probably unnecessary?

friedbunny · 2017-11-28T00:32:13Z

Podfile

@@ -10,7 +10,9 @@ target 'Examples' do
 end

 target 'DocsCode' do
-  shared_pods


Thanks. 👍

friedbunny · 2017-11-28T00:40:22Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.m

+
+    // #-code-snippet: navigation gesture-recognizer-objc
+    // Add a gesture recognizer to the map view
+    UITapGestureRecognizer *setDestination = [[UILongPressGestureRecognizer alloc] initWithTarget:self action:@selector(didLongPress:)];


This incorrectly declares UILongPressGestureRecognizer as UITapGestureRecognizer .

friedbunny · 2017-11-28T00:43:46Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.m

+    MBNavigationRouteOptions *options = [[MBNavigationRouteOptions alloc] initWithWaypoints:@[originWaypoint, destinationWaypoint] profileIdentifier:MBDirectionsProfileIdentifierAutomobileAvoidingTraffic];
+
+    // Generate the route object and draw it on the map
+    NSURLSessionDataTask *task = [[MBDirections sharedDirections] calculateDirectionsWithOptions:options completionHandler:^(


This line triggers the warning, Unused variable “task”.

Until mapbox/mapbox-directions-swift#209 lands, the way to avoid this is to explicitly discard the result, like so:

(void)[[[MBDirections sharedDirections] calc...

friedbunny · 2017-11-28T00:51:34Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.swift

+        calculateRoute(from: (mapView.userLocation!.coordinate), to: annotation.coordinate) { (route, error) in
+            if error != nil {
+                // Print an error message
+                print("Error calculating route")


friedbunny · 2017-11-28T18:49:53Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.swift

@@ -58,7 +58,7 @@ class ViewController: UIViewController, MGLMapViewDelegate {
        // Calcuate the route from the user's location to the set destination
        calculateRoute(from: (mapView.userLocation!.coordinate), to: annotation.coordinate) { (route, error) in
            if error != nil {
-                print("Error calculating route")
+                NSLog("Error calculating route: \(String(describing: error))")


So print() is the recommended way to put text to console in Swift. 😁 It’s possible to use NSLog() in Swift (and there are behavioral differences that aren’t relevant here), but print() is preferred.

The issue with the ObjC code was in using printf(), which is a C function.

friedbunny · 2017-11-28T18:54:48Z

DocsCode/NavigationTutorial/NavigationTutorialViewController.swift

+        let options = NavigationRouteOptions(waypoints: [origin, destination], profileIdentifier: .automobileAvoidingTraffic)
+
+        // Generate the route object and draw it on the map
+        Directions.shared.calculate(options) { (waypoints, routes, error) in


We need _ = to avoid a warning about an unused return value. We can remove it when mapbox/mapbox-directions-swift#209 makes it into a release.

captainbarbosa requested a review from friedbunny November 21, 2017 02:49

captainbarbosa self-assigned this Nov 21, 2017

friedbunny reviewed Nov 21, 2017

View reviewed changes

captainbarbosa force-pushed the all-the-tuts branch from a262b67 to 3f3dbb6 Compare November 21, 2017 21:02

captainbarbosa added 11 commits November 27, 2017 14:31

Add first steps tutorial

5ff1606

Add data-driven styling tutorial

dfe56ba

Edit magic comment placement syntax for DDS tutorial

Update magic comment syntax on first steps tutorial

67ed234

Update with flyTo snippet

db776e8

Bump min deployment target in Podfile; add Navigation SDK

3ec6b23

Add navigation tutorial

b99d5b6

More edits

7692b91

Add back syntax identifier in code snippet

8b99bb7

Fixes for navigation tutorial

5ac0a23

Update nav sdk to v0.11.0-rc.1

568b40f

Convert navigation tutorial to Swift 4; allow background audio

bfd8f60

captainbarbosa force-pushed the all-the-tuts branch from 6e3a94c to bfd8f60 Compare November 27, 2017 19:31

friedbunny suggested changes Nov 27, 2017

View reviewed changes

captainbarbosa added 8 commits November 27, 2017 17:54

Remove unsightly spaces from objc tutorial

f7494c3

Indent

f69543f

Add comment about coordinate accuracy; include accuracy on Swift version

03382fc

Break down complex line

f6e4f37

Remove unowned self

9db1121

Remove unnecessary route constant

808c8b2

Remove extra space before color literal

341e726

Remove shared pods, use unsymbolicated framework

0ddcc7f

friedbunny suggested changes Nov 28, 2017

View reviewed changes

captainbarbosa added 2 commits November 27, 2017 19:46

Prints to logs

deb4269

Alignment

90b1ae7

captainbarbosa added 5 commits November 27, 2017 19:47

Get outta here ya space

ebe9232

De-indent

8a7ca53

Remove comment

3a89bfb

Small edit

cf142a3

Fix gesture recognizer type

b67c2ab

friedbunny suggested changes Nov 28, 2017

View reviewed changes

captainbarbosa added 2 commits November 27, 2017 19:56

Refactor directions call

956938b

Make errors logs instead of prints

940af9e

friedbunny suggested changes Nov 28, 2017

View reviewed changes

captainbarbosa added 2 commits November 28, 2017 15:37

Feedback edits

f03b674

Add unopened self

ffa0330

friedbunny approved these changes Nov 28, 2017

View reviewed changes

captainbarbosa merged commit 197dd60 into master Nov 28, 2017

captainbarbosa deleted the all-the-tuts branch November 28, 2017 20:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tutorial code from mapbox.com/help #127

Add tutorial code from mapbox.com/help #127

captainbarbosa commented Nov 21, 2017

friedbunny Nov 21, 2017

friedbunny Nov 21, 2017

friedbunny Nov 21, 2017

friedbunny Nov 21, 2017

captainbarbosa Nov 21, 2017

captainbarbosa commented Nov 21, 2017

friedbunny Nov 27, 2017

friedbunny Nov 27, 2017

friedbunny Nov 27, 2017

friedbunny Nov 27, 2017 •

edited

Loading

friedbunny Nov 27, 2017

friedbunny Nov 27, 2017

friedbunny Nov 28, 2017

friedbunny Nov 27, 2017

friedbunny Nov 27, 2017

friedbunny Nov 27, 2017 •

edited

Loading

friedbunny Nov 28, 2017

friedbunny Nov 28, 2017

friedbunny Nov 28, 2017

friedbunny Nov 28, 2017

friedbunny Nov 28, 2017

friedbunny Nov 28, 2017 •

edited

Loading

friedbunny Nov 28, 2017

friedbunny Nov 28, 2017

friedbunny Nov 28, 2017

friedbunny Nov 28, 2017 •

edited

Loading

friedbunny Nov 28, 2017

friedbunny Nov 28, 2017

friedbunny Nov 28, 2017

friedbunny Nov 28, 2017

Add tutorial code from mapbox.com/help #127

Add tutorial code from mapbox.com/help #127

Conversation

captainbarbosa commented Nov 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

captainbarbosa commented Nov 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedbunny Nov 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedbunny Nov 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedbunny Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedbunny Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedbunny Nov 27, 2017 •

edited

Loading

friedbunny Nov 27, 2017 •

edited

Loading

friedbunny Nov 28, 2017 •

edited

Loading

friedbunny Nov 28, 2017 •

edited

Loading