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

[WIP] Annotation callout views #1074

Closed
wants to merge 31 commits into from
Closed

[WIP] Annotation callout views #1074

wants to merge 31 commits into from

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Mar 23, 2015

Yet another branch related to annotations. This PR adds very basic callout views. Tapping an annotation now displays a callout view. Changing the viewport in any way dismisses the callout view. Only one callout view is displayed at a time.

Remaining work:

  • Get sprite sizes to anchor callout views at the top of sprites rather than at their center points

Fixes #894.

Tapping an annotation now displays a callout view. Changing the viewport in any way dismisses the callout view. Only one callout view is displayed at a time.

Ref #894
@1ec5 1ec5 added the iOS Mapbox Maps SDK for iOS label Mar 23, 2015
@1ec5 1ec5 added this to the iOS Beta 1 milestone Mar 23, 2015
@1ec5 1ec5 mentioned this pull request Mar 24, 2015
@incanus incanus self-assigned this Mar 24, 2015
We will eventually have callouts on Android and other platforms,
perhaps also third-party libraries. We should name here more
specifically, as well as pop things into a vendor folder for
tidiness.
We will hopefully eventually move to a more MapKit-like model
of passing the annotation _view_ here and not the annotation
in future.
@incanus
Copy link
Contributor

incanus commented Mar 24, 2015

Backlog right now:

  • Callout accessory control delegate method
  • Callout delegate for pan repositioning (could punt for now if complex)
  • Tighten up selection fuzziness to avoid some misfires

@incanus
Copy link
Contributor

incanus commented Mar 24, 2015

Callout accessory control delegate method

On this front, I am going to go the route of our iOS SDK and attach the accessory view properties to the annotation for now. When we get a proper annotation view, we can move it out to there.

@incanus
Copy link
Contributor

incanus commented Mar 24, 2015

Actually, scratch that, I will put these in temp methods on the map view delegate for now since we'll do some cleanup there around marker symbology, which is a holdover.

@incanus
Copy link
Contributor

incanus commented Mar 25, 2015

Callout delegate for pan repositioning (could punt for now if complex)

Yeah, this is complex, so gonna punt for now. the problem is if you tap a marker whose presented callout would be partially offscreen. SMCalloutView has an awesome delegate method to handle this, letting you move your content and return a delay by which to postpone the callout appearance animation while you get ready, used like so:

- (NSTimeInterval)calloutView:(SMCalloutView *)calloutView delayForRepositionWithSize:(CGSize)offset
{
    (void)calloutView;

    NSTimeInterval adjustmentDuration = 0.3;

    self.repositioningForCallout = YES;

    mbglMap->moveBy(offset.width, offset.height, secondsAsDuration(adjustmentDuration));

    return adjustmentDuration;
}

Here we use self.repositioningForCallout in order to temporarily disable dismissal of the nascent popup since the map is going to have to move, ordinarily canceling it.

The problem here though is that mbglMap->move(), while moving the appearance of the map, unlike the UIScrollView in our SDK, doesn't actually move the view that the callout is already anchored to. At this point in the callout view's lifecycle, the appearance animation is already queued up; it will just be given a delay when we return. So the callout still shows up offscreen because nothing in its coordinate system has actually been adjusted.

One way to solve this would be to hack up SMCalloutView a bit (hey, I've been there before) to somehow allow the offset to be set before the animation time.

Again, not a huge deal right now, as we also likely have issues with navigation and/or toolbars and need to work with -[SMCalloutView constrainedInsets] anyway.

👞 🏈

@incanus
Copy link
Contributor

incanus commented Mar 25, 2015

Repositioning noodling looking this for when we hit this:

https://gist.github.com/incanus/41f14d1ad54ae55101b5

@incanus
Copy link
Contributor

incanus commented Mar 25, 2015

BTW one clear way to do this would be to create a UIView laid atop the GLKView with userInteractionEnabled = NO. Then, have its backing layer be a CAScrollLayer that would could scrollToPoint() for offset adjustments. But that might impact eventual CADisplayLink work for callout & view movement syncing, since @bleege turned up better performance with the GLKView as the superview directly.

@incanus
Copy link
Contributor

incanus commented Mar 25, 2015

Tighten up selection fuzziness to avoid some misfires

This feels much better with 0dbed13.

@incanus
Copy link
Contributor

incanus commented Mar 25, 2015

Giving this some more tire-kicking, then rolling a squash PR.

@1ec5
Copy link
Contributor Author

1ec5 commented Mar 25, 2015

Let me know before you squash, so I can update 756-user-dot first.

@incanus
Copy link
Contributor

incanus commented Mar 25, 2015

Squash over in #1088. I won't delete any branches until you say, @1ec5 :-)

@incanus
Copy link
Contributor

incanus commented Mar 25, 2015

I was also planning on running with #1082 next anyway.

@incanus incanus closed this in 10a30da Mar 25, 2015
incanus added a commit that referenced this pull request Mar 25, 2015
@incanus incanus deleted the 894-callout branch March 26, 2015 21:33
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 7efb15e on 894-callout into * on master*.

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

Successfully merging this pull request may close these issues.

popups
3 participants