Migrate to GL JS–powered feedback form (#9078)

1ec5 · web-flow · commit a39116a552bb · 2017-05-30T15:14:00.000-07:00
* [ios, macos] Updated feedback URL

* [ios, macos] Add referrer, heading, pitch to feedback URL

* [ios, macos] Updated changelogs for feedback changes

* [ios] Vary referrer by platform
diff --git a/platform/darwin/src/MGLAttributionInfo.mm b/platform/darwin/src/MGLAttributionInfo.mm
@@ -6,8 +6,10 @@
     #import <Cocoa/Cocoa.h>
 #endif
 
+#import "MGLAccountManager.h"
 #import "MGLMapCamera.h"
 #import "NSArray+MGLAdditions.h"
+#import "NSBundle+MGLAdditions.h"
 #import "NSString+MGLAdditions.h"
 
 #include <string>
@@ -126,13 +128,34 @@ - (instancetype)initWithTitle:(NSAttributedString *)title URL:(NSURL *)URL {
 }
 
 - (nullable NSURL *)feedbackURLAtCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate zoomLevel:(double)zoomLevel {
+    return [self feedbackURLForStyleURL:nil atCenterCoordinate:centerCoordinate zoomLevel:zoomLevel direction:0 pitch:0];
+}
+
+- (nullable NSURL *)feedbackURLForStyleURL:(nullable NSURL *)styleURL atCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate zoomLevel:(double)zoomLevel direction:(CLLocationDirection)direction pitch:(CGFloat)pitch {
     if (!self.feedbackLink) {
         return nil;
     }
-
-    NSURLComponents *components = [NSURLComponents componentsWithURL:self.URL resolvingAgainstBaseURL:NO];
-    components.fragment = [NSString stringWithFormat:@"/%.5f/%.5f/%i",
-                           centerCoordinate.longitude, centerCoordinate.latitude, (int)round(zoomLevel + 1)];
+    
+    NSURLComponents *components = [NSURLComponents componentsWithString:@"https://www.mapbox.com/feedback/"];
+    components.fragment = [NSString stringWithFormat:@"/%.5f/%.5f/%.2f/%.1f/%i",
+                           centerCoordinate.longitude, centerCoordinate.latitude, zoomLevel,
+                           direction, (int)round(pitch)];
+    
+    NSURLQueryItem *referrerQueryItem = [NSURLQueryItem queryItemWithName:@"referrer"
+                                                                    value:[NSBundle mgl_applicationBundleIdentifier]];
+    NSMutableArray<NSURLQueryItem *> *queryItems = [NSMutableArray arrayWithObject:referrerQueryItem];
+    if ([styleURL.scheme isEqualToString:@"mapbox"] && [styleURL.host isEqualToString:@"styles"]) {
+        NSArray<NSString *> *stylePathComponents = styleURL.pathComponents;
+        if (stylePathComponents.count >= 3) {
+            [queryItems addObjectsFromArray:@[
+                [NSURLQueryItem queryItemWithName:@"owner" value:stylePathComponents[1]],
+                [NSURLQueryItem queryItemWithName:@"id" value:stylePathComponents[2]],
+                [NSURLQueryItem queryItemWithName:@"access_token" value:[MGLAccountManager accessToken]],
+            ]];
+        }
+    }
+    components.queryItems = queryItems;
+    
     return components.URL;
 }
 
diff --git a/platform/darwin/src/MGLAttributionInfo_Private.h b/platform/darwin/src/MGLAttributionInfo_Private.h
@@ -20,6 +20,24 @@ NS_ASSUME_NONNULL_BEGIN
 
 + (NSAttributedString *)attributedStringForAttributionInfos:(NS_ARRAY_OF(MGLAttributionInfo *) *)attributionInfos;
 
+/**
+ Returns a copy of the `URL` property modified to account for the given style
+ URL, center coordinate, and zoom level.
+
+ @param styleURL The map’s style URL.
+ @param centerCoordinate The map’s center coordinate.
+ @param zoomLevel The map’s zoom level. See the `MGLMapView.zoomLevel` property
+    for more information.
+ @param direction The heading of the map, measured in degrees clockwise from
+    true north.
+ @param pitch Pitch toward the horizon measured in degrees, with 0 degrees
+    resulting in a two-dimensional map.
+ @return A modified URL containing a fragment that points to the specified
+    viewport. If the `feedbackLink` property is set to `NO`, this method returns
+    `nil`.
+ */
+- (nullable NSURL *)feedbackURLForStyleURL:(nullable NSURL *)styleURL atCenterCoordinate:(CLLocationCoordinate2D)centerCoordinate zoomLevel:(double)zoomLevel direction:(CLLocationDirection)direction pitch:(CGFloat)pitch;
+
 @end
 
 @interface NSMutableArray (MGLAttributionInfoAdditions)
diff --git a/platform/darwin/src/MGLOfflineStorage.mm b/platform/darwin/src/MGLOfflineStorage.mm
@@ -7,6 +7,7 @@
 #import "MGLOfflinePack_Private.h"
 #import "MGLOfflineRegion_Private.h"
 #import "MGLTilePyramidOfflineRegion.h"
+#import "NSBundle+MGLAdditions.h"
 #import "NSValue+MGLAdditions.h"
 
 #include <mbgl/util/string.hpp>
@@ -132,7 +133,7 @@ + (NSURL *)cacheURLIncludingSubdirectory:(BOOL)useSubdirectory {
                                                              appropriateForURL:nil
                                                                         create:YES
                                                                          error:nil];
-    NSString *bundleIdentifier = [self bundleIdentifier];
+    NSString *bundleIdentifier = [NSBundle mgl_applicationBundleIdentifier];
     if (!bundleIdentifier) {
         // There’s no main bundle identifier when running in a unit test bundle.
         bundleIdentifier = [NSBundle bundleForClass:self].bundleIdentifier;
@@ -166,7 +167,7 @@ + (NSString *)legacyCachePath {
     NSString *legacyCachePath = [legacyPaths.firstObject stringByAppendingPathComponent:MGLOfflineStorageFileName3_2_0_beta_1];
 #elif TARGET_OS_MAC
     // ~/Library/Caches/tld.app.bundle.id/offline.db
-    NSString *bundleIdentifier = [self bundleIdentifier];
+    NSString *bundleIdentifier = [NSBundle mgl_applicationBundleIdentifier];
     NSURL *legacyCacheDirectoryURL = [[NSFileManager defaultManager] URLForDirectory:NSCachesDirectory
                                                                             inDomain:NSUserDomainMask
                                                                    appropriateForURL:nil
@@ -219,15 +220,6 @@ - (instancetype)init {
     return self;
 }
 
-+ (NSString *)bundleIdentifier {
-    NSString *bundleIdentifier = [NSBundle mainBundle].bundleIdentifier;
-    if (!bundleIdentifier) {
-        // There’s no main bundle identifier when running in a unit test bundle.
-        bundleIdentifier = [NSBundle bundleForClass:self].bundleIdentifier;
-    }
-    return bundleIdentifier;
-}
-
 - (void)dealloc {
     [[NSNotificationCenter defaultCenter] removeObserver:self];
     [[MGLNetworkConfiguration sharedManager] removeObserver:self forKeyPath:@"apiBaseURL"];
diff --git a/platform/darwin/src/NSBundle+MGLAdditions.h b/platform/darwin/src/NSBundle+MGLAdditions.h
@@ -36,6 +36,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 + (nullable NS_DICTIONARY_OF(NSString *, id) *)mgl_frameworkInfoDictionary;
 
++ (nullable NSString *)mgl_applicationBundleIdentifier;
+
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/platform/darwin/src/NSBundle+MGLAdditions.m b/platform/darwin/src/NSBundle+MGLAdditions.m
@@ -31,4 +31,13 @@ + (nullable NSString *)mgl_frameworkBundleIdentifier {
     return bundle.infoDictionary;
 }
 
++ (nullable NSString *)mgl_applicationBundleIdentifier {
+    NSString *bundleIdentifier = [NSBundle mainBundle].bundleIdentifier;
+    if (!bundleIdentifier) {
+        // There’s no main bundle identifier when running in a unit test bundle.
+        bundleIdentifier = [NSBundle bundleForClass:[MGLAccountManager class]].bundleIdentifier;
+    }
+    return bundleIdentifier;
+}
+
 @end
diff --git a/platform/darwin/test/MGLAttributionInfoTests.m b/platform/darwin/test/MGLAttributionInfoTests.m
@@ -46,8 +46,18 @@ - (void)testParsing {
     XCTAssertEqualObjects(infos[3].title.string, @"Improve this map");
     XCTAssertEqualObjects(infos[3].URL, [NSURL URLWithString:@"https://www.mapbox.com/map-feedback/"]);
     XCTAssertTrue(infos[3].feedbackLink);
+    NSURL *styleURL = [MGLStyle satelliteStreetsStyleURLWithVersion:99];
+#if TARGET_OS_IPHONE
+    XCTAssertEqualObjects([infos[3] feedbackURLAtCenterCoordinate:mapbox zoomLevel:14],
+                          [NSURL URLWithString:@"https://www.mapbox.com/feedback/?referrer=com.mapbox.sdk.ios#/77.63680/12.98108/14.00/0.0/0"]);
+    XCTAssertEqualObjects([infos[3] feedbackURLForStyleURL:styleURL atCenterCoordinate:mapbox zoomLevel:3.14159 direction:90.9 pitch:12.5],
+                          [NSURL URLWithString:@"https://www.mapbox.com/feedback/?referrer=com.mapbox.sdk.ios&owner=mapbox&id=satellite-streets-v99&access_token#/77.63680/12.98108/3.14/90.9/13"]);
+#else
     XCTAssertEqualObjects([infos[3] feedbackURLAtCenterCoordinate:mapbox zoomLevel:14],
-                          [NSURL URLWithString:@"https://www.mapbox.com/map-feedback/#/77.63680/12.98108/15"]);
+                          [NSURL URLWithString:@"https://www.mapbox.com/feedback/?referrer=com.mapbox.MapboxGL#/77.63680/12.98108/14.00/0.0/0"]);
+    XCTAssertEqualObjects([infos[3] feedbackURLForStyleURL:styleURL atCenterCoordinate:mapbox zoomLevel:3.14159 direction:90.9 pitch:12.5],
+                          [NSURL URLWithString:@"https://www.mapbox.com/feedback/?referrer=com.mapbox.MapboxGL&owner=mapbox&id=satellite-streets-v99&access_token#/77.63680/12.98108/3.14/90.9/13"]);
+#endif
 }
 
 - (void)testStyle {
diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md
@@ -37,6 +37,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 * Fixed an issue where gesture recognizers associated with map view interactivity were not disabled when their related interactions were disabled. ([#8304](https://github.com/mapbox/mapbox-gl-native/pull/8304))
 * Fixed an issue preventing the Mapbox Telemetry confirmation dialog from appearing when opened from within a map view in a modal view controller. ([#9027](https://github.com/mapbox/mapbox-gl-native/pull/9027))
 * Corrected the size of MGLMapView’s compass. ([#9060](https://github.com/mapbox/mapbox-gl-native/pull/9060))
+* The Improve This Map button in the attribution action sheet now leads to a feedback tool that matches MGLMapView’s rotation and pitch. `-[MGLAttributionInfo feedbackURLAtCenterCoordinate:zoomLevel:]` no longer respects the feedback URL specified in TileJSON. ([#9078](https://github.com/mapbox/mapbox-gl-native/pull/9078))
 
 ### Other changes
 
diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
@@ -64,7 +64,7 @@
 #import "MGLCompactCalloutView.h"
 #import "MGLAnnotationContainerView.h"
 #import "MGLAnnotationContainerView_Private.h"
-#import "MGLAttributionInfo.h"
+#import "MGLAttributionInfo_Private.h"
 
 #include <algorithm>
 #include <cstdlib>
@@ -1895,8 +1895,12 @@ - (void)showAttribution
             {
                 if (info.feedbackLink)
                 {
-                    url = [info feedbackURLAtCenterCoordinate:self.centerCoordinate
-                                                    zoomLevel:self.zoomLevel];
+                    MGLMapCamera *camera = self.camera;
+                    url = [info feedbackURLForStyleURL:self.styleURL
+                                    atCenterCoordinate:camera.centerCoordinate
+                                             zoomLevel:self.zoomLevel
+                                             direction:camera.heading
+                                                 pitch:camera.pitch];
                 }
                 [[UIApplication sharedApplication] openURL:url];
             }
diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md
@@ -25,6 +25,7 @@
 * Fixed a crash when calling `MGLMultiPolygon.coordinate` [#8713](https://github.com/mapbox/mapbox-gl-native/pull/8713)
 * Fixed an issue causing attribution button text to appear blue instead of black. ([#8701](https://github.com/mapbox/mapbox-gl-native/pull/8701))
 * Fixed a crash or console spew when MGLMapView is initialized with a frame smaller than 64 points wide by 64 points tall. ([#8562](https://github.com/mapbox/mapbox-gl-native/pull/8562))
+* The Improve This Map button in the attribution action sheet now leads to a feedback tool that matches MGLMapView’s rotation and pitch. `-[MGLAttributionInfo feedbackURLAtCenterCoordinate:zoomLevel:]` no longer respects the feedback URL specified in TileJSON. ([#9078](https://github.com/mapbox/mapbox-gl-native/pull/9078))
 * The error passed into `-[MGLMapViewDelegate mapViewDidFailLoadingMap:withError:]` now includes a more specific description and failure reason. ([#8418](https://github.com/mapbox/mapbox-gl-native/pull/8418))
 * The `MGLPolyline.coordinate` and `MGLPolygon.coordinate` properties now return the midpoint and centroid, respectively, instead of the first coordinate. ([#8713](https://github.com/mapbox/mapbox-gl-native/pull/8713))
 * Improved CPU and battery performance while animating a tilted map’s camera in an area with many labels. ([#9031](https://github.com/mapbox/mapbox-gl-native/pull/9031))
diff --git a/platform/macos/src/MGLMapView.mm b/platform/macos/src/MGLMapView.mm
@@ -1,17 +1,18 @@
 #import "MGLMapView_Private.h"
-#import "MGLAnnotationImage_Private.h"
+
 #import "MGLAttributionButton.h"
-#import "MGLAttributionInfo.h"
 #import "MGLCompassCell.h"
 #import "MGLOpenGLLayer.h"
 #import "MGLStyle.h"
 
+#import "MGLAnnotationImage_Private.h"
+#import "MGLAttributionInfo_Private.h"
 #import "MGLFeature_Private.h"
+#import "MGLFoundation_Private.h"
 #import "MGLGeometry_Private.h"
 #import "MGLMultiPoint_Private.h"
 #import "MGLOfflineStorage_Private.h"
 #import "MGLStyle_Private.h"
-#import "MGLFoundation_Private.h"
 
 #import "MGLAccountManager.h"
 #import "MGLMapCamera.h"
@@ -1735,11 +1736,15 @@ - (IBAction)rotate:(NSSlider *)sender {
 }
 
 - (IBAction)giveFeedback:(id)sender {
-    CLLocationCoordinate2D centerCoordinate = self.centerCoordinate;
+    MGLMapCamera *camera = self.camera;
     double zoomLevel = self.zoomLevel;
     NSMutableArray *urls = [NSMutableArray array];
     for (MGLAttributionInfo *info in [self.style attributionInfosWithFontSize:0 linkColor:nil]) {
-        NSURL *url = [info feedbackURLAtCenterCoordinate:centerCoordinate zoomLevel:zoomLevel];
+        NSURL *url = [info feedbackURLForStyleURL:self.styleURL
+                               atCenterCoordinate:camera.centerCoordinate
+                                        zoomLevel:zoomLevel
+                                        direction:camera.heading
+                                            pitch:camera.pitch];
         if (url) {
             [urls addObject:url];
         }

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@`
`64`	`64`	`#import "MGLCompactCalloutView.h"`
`65`	`65`	`#import "MGLAnnotationContainerView.h"`
`66`	`66`	`#import "MGLAnnotationContainerView_Private.h"`
`67`		`-#import "MGLAttributionInfo.h"`
	`67`	`+#import "MGLAttributionInfo_Private.h"`
`68`	`68`
`69`	`69`	`#include <algorithm>`
`70`	`70`	`#include <cstdlib>`
`@@ -1895,8 +1895,12 @@ - (void)showAttribution`
`1895`	`1895`	`{`
`1896`	`1896`	`if (info.feedbackLink)`
`1897`	`1897`	`{`
`1898`		`- url = [info feedbackURLAtCenterCoordinate:self.centerCoordinate`
`1899`		`- zoomLevel:self.zoomLevel];`
	`1898`	`+ MGLMapCamera *camera = self.camera;`
	`1899`	`+ url = [info feedbackURLForStyleURL:self.styleURL`
	`1900`	`+ atCenterCoordinate:camera.centerCoordinate`
	`1901`	`+ zoomLevel:self.zoomLevel`
	`1902`	`+ direction:camera.heading`
	`1903`	`+ pitch:camera.pitch];`
`1900`	`1904`	`}`
`1901`	`1905`	`[[UIApplication sharedApplication] openURL:url];`
`1902`	`1906`	`}`