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

Update annotations and annotation images dynamically #3835

Merged
merged 5 commits into from
Apr 19, 2016

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Feb 5, 2016

MGLMapView observes changes to the coordinate property of each MGLAnnotation added to it. Changing the coordinate property in a KVO-compliant way causes the annotation to be relocated and its callout view, if present, to be dismissed. To avoid observing the same annotation twice yet also avoid expensive lookups when adding or removing annotations, MGLMapView indexes added point annotations in an NSMutableSet.

Added the ability to delete an unused annotation image’s images on iOS. (The OS X SDK doesn’t yet support changing an annotation image’s image.) When you nil out the image of an MGLAnnotationImage, MGLMapView deletes the sprite from the style and resets any annotation associated with the MGLAnnotationImage instance to use the default red pin icon under the hood. The annotation remains associated with the MGLAnnotationImage, which comes in handy in the event that the annotation image’s image becomes non-nil.

In iosapp, tapping a callout view moves the selected annotation to the center of the screen and deselects it. Deselecting an annotation resets its image to the default; deselecting it again restores the image.

Fixes #1980.

/ref #3185
/cc @boundsj @friedbunny

@1ec5 1ec5 added feature iOS Mapbox Maps SDK for iOS refactor labels Feb 5, 2016
@1ec5 1ec5 self-assigned this Feb 5, 2016
@1ec5 1ec5 added this to the ios-v3.2.0 milestone Feb 5, 2016
@@ -169,6 +169,7 @@ - (void)showSettings
@"Add Test Shapes",
@"Start World Tour",
@"Add Custom Callout Point",
@"Reset Annotation Images",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not seeing a new handler for this action.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I didn’t end up needing that action. I’ll remove it.

@1ec5
Copy link
Contributor Author

1ec5 commented Feb 10, 2016

Implementing the annotation replacement in mbgl, as in #3885, would probably be more efficient.

@1ec5 1ec5 modified the milestones: ios-v3.3.0, ios-v3.2.0 Mar 3, 2016
@1ec5 1ec5 modified the milestones: ios-v3.2.0, ios-v3.3.0 Apr 3, 2016
@1ec5
Copy link
Contributor Author

1ec5 commented Apr 3, 2016

Moving to 3.3.0 milestone in light of #4556.

@1ec5 1ec5 force-pushed the 1ec5-annotation-image-delete-3185 branch from e0199a7 to b69e525 Compare April 17, 2016 18:06
@1ec5 1ec5 changed the title Annotation image deletion Annotation image replacement and deletion Apr 17, 2016
@1ec5 1ec5 changed the title Annotation image replacement and deletion Update annotations and annotation images dynamically Apr 17, 2016
@1ec5
Copy link
Contributor Author

1ec5 commented Apr 17, 2016

I added support for atomically relocating an annotation without having to replace the annotation outright. The PR description has been updated.

MGLAnnotationContext context;
context.annotation = annotations[i];
context.symbolIdentifier = @(points[i].icon.c_str());
context.imageReuseIdentifier = annotationImage.reuseIdentifier;
Copy link
Contributor

Choose a reason for hiding this comment

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

Great! I made a similar refactor on my annotation branch. This is better and includes the new and better property name on MGLAnnotationContext

@boundsj
Copy link
Contributor

boundsj commented Apr 19, 2016

👍 this looks great! I will base the change I'm about to make for a custom view off of this branch until it lands in master.

Added an API for deleting unused annotation images’ images. When you nil out the image of an MGLAnnotationImage, MGLMapView deletes the sprite from the style and recreates any annotation associated with the MGLAnnotationImage instance; the MGLAnnotationImage’s falls back to SDK’s default annotation image.

In iosapp, deselecting an annotation resets its image to the default; deselecting it again restores the image.

ref #3185
When refreshing an annotation’s image, update the annotation instead of replacing it outright.
MGLMapView observes changes to the coordinate property of each MGLAnnotation added to it. Changing the coordinate property in a KVO-compliant way causes the annotation to be relocated and its callout view, if present, to be dismissed. To avoid observing the same annotation twice yet also avoid expensive lookups when adding or removing annotations, MGLMapView indexes added point annotations in an NSMutableSet.

In iosapp, tapping a callout view moves the selected annotation to the center of the screen and deselects it.

Fixes #1980.
MGLMapView observes changes to the coordinate property of each MGLAnnotation added to it. Changing the coordinate property in a KVO-compliant way causes the annotation to be relocated and its callout view, if present, to be dismissed. To avoid observing the same annotation twice yet also avoid expensive lookups when adding or removing annotations, MGLMapView indexes added point annotations in an NSMutableSet.
@1ec5 1ec5 force-pushed the 1ec5-annotation-image-delete-3185 branch from 7401aab to f8c52d5 Compare April 19, 2016 21:25
@1ec5 1ec5 added the macOS Mapbox Maps SDK for macOS label Apr 19, 2016
@1ec5 1ec5 merged commit f8c52d5 into master Apr 19, 2016
@1ec5 1ec5 deleted the 1ec5-annotation-image-delete-3185 branch April 19, 2016 21:53
@t-yoshii
Copy link

Hi,
Does this feature cover MGLPolyLine?

Removing MGLPolyline object from MGLMapView causes app crash.

override func viewDidLoad() {
    super.viewDidLoad()

    mapView = MGLMapView(frame: view.bounds)
    mapView.autoresizingMask = [.FlexibleWidth, .FlexibleHeight]

    var coordinates: [CLLocationCoordinate2D] = [CLLocationCoordinate2DMake(40.7326808, -73.9843407), CLLocationCoordinate2DMake(40.7326808, -73.9943407)]
    polyline = MGLPolyline(coordinates: &coordinates, count: 2)

    // set the map's center coordinate
    mapView.setCenterCoordinate(CLLocationCoordinate2D(latitude: 40.7326808,
        longitude: -73.9843407),
        zoomLevel: 12, animated: false)

    mapView.addAnnotation(polyline!)

    mapView.removeAnnotation(polyline!)  /// CRASH HAPPENS HERE!
    view.addSubview(mapView)
}

Exception detail:

2016-04-20 18:11:23.932 MapboxTest[364:56042] *** Terminating app due to uncaught exception 'NSRangeException', reason: 'Cannot remove an observer <MGLMapView 0x15ee754d0> for the key path "coordinate" from <MGLPolyline 0x15ee602a0> because it is not registered as an observer.'

@1ec5
Copy link
Contributor Author

1ec5 commented Apr 20, 2016

@t-yoshii, good catch. It still isn’t possible to change a polyline or polygon’s location or form on the fly. A fix for this crash is in #4766.

@t-yoshii
Copy link

@1ec5 Thank you for your reply! I will try the version which includes #4766.

@incanus
Copy link
Contributor

incanus commented Jul 26, 2016

@1ec5 I realized I undid your deselection behavior over in #5786 (see opening description). How important is that to bring back?

@1ec5
Copy link
Contributor Author

1ec5 commented Jul 26, 2016

Here’s what this PR implemented originally:

In iosapp, tapping a callout view moves the selected annotation to the center of the screen and deselects it. Deselecting an annotation resets its image to the default; deselecting it again restores the image.

We should have something to exercise both pieces of functionality, but feel free to find a better way to exercise them.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants