[ios, macos] Update the attribution format for small snapshots. #10456

fabian-guerra · 2017-11-13T22:30:45Z

Fixes #10423

Update helmet image
macOS impl
iOS impl
test cases

fabian-guerra · 2017-11-16T01:50:36Z

This is how it looks.
No logo.

Small logo.

1ec5 · 2017-11-16T03:08:08Z

What changed in mapbox.pdf, other than downgrading from PDF version 1.5 to 1.4?

Also, helmet.pdf appears to be slightly off-center.

fabian-guerra · 2017-11-16T15:04:41Z

I didn't realize mapbox.pdf was changed. I only opened in Xcode. The helmet is an image I made to test. I need the design team give me the right version.

fabian-guerra · 2017-11-17T15:57:57Z

@1ec5 do you think this is OK to go on the final version?

1ec5 · 2017-11-17T17:08:52Z

platform/darwin/src/MGLMapSnapshotter.mm

    return [[NSImage alloc] initWithCGImage:cgimg size:[backgroundImage extent].size];
 #endif
 }

+- (MGLSnapshotAttributionOptions *)attributionOptionsForSize:(CGSize)snapshotSize attributionInfo:(NSArray *)attributionInfo
+{
+    NSMutableArray *attributionOptions = [NSMutableArray array];


It’s a bit confusing that an array of attribution options is called attributionOptions, as though it were only a single MGLSnapshotAttributionOptions object.

1ec5 · 2017-11-17T17:11:10Z

platform/darwin/src/MGLMapSnapshotter.mm

+
+    [attributionOptions addObject:largeLogoAttribution];
+    [attributionOptions addObject:smallLogoAttribution];
+    [attributionOptions addObject:noLogoAttribution];


Can you think of a way to defer the logoImage logic above until we know which size to use, so that we don’t have to put together three MGLSnapshotAttributionOptions objects?

What do you mean? Each of these attribution options represent a rule as in #10423 (comment) which I use to get the best size.

Currently, we precompute the attribution styles before selecting the most appropriate one. My suggestion was to figure out which style would be most appropriate before loading the relevant resources, like the logo, since we ultimately only care about one of the styles. That way, the attributionOptions array becomes unnecessary.

We’re in a hurry to get v3.7.0 out the door, so I’ve outlined the refactor that needs to take place for v3.7.1 in #10490.

1ec5 · 2017-11-17T17:13:48Z

platform/darwin/src/MGLMapSnapshotter.mm

+- (MGLSnapshotAttributionOptions *)attributionOptionsForAttributionInfo:(NSArray *)attributionInfo abbreviated:(BOOL)abbreviated
+{
+    NSString *openStreetMap = @"OpenStreetMap";
+    NSString *OSM = @"OSM";


Make these strings localizable, in case one of the localizations translates “OpenStreetMap” and “OSM” into a different script (as Chinese and Japanese probably should).

1ec5

Some minor changes, but the big stuff is in #10490 now, since we’re in a hurry.

1ec5 · 2017-11-17T19:08:50Z

platform/darwin/src/MGLMapSnapshotter.mm

+            continue;
+        }
+        MGLAttributionInfo *attribution = [info copy];
+        if (abbreviated && [[info.title string] rangeOfString:openStreetMap].location != NSNotFound) {


The string we’re looking for is “OpenStreetMap”, unlocalized, because the most common OpenStreetMap-based sources have this string baked in (mapbox/tilejson-spec#20). If we see “OpenStreetMap” (unlocalized), we should replace it with “OpenStreetMap” (localized) or “OSM” (localized) depending on isAbbreviated.

1ec5 · 2017-11-17T19:09:04Z

platform/darwin/src/MGLMapSnapshotter.mm

+    return noLogoAttribution;
+}
+
+- (MGLSnapshotAttributionOptions *)attributionOptionsForAttributionInfo:(NSArray *)attributionInfo abbreviated:(BOOL)abbreviated


abbreviated:(BOOL)isAbbreviated

1ec5 · 2017-11-17T19:10:22Z

platform/darwin/src/MGLMapSnapshotter.mm

+- (MGLSnapshotAttributionOptions *)attributionOptionsForAttributionInfo:(NSArray *)attributionInfo abbreviated:(BOOL)abbreviated
+{
+    NSString *openStreetMap = NSLocalizedStringWithDefaultValue(@"OPEN_STREET_MAP_ATTRIBUTION", nil, nil, @"OpenStreetMap", @"Open Street Map full name attribution");
+    NSString *OSM = NSLocalizedStringWithDefaultValue(@"OSM_ATTRIBUTION", nil, nil, @"OSM", @"Open Street Map short name attribution");


“OpenStreetMap” has no spaces. The keys should be OSM_LONG_NAME and OSM_SHORT_NAME, respectively, and the comments should be “Full/abbreviated name of OpenStreetMap in attribution text.”

1ec5 · 2017-11-17T19:10:33Z

platform/ios/resources/Base.lproj/Localizable.strings

@@ -64,6 +64,9 @@
 /* User-friendly error description */
 "PARSE_STYLE_FAILED_DESC" = "The map failed to load because the style is corrupted.";

+/* String format for accessibility value for road feature; {starting compass direction}, {ending compass direction} */
+"ROAD_DIRECTION_A11Y_FMT" = "%1$@ to %2$@";


Thanks for catching this!

1ec5 · 2017-11-17T19:14:46Z

I updated the path to Localizable.strings in Transifex to point to the release-agua branch, so that translators can get started right away.

1ec5 · 2017-11-17T20:08:26Z

platform/darwin/src/MGLMapSnapshotter.mm

+        }
+        MGLAttributionInfo *attribution = [info copy];
+        NSMutableAttributedString *title = [[NSMutableAttributedString alloc] initWithAttributedString:info.title];
+        [title removeAttribute:NSUnderlineStyleAttributeName range:NSMakeRange(0, [title.string length])];


Technically a source can underline something that isn’t a link, using <u> for example. The more correct attribute to remove is the link attribute, which would be nonfunctional on an image.

I've tried that NSLinkAttributeName but didn't work that's why I used NSUnderlineStyleAttributeName

Ah, it probably has to do with the HTML-to-attributed-string conversion. I think that step started forcefully applying an underline (and color) around the Sierra timeframe. I’m fine with this change (which fixes #10493), but can you add a comment explaining the removal, in case we need to accommodate intentional underlining in the future?

fabian-guerra self-assigned this Nov 13, 2017

1ec5 added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS labels Nov 14, 2017

fabian-guerra force-pushed the fabian-narrow-attribution-10423 branch from e23d217 to 90f6cc7 Compare November 16, 2017 01:43

fabian-guerra requested a review from 1ec5 November 16, 2017 01:46

fabian-guerra force-pushed the fabian-narrow-attribution-10423 branch 2 times, most recently from d8413c5 to f25bb65 Compare November 17, 2017 15:54

1ec5 reviewed Nov 17, 2017

View reviewed changes

fabian-guerra force-pushed the fabian-narrow-attribution-10423 branch from f25bb65 to ab8203a Compare November 17, 2017 17:39

1ec5 mentioned this pull request Nov 17, 2017

Refactor MGLMapSnapshotter attribution #10490

Closed

1ec5 approved these changes Nov 17, 2017

View reviewed changes

fabian-guerra force-pushed the fabian-narrow-attribution-10423 branch from ab8203a to e8d8e1f Compare November 17, 2017 19:13

fabian-guerra force-pushed the fabian-narrow-attribution-10423 branch 2 times, most recently from 9a248de to c85fc0a Compare November 17, 2017 20:07

[ios, macos] Update the attribution format for small snapshots.

aadd25c

1ec5 reviewed Nov 17, 2017

View reviewed changes

fabian-guerra force-pushed the fabian-narrow-attribution-10423 branch from c85fc0a to aadd25c Compare November 17, 2017 20:08

1ec5 mentioned this pull request Nov 17, 2017

Remove underline from static snapshot attribution text #10493

Closed

fabian-guerra merged commit 5eb050d into release-agua Nov 17, 2017

fabian-guerra deleted the fabian-narrow-attribution-10423 branch November 17, 2017 21:40

1ec5 mentioned this pull request Dec 18, 2017

Clean up base localization files #10731

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ios, macos] Update the attribution format for small snapshots. #10456

[ios, macos] Update the attribution format for small snapshots. #10456

fabian-guerra commented Nov 13, 2017 •

edited

Loading

fabian-guerra commented Nov 16, 2017

1ec5 commented Nov 16, 2017

fabian-guerra commented Nov 16, 2017

fabian-guerra commented Nov 17, 2017

1ec5 Nov 17, 2017

1ec5 Nov 17, 2017

fabian-guerra Nov 17, 2017

1ec5 Nov 17, 2017

1ec5 Nov 17, 2017

1ec5 Nov 17, 2017

1ec5 left a comment

1ec5 Nov 17, 2017

1ec5 Nov 17, 2017

1ec5 Nov 17, 2017

1ec5 Nov 17, 2017

1ec5 commented Nov 17, 2017

1ec5 Nov 17, 2017

fabian-guerra Nov 17, 2017

1ec5 Nov 17, 2017

[ios, macos] Update the attribution format for small snapshots. #10456

[ios, macos] Update the attribution format for small snapshots. #10456

Conversation

fabian-guerra commented Nov 13, 2017 • edited Loading

fabian-guerra commented Nov 16, 2017

1ec5 commented Nov 16, 2017

fabian-guerra commented Nov 16, 2017

fabian-guerra commented Nov 17, 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

Choose a reason for hiding this comment

1ec5 left a 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

1ec5 commented Nov 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabian-guerra commented Nov 13, 2017 •

edited

Loading