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

[ios] fixes #5127, #5128, #5130 added enabled and selected property to MGLAnnotationView #5297

Merged
merged 1 commit into from
Jun 14, 2016

Conversation

frederoni
Copy link
Contributor

@frederoni frederoni commented Jun 9, 2016

Fixes #5127 #5128 #5130

Added selected and enabled property to MGLAnnotationView and hooked it up.
Added mapView:did(De)SelectAnnotationView: protocol methods.

@boundsj
I kept the annotation and annotation views selection protocol methods separated but let me know if you want me to combine them into mapView:did(De)SelectAnnotation:annotationView:

/cc @1ec5 @friedbunny

@frederoni frederoni added iOS Mapbox Maps SDK for iOS ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold annotations Annotations on iOS and macOS or markers on Android labels Jun 9, 2016
@@ -41,6 +41,19 @@ NS_ASSUME_NONNULL_BEGIN
@property (nonatomic, assign, getter=isFlat) BOOL flat;

/**
This property defaults to NO and becomes YES when the view is tapped/clicked on.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Let’s remove the word “clicked”; that term is only appropriate for OS X, and this file isn’t shared with the OS X SDK.

Copy link
Contributor

Choose a reason for hiding this comment

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

As with the MKAnnotationView.selected property, the documentation for this property should mention that you shouldn’t set it directly.

@frederoni
Copy link
Contributor Author

That should cover it. Let me know if there's anything else @1ec5

@@ -41,10 +41,17 @@ NS_ASSUME_NONNULL_BEGIN
@property (nonatomic, assign, getter=isFlat) BOOL flat;

/**
This property defaults to NO and becomes YES when the view is tapped/clicked on.
This property should not be changed directly.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, this line should actually be at the end of the comment, rather than the beginning. The first several words of a documentation comment appear in the code completion popup, so it should be descriptive and reasonably unique.

Though it’s intended for Swift and Markdown instead of Objective-C and HeaderDoc, this Swift API design guideline document has a great guideline on forming documentation comments (scroll up and click “Expand all details now” to see it).

@1ec5
Copy link
Contributor

1ec5 commented Jun 9, 2016

This looks great, other than the one comment about comments.

@frederoni frederoni force-pushed the 5128-selected-annotation-view branch from 8f2c430 to 968afc3 Compare June 10, 2016 09:29
@frederoni frederoni changed the title [ios] fixes #5128 selected property to MGLAnnotationView [ios] fixes #5127, #5128 added enabled and selected property to MGLAnnotationView Jun 10, 2016
@frederoni frederoni removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Jun 10, 2016
@frederoni frederoni force-pushed the 5128-selected-annotation-view branch from 968afc3 to 1e75237 Compare June 13, 2016 14:51
@frederoni frederoni added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Jun 13, 2016
@1ec5
Copy link
Contributor

1ec5 commented Jun 14, 2016

👍

@frederoni frederoni changed the title [ios] fixes #5127, #5128 added enabled and selected property to MGLAnnotationView [ios] fixes #5127, #5128, #5130 added enabled and selected property to MGLAnnotationView Jun 14, 2016
@frederoni frederoni removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Jun 14, 2016
@frederoni frederoni merged commit a557e1e into release-ios-v3.3.0 Jun 14, 2016
@frederoni frederoni deleted the 5128-selected-annotation-view branch June 14, 2016 11:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
annotations Annotations on iOS and macOS or markers on Android iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants