Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Migrate to GL JS–powered feedback form #9078

Merged
merged 4 commits into from
May 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions platform/darwin/src/MGLAttributionInfo.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This format string hard-codes precisions for the various components instead of varying the latitude and longitude precision by the zoom level. Do you think that would be a problem @tristen @mollymerp?

Copy link
Contributor

Choose a reason for hiding this comment

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

if I'm reading correctly and you're just automatically providing lng/lat precision down to 5 decimal places I think you should be fine. In GL-JS we provide more precision the higher the ZL, but adding additional precision at high ZL shouldn't be a problem.

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;
}

Expand Down
18 changes: 18 additions & 0 deletions platform/darwin/src/MGLAttributionInfo_Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 3 additions & 11 deletions platform/darwin/src/MGLOfflineStorage.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"];
Expand Down
2 changes: 2 additions & 0 deletions platform/darwin/src/NSBundle+MGLAdditions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions platform/darwin/src/NSBundle+MGLAdditions.m
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 11 additions & 1 deletion platform/darwin/test/MGLAttributionInfoTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 7 additions & 3 deletions platform/ios/src/MGLMapView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -1907,8 +1907,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];
}
Expand Down
1 change: 1 addition & 0 deletions platform/macos/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
15 changes: 10 additions & 5 deletions platform/macos/src/MGLMapView.mm
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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];
}
Expand Down