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

MGLMapView moves custom MGLCalloutView, ignoring custom frame #7483

Closed
andrewstay opened this issue Dec 19, 2016 · 8 comments
Closed

MGLMapView moves custom MGLCalloutView, ignoring custom frame #7483

andrewstay opened this issue Dec 19, 2016 · 8 comments
Assignees
Labels
annotations Annotations on iOS and macOS or markers on Android bug iOS Mapbox Maps SDK for iOS release blocker Blocks the next final release
Milestone

Comments

@andrewstay
Copy link

Platform: iOS
Mapbox SDK version: ios-v3.4.0-beta.5

Steps to trigger behavior

  1. Implement your custom Callout view which conforms to MGLCalloutView protocol.
  2. Implement -[MGLCalloutView presentWithContentSize:representedFromRect:inView:constrainedToView:animated:] and make your custom layout logic in there.

Expected behavior

Custom frame set to the callout should remain persistent within the superview the callout was added to.

Actual behavior

SDK moves the custom callout based on its own undefined rules.

Regression

Issue is not reproducible in ios-v3.4.0-beta.4

Please review the test application attached.
Read "readme.txt" for details on how to run the test application.
TestMapbox.zip

@friedbunny friedbunny added the iOS Mapbox Maps SDK for iOS label Dec 19, 2016
@frederoni
Copy link
Contributor

frederoni commented Dec 21, 2016

Thank you for your report.
What changed between beta 4 and 5 is that the callout view stays open when you pan the map.
If you want the same behavior as in beta 4, you could call -[MGLMapView deselectAnnotation:animated:] from -[MGLMapViewDelegate mapView:regionDidChangeAnimated:] if you have a visible callout view.

If you want the behavior of beta 5 but with representedFromRect: taken into account, then we need to figure out another solution.

@andrewstay
Copy link
Author

@frederoni the solution you have suggested does not work, as callout gets misplaced by the SDK before it gets dismissed by the client request. I implemented the following code:

- (void)mapViewRegionIsChanging:(MGLMapView *)aView
{
    [self.mapView deselectAnnotation:self.mapView.selectedAnnotations.lastObject animated:YES];
}

Please review the video of the result.
https://www.dropbox.com/s/jwc28dofqoxqjn4/CallotMisplaced.mov?dl=0

@1ec5
Copy link
Contributor

1ec5 commented Dec 22, 2016

(Noting that this regression was introduced in #6676.)

@1ec5 1ec5 added the annotations Annotations on iOS and macOS or markers on Android label Dec 22, 2016
@1ec5 1ec5 added this to the ios-v3.4.0 milestone Dec 22, 2016
@1ec5
Copy link
Contributor

1ec5 commented Dec 22, 2016

We’re repositioning the callout view by setting its center property. One workaround might be to override -center and -setCenter: to be fixed at the desired location. It’s a bit unfortunate if a developer tries to use constraints to affix the callout view to a particular location, though.

We should add a new optional method to MGLCalloutView, -annotationDidMoveToCoordinate:, that MGLCompactCalloutView implements by checking center and setting center but that other implementations, such as @andrewstay’s, could omit.

(Incidentally, we should formalize the requirement that MGLCalloutView is a UIView: #7523.)

@andrewstay
Copy link
Author

andrewstay commented Dec 22, 2016

It’s a bit unfortunate if a developer tries to use constraints to affix the callout view to a particular location, though.

The following method says the opposite:

/**
 Presents a callout view by adding it to `view` and pointing at the given rect
 of `view`’s bounds. Constrains the callout to the bounds of the given view.
 */
- (void)presentCalloutFromRect:(CGRect)rect inView:(UIView *)view constrainedToView:(UIView *)constrainedView animated:(BOOL)animated;

So, SDK requires positioning code to be implemented on the client side.
My suggestion is to make that method optional and if it is not implemented, let SDK center the callout.

@1ec5
Copy link
Contributor

1ec5 commented Dec 23, 2016

The protocol is a bit awkward, since #3322 took the shortest possible path to extracting SMCalloutView method calls into a protocol. We’ve been planning to completely revamp this API in #4392, but until then, I’m open to just about anything that works for both the built-in MGLCompactCalloutView and use cases like yours, since anything more sophisticated probably has to implement a custom callout view as something other than MGLCalloutView anyways.

I’m not entirely clear to me how the default behavior of “centering the callout” would lead to behavior similar to what’s in StayMapCallout, though.

@andrewstay
Copy link
Author

andrewstay commented Dec 23, 2016

@1ec5 Currently the very issue is that SDK moves the callout before the client has a chance to dismiss it itself. So, my suggestion is to make presentCalloutFromRect:inView:constrainedToView:animated: optional. Then, when client instantiates instance of the custom callout view you check in the SDK if the instance responds to this selector. if not - SDK uses its logic to center the callout. If the method is implemented, SDK calls it and does not position this calllout view itself.

@boundsj boundsj added the bug label Jan 3, 2017
@1ec5 1ec5 added the release blocker Blocks the next final release label Jan 11, 2017
@1ec5 1ec5 changed the title [iOS]SDK ignores the custom frame set to <MGLCalloutView> MGLMapView moves custom MGLCalloutView, ignoring custom frame Jan 11, 2017
@1ec5
Copy link
Contributor

1ec5 commented Jan 11, 2017

Fixed in #7646 on the release-ios-v3.4.0 branch. We’ll issue a prerelease with this fix shortly for you to try out.

@1ec5 1ec5 closed this as completed Jan 11, 2017
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 bug iOS Mapbox Maps SDK for iOS release blocker Blocks the next final release
Projects
None yet
Development

No branches or pull requests

5 participants