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

[ios, macos] Fix querying features returning nil when features available. #9784

Merged
merged 2 commits into from
Aug 17, 2017
Merged

[ios, macos] Fix querying features returning nil when features available. #9784

merged 2 commits into from
Aug 17, 2017

Conversation

fabian-guerra
Copy link
Contributor

Fixes #9646

@fabian-guerra fabian-guerra self-assigned this Aug 16, 2017
@fabian-guerra fabian-guerra added the iOS Mapbox Maps SDK for iOS label Aug 16, 2017
@fabian-guerra fabian-guerra added this to the ios-v3.6.2 milestone Aug 16, 2017
jfirebaugh
jfirebaugh reviewed Aug 16, 2017
View reviewed changes
Copy link
Contributor

@jfirebaugh jfirebaugh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Oh, good catch.

Would it be better to fix this by adding a style:(MGLStyle*) parameter to -[MGLSource initWithRawSource:], keeping the style property readonly?

Also, is there somewhere you can add a test for this?

1ec5
1ec5 reviewed Aug 16, 2017
View reviewed changes
Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

I agree that we probably should add a parameter to -[MGLSource initWithRawSource:] and its subclass implementations instead of making mapView writeable.

Also, this bug fix is significant enough to deserve a mention in the iOS and macOS changelogs.

platform/darwin/src/MGLStyle.mm Outdated
@@ -187,7 +187,13 @@ - (MGLSource *)memberOfSources:(MGLSource *)object {
- (MGLSource *)sourceWithIdentifier:(NSString *)identifier
{
auto rawSource = self.mapView.mbglMap->getSource(identifier.UTF8String);
return rawSource ? [self sourceFromMBGLSource:rawSource] : nil;
MGLSource *source = nil;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: it isn’t necessary to explicitly set a local variable to nil in Objective-C.

@1ec5 1ec5 added the macOS Mapbox Maps SDK for macOS label Aug 16, 2017
@1ec5 1ec5 changed the title [ios] Fix querying features returning nil when features available. [ios, macos] Fix querying features returning nil when features available. Aug 16, 2017
@fabian-guerra fabian-guerra force-pushed the fabian-empty-features-9646 branch 3 times, most recently from 79c088e to fd3002a Compare August 16, 2017 23:10
1ec5
1ec5 reviewed Aug 17, 2017
View reviewed changes
platform/darwin/src/MGLStyle.mm Outdated
@@ -187,7 +187,8 @@ - (MGLSource *)memberOfSources:(MGLSource *)object {
- (MGLSource *)sourceWithIdentifier:(NSString *)identifier
{
auto rawSource = self.mapView.mbglMap->getSource(identifier.UTF8String);
return rawSource ? [self sourceFromMBGLSource:rawSource] : nil;

return rawSource ? [self sourceFromMBGLSource:rawSource] : nil;;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: stray semicolon.

platform/ios/CHANGELOG.md Outdated
@@ -7,6 +7,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
* Added an MGLStyle.localizesLabels property, off by default, that localizes any Mapbox Streets–sourced symbol layer into the user’s preferred language. ([#9582](https://github.com/mapbox/mapbox-gl-native/pull/9582))
* Added an additional camera method to MGLMapView that accepts an edge padding parameter. ([#9651](https://github.com/mapbox/mapbox-gl-native/pull/9651))
* Fixed an issue with the scaling of the user location annotation’s horizontal accuracy indicator. ([#9721](https://github.com/mapbox/mapbox-gl-native/pull/9721))
* Fixed an issue that caused querying features in a source added through Mapbox Studio returned nil. ([#9784](https://github.com/mapbox/mapbox-gl-native/pull/9784))
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: s/returned/to return/. Also, if I understand correctly, the issue affected all vector sources, not just those added through Mapbox Studio.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right.

@fabian-guerra fabian-guerra force-pushed the fabian-empty-features-9646 branch from fd3002a to defbfa5 Compare August 17, 2017 13:36
@1ec5
[ios, macos] Copyedited changelogs
fdf7a3f 
Also, macOS SDK v0.5.1 has yet to be released, because none of the changes that went into iOS SDK v3.6.1 were relevant to macOS.
@1ec5
Copy link
Contributor

1ec5 commented Aug 17, 2017

Made some changes to the changelogs in fdf7a3f:

  • The blurb now refers to the specific affected methods, to better distinguish between source querying and layer querying.
  • macOS SDK v0.5.1 has yet to be released, because none of the changes that went into iOS SDK v3.6.1 were relevant to macOS.

@fabian-guerra
Copy link
Contributor Author

Thanks @1ec5!

@fabian-guerra fabian-guerra merged commit c7d7319 into release-ios-v3.6.0-android-v5.1.0 Aug 17, 2017
@fabian-guerra fabian-guerra deleted the fabian-empty-features-9646 branch August 17, 2017 23:19
This was referenced Aug 17, 2017
Closed
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jfirebaugh jfirebaugh jfirebaugh left review comments

@1ec5 1ec5 1ec5 approved these changes

Assignees

@fabian-guerra fabian-guerra

Labels
iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS
Projects
None yet
Milestone
ios-v3.6.2
Development

Successfully merging this pull request may close these issues.
3 participants
@fabian-guerra @1ec5 @jfirebaugh