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

[not ready] Cocoa annotations API #1037

Closed
wants to merge 25 commits into from
Closed

Conversation

incanus
Copy link
Contributor

@incanus incanus commented Mar 20, 2015

Goal here is to take the best parts of these intermingled branches:

Then to get rid of them and shore up the definitive Cocoa model API here.

/cc @1ec5

@incanus incanus added feature iOS Mapbox Maps SDK for iOS labels Mar 20, 2015
@incanus incanus self-assigned this Mar 20, 2015
@incanus incanus added this to the iOS Beta 1 milestone Mar 20, 2015
@incanus
Copy link
Contributor Author

incanus commented Mar 20, 2015

The display link work is pretty straightforward. Correct me if wrong @bleege but there are two parts to the approach:

  • Be sure to add the views you're keeping in sync as subviews of the GLKView.
  • As is usually the case, be sure to add the display link to the main run loop for NSRunLoopCommonModes.

I've captured this branch's diff here since it's pretty light:

https://gist.github.com/incanus/286b534c636c601a73f6

Now deleting 877-display-link.

@incanus
Copy link
Contributor Author

incanus commented Mar 20, 2015

877-point-callout's whole, um, point, was to merge in an early sneak peek of the C++ annotations API into 877-callout. Now that that is in master, these branches are identical, so removing 877-point-callout.

@incanus
Copy link
Contributor Author

incanus commented Mar 20, 2015

Ok, I have the important bits of 877-callout ported over, with some discussion with @1ec5 on retains of annotations models from the client, fast lookup upon deletion, and then replacing the idea of annotation views as both a) a link between callouts and annotation models and b) a way to avoid copying annotation models as collection storage keys. We're using an NSMapTable with NSMapTableStrongMemory for both keys and values so that we retain the annotation models (keys) like MapKit does, as well as whatever we create (NSNumber value wrappers for C++ annotation IDs, for now) but don't copy them (by not using NSMapTableCopyIn).

Capturing diff of 877-callout here for some annotation selection/deselection logic that I might use, but that I've also prototyped out a bit anyway in annotation-selection.

https://gist.github.com/incanus/efe93044e9a9aa44e487

Deleting 877-callout, then refining what we have here.

@incanus
Copy link
Contributor Author

incanus commented Mar 20, 2015

Backlog:

  • basic addition & removal
  • consult delegate for symbology
  • selection/deselection gestures (no popups; that's in popups #894)
  • header docs

@incanus
Copy link
Contributor Author

incanus commented Mar 21, 2015

Comparing selection work from annotation-selection here, then deleting that branch:

https://gist.github.com/incanus/1421fcb93700596cba6d

@incanus
Copy link
Contributor Author

incanus commented Mar 21, 2015

Based on some rough tests with MapKit, it would appear that despite taking and returning an array of selected annotations, MKMapView only ever really selects and/or pops up a callout on one at a time. Docs point somewhat to this, re: -[MKMapView selectedAnnotations]:

Assigning a new array to this property selects only the first annotation in the array.

Then, only one annotation is ever returned in this array, so I've mirrored that behavior.

Next up is pulling over the gesture work to 1) properly select annotation based on GL-side marker taps and 2) properly iterate through nearby markers when the group is tapped upon.

@incanus
Copy link
Contributor Author

incanus commented Mar 22, 2015

Working through the intricacies of gestures & cycling selection state now, which is tricky since the markers aren't actual touch-enabled views at all, just pinpoint coordinates, plus we somehow have to communicate the sprite image size back to the gesture interpretation, as well as whether it has transparent areas.

anigif-1427000240

@incanus
Copy link
Contributor Author

incanus commented Mar 22, 2015

Ok, I think I've gotten annotation selection/deselection gestures tuned well enough for a typical marker size and offset, including with map rotation (which doesn't change marker orientation, but does change the translation of screen-to-geography bounding boxes). The proof of this of course will be a natural-feeling popup in response to map taps, so I'm jumping over to #894 next.

Per #1037 (comment), that just leaves in this ticket the header documentation for the new public API methods on MGLMapView so it appears in the SDK docs.

For posterity, in case anyone ever needs the flashing red rectangle shown above for debug purposes, it's easily done near the top of -[MGLMapView handleSingleTapGesture:] with:

UIView *debugView = [[UIView alloc] initWithFrame:tapRect];
debugView.backgroundColor = [[UIColor redColor] colorWithAlphaComponent:0.25];
debugView.userInteractionEnabled = NO;
[self addSubview:debugView];
[UIView animateWithDuration:1
                 animations:^
                 {
                     debugView.alpha = 0;
                 }
                 completion:^(BOOL finished)
                 {
                     (void)finished;
                     [debugView removeFromSuperview];
                 }];

@incanus
Copy link
Contributor Author

incanus commented Mar 22, 2015

The proof of this of course will be a natural-feeling popup in response to map taps, so I'm jumping over to #894 next.

Eh, scratch this. We should finish the docs here first and get this merged to master before starting on popups since they rely on selection state.

@incanus
Copy link
Contributor Author

incanus commented Mar 22, 2015

Header docs are done in 0a5e02d; making a new squashed PR now.

@incanus incanus mentioned this pull request Mar 22, 2015
@incanus
Copy link
Contributor Author

incanus commented Mar 22, 2015

#1055

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 0af36ae on cocoa-annotations-api into * on master*.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants