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

Fixes a bug where the animated parameter of -[MGLMapView selectAnnotation:animated:] was ignored. #13689

Merged
merged 6 commits into from
Jan 9, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions platform/ios/CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT

* Added an `MGLStyle.enablePlacementTransitions` property to control how long it takes for colliding labels to fade out. ([#13565](https://github.com/mapbox/mapbox-gl-native/pull/13565))
* Fixed a crash when casting large numbers in `NSExpression`. ([#13580](https://github.com/mapbox/mapbox-gl-native/pull/13580))
* Fixed a bug where the `animated` parameter to `-[MGLMapView selectAnnotation:animated:]` was being ignored. ([#13689](https://github.com/mapbox/mapbox-gl-native/pull/13689))

## 4.7.0 - December 18, 2018

Expand Down
22 changes: 17 additions & 5 deletions platform/ios/src/MGLMapView.h
Expand Up @@ -1415,14 +1415,16 @@ MGL_EXPORT IB_DESIGNABLE
/**
Selects an annotation and displays its callout view.

The `animated` parameter determines whether the map is panned to bring the
annotation on-screen, specifically:

The `animated` parameter determines whether the selection is animated including whether the map is
panned to bring the annotation into view, specifically:
| `animated` parameter | Effect |
|------------------|--------|
| `NO` | The annotation is selected, and the callout is presented. However the map is not panned to bring the annotation or callout onscreen. The presentation of the callout is animated. |
| `YES` | The annotation is selected, and the callout is presented. If the annotation is offscreen *and* is of type `MGLPointAnnotation`, the map is panned so that the annotation and its callout are brought just onscreen. The annotation is *not* centered within the viewport. |
| `NO` | The annotation is selected, and the callout is presented. However the map is not panned to bring the annotation or callout into view. The presentation of the callout is NOT animated. |
| `YES` | The annotation is selected, and the callout is presented. If the annotation is not visible (or is partially visible) *and* is of type `MGLPointAnnotation`, the map is panned so that the annotation and its callout are brought into view. The annotation is *not* centered within the viewport. |

Note that a selection initiated by a single tap gesture is always animated.

@param annotation The annotation object to select.
@param animated If `YES`, the annotation and callout view are animated on-screen.

Expand All @@ -1431,6 +1433,16 @@ MGL_EXPORT IB_DESIGNABLE
*/
- (void)selectAnnotation:(id <MGLAnnotation>)annotation animated:(BOOL)animated;

/**
:nodoc:
friedbunny marked this conversation as resolved.
Show resolved Hide resolved
Selects an annotation and displays its callout view.

@param annotation The annotation object to select.
@param moveIntoView If the annotation is not visible (or is partially visible) *and* is of type `MGLPointAnnotation`, the map is panned so that the annotation and its callout are brought into view. The annotation is *not* centered within the viewport. |
@param animateSelection If `YES`, the annotation's selection state and callout view's presentation are animated.
*/
- (void)selectAnnotation:(id <MGLAnnotation>)annotation moveIntoView:(BOOL)moveIntoView animateSelection:(BOOL)animateSelection;

/**
Deselects an annotation and hides its callout view.

Expand Down
35 changes: 20 additions & 15 deletions platform/ios/src/MGLMapView.mm
Expand Up @@ -1768,7 +1768,7 @@ - (void)handleSingleTapGesture:(UITapGestureRecognizer *)singleTap
{
CGPoint calloutPoint = [singleTap locationInView:self];
CGRect positionRect = [self positioningRectForAnnotation:annotation defaultCalloutPoint:calloutPoint];
[self selectAnnotation:annotation moveOnscreen:YES animateSelection:YES calloutPositioningRect:positionRect];
[self selectAnnotation:annotation moveIntoView:YES animateSelection:YES calloutPositioningRect:positionRect];
}
else if (self.selectedAnnotation)
{
Expand Down Expand Up @@ -4525,7 +4525,7 @@ - (MGLAnnotationTag)annotationTagAtPoint:(CGPoint)point persistingResults:(BOOL)
}


- (BOOL)isBringingAnnotationOnscreenSupportedForAnnotation:(id<MGLAnnotation>)annotation animated:(BOOL)animated {
- (BOOL)isMovingAnnotationIntoViewSupportedForAnnotation:(id<MGLAnnotation>)annotation animated:(BOOL)animated {
// Consider delegating
return animated && [annotation isKindOfClass:[MGLPointAnnotation class]];
}
Expand Down Expand Up @@ -4577,12 +4577,17 @@ - (void)setSelectedAnnotations:(NSArray<id <MGLAnnotation>> *)selectedAnnotation

- (void)selectAnnotation:(id <MGLAnnotation>)annotation animated:(BOOL)animated
{
MGLLogDebug(@"Selecting annotation: %@ animated: %@", annotation, MGLStringFromBOOL(animated));
[self selectAnnotation:annotation moveIntoView:animated animateSelection:animated];
}

- (void)selectAnnotation:(id <MGLAnnotation>)annotation moveIntoView:(BOOL)moveIntoView animateSelection:(BOOL)animateSelection
{
MGLLogDebug(@"Selecting annotation: %@ moveIntoView: %@ animateSelection: %@", annotation, MGLStringFromBOOL(moveIntoView), MGLStringFromBOOL(animateSelection));
CGRect positioningRect = [self positioningRectForAnnotation:annotation defaultCalloutPoint:CGPointZero];
[self selectAnnotation:annotation moveOnscreen:animated animateSelection:YES calloutPositioningRect:positioningRect];
[self selectAnnotation:annotation moveIntoView:moveIntoView animateSelection:animateSelection calloutPositioningRect:positioningRect];
Copy link
Contributor

Choose a reason for hiding this comment

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

Should deselection in response to another annotation being selected also honor the animated flag, for consistency?

[self deselectAnnotation:self.selectedAnnotation animated:NO];

[annotationView setSelected:NO animated:animated];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the quick review!

I had the very same question 🙂 I can imagine situations where different applications would want different results. I think leaving as is makes the most sense at present, and we can tackle in a subsequent PR if necessary.

}

- (void)selectAnnotation:(id <MGLAnnotation>)annotation moveOnscreen:(BOOL)moveOnscreen animateSelection:(BOOL)animateSelection calloutPositioningRect:(CGRect)calloutPositioningRect
- (void)selectAnnotation:(id <MGLAnnotation>)annotation moveIntoView:(BOOL)moveIntoView animateSelection:(BOOL)animateSelection calloutPositioningRect:(CGRect)calloutPositioningRect
{
if ( ! annotation) return;

Expand Down Expand Up @@ -4617,8 +4622,8 @@ - (void)selectAnnotation:(id <MGLAnnotation>)annotation moveOnscreen:(BOOL)moveO
self.selectedAnnotation = annotation;

// Determine if we're allowed to move this offscreen annotation on screen, even though we've asked it to
if (moveOnscreen) {
moveOnscreen = [self isBringingAnnotationOnscreenSupportedForAnnotation:annotation animated:animateSelection];
if (moveIntoView) {
moveIntoView = [self isMovingAnnotationIntoViewSupportedForAnnotation:annotation animated:animateSelection];
}

// If we have an invalid positioning rect, we need to provide a suitable default.
Expand Down Expand Up @@ -4702,15 +4707,15 @@ - (void)selectAnnotation:(id <MGLAnnotation>)annotation moveOnscreen:(BOOL)moveO

// If the callout view provides inset (outset) information, we can use it to expand our positioning
// rect, which we then use to help move the annotation on-screen if want need to.
if (moveOnscreen && [calloutView respondsToSelector:@selector(marginInsetsHintForPresentationFromRect:)]) {
if (moveIntoView && [calloutView respondsToSelector:@selector(marginInsetsHintForPresentationFromRect:)]) {
UIEdgeInsets margins = [calloutView marginInsetsHintForPresentationFromRect:calloutPositioningRect];
expandedPositioningRect = UIEdgeInsetsInsetRect(expandedPositioningRect, margins);
}
}

if (moveOnscreen)
if (moveIntoView)
{
moveOnscreen = NO;
moveIntoView = NO;

// Need to consider the content insets.
CGRect bounds = UIEdgeInsetsInsetRect(self.bounds, self.contentInset);
Expand All @@ -4719,23 +4724,23 @@ - (void)selectAnnotation:(id <MGLAnnotation>)annotation moveOnscreen:(BOOL)moveO
if (CGRectGetMinX(calloutPositioningRect) < CGRectGetMinX(bounds))
{
constrainedRect.origin.x = expandedPositioningRect.origin.x;
moveOnscreen = YES;
moveIntoView = YES;
}
else if (CGRectGetMaxX(calloutPositioningRect) > CGRectGetMaxX(bounds))
{
constrainedRect.origin.x = CGRectGetMaxX(expandedPositioningRect) - constrainedRect.size.width;
moveOnscreen = YES;
moveIntoView = YES;
}

if (CGRectGetMinY(calloutPositioningRect) < CGRectGetMinY(bounds))
{
constrainedRect.origin.y = expandedPositioningRect.origin.y;
moveOnscreen = YES;
moveIntoView = YES;
}
else if (CGRectGetMaxY(calloutPositioningRect) > CGRectGetMaxY(bounds))
{
constrainedRect.origin.y = CGRectGetMaxY(expandedPositioningRect) - constrainedRect.size.height;
moveOnscreen = YES;
moveIntoView = YES;
}
}

Expand All @@ -4756,7 +4761,7 @@ - (void)selectAnnotation:(id <MGLAnnotation>)annotation moveOnscreen:(BOOL)moveO
[self.delegate mapView:self didSelectAnnotationView:annotationView];
}

if (moveOnscreen)
if (moveIntoView)
{
CGPoint center = CGPointMake(CGRectGetMidX(constrainedRect), CGRectGetMidY(constrainedRect));
CLLocationCoordinate2D centerCoord = [self convertPoint:center toCoordinateFromView:self];
Expand Down
28 changes: 14 additions & 14 deletions platform/macos/src/MGLMapView.mm
Expand Up @@ -2279,7 +2279,7 @@ - (void)setSelectedAnnotations:(NSArray<id <MGLAnnotation>> *)selectedAnnotation
[self selectAnnotation:firstAnnotation];
}

- (BOOL)isBringingAnnotationOnscreenSupportedForAnnotation:(id<MGLAnnotation>)annotation animated:(BOOL)animated {
- (BOOL)ismovingAnnotationIntoViewSupportedForAnnotation:(id<MGLAnnotation>)annotation animated:(BOOL)animated {
friedbunny marked this conversation as resolved.
Show resolved Hide resolved
// Consider delegating
return animated && [annotation isKindOfClass:[MGLPointAnnotation class]];
}
Expand All @@ -2293,12 +2293,12 @@ - (void)selectAnnotation:(id <MGLAnnotation>)annotation
- (void)selectAnnotation:(id <MGLAnnotation>)annotation atPoint:(NSPoint)gesturePoint
{
MGLLogDebug(@"Selecting annotation: %@ atPoint: %@", annotation, NSStringFromPoint(gesturePoint));
[self selectAnnotation:annotation atPoint:gesturePoint moveOnscreen:YES animateSelection:YES];
[self selectAnnotation:annotation atPoint:gesturePoint moveIntoView:YES animateSelection:YES];
}

- (void)selectAnnotation:(id <MGLAnnotation>)annotation atPoint:(NSPoint)gesturePoint moveOnscreen:(BOOL)moveOnscreen animateSelection:(BOOL)animateSelection
- (void)selectAnnotation:(id <MGLAnnotation>)annotation atPoint:(NSPoint)gesturePoint moveIntoView:(BOOL)moveIntoView animateSelection:(BOOL)animateSelection
{
MGLLogDebug(@"Selecting annotation: %@ atPoint: %@ moveOnscreen: %@ animateSelection: %@", annotation, NSStringFromPoint(gesturePoint), MGLStringFromBOOL(moveOnscreen), MGLStringFromBOOL(animateSelection));
MGLLogDebug(@"Selecting annotation: %@ atPoint: %@ moveIntoView: %@ animateSelection: %@", annotation, NSStringFromPoint(gesturePoint), MGLStringFromBOOL(moveIntoView), MGLStringFromBOOL(animateSelection));
id <MGLAnnotation> selectedAnnotation = self.selectedAnnotation;
if (annotation == selectedAnnotation) {
return;
Expand All @@ -2313,8 +2313,8 @@ - (void)selectAnnotation:(id <MGLAnnotation>)annotation atPoint:(NSPoint)gesture
[self addAnnotation:annotation];
}

if (moveOnscreen) {
moveOnscreen = [self isBringingAnnotationOnscreenSupportedForAnnotation:annotation animated:animateSelection];
if (moveIntoView) {
moveIntoView = [self ismovingAnnotationIntoViewSupportedForAnnotation:annotation animated:animateSelection];
}

// The annotation's anchor will bounce to the current click.
Expand All @@ -2326,7 +2326,7 @@ - (void)selectAnnotation:(id <MGLAnnotation>)annotation atPoint:(NSPoint)gesture
positioningRect.origin = [self convertCoordinate:origin toPointToView:self];
}

if (!moveOnscreen && NSIsEmptyRect(NSIntersectionRect(positioningRect, self.bounds))) {
if (!moveIntoView && NSIsEmptyRect(NSIntersectionRect(positioningRect, self.bounds))) {
if (!NSEqualPoints(gesturePoint, NSZeroPoint)) {
positioningRect = CGRectMake(gesturePoint.x, gesturePoint.y, positioningRect.size.width, positioningRect.size.height);
}
Expand Down Expand Up @@ -2359,9 +2359,9 @@ - (void)selectAnnotation:(id <MGLAnnotation>)annotation atPoint:(NSPoint)gesture
[callout showRelativeToRect:positioningRect ofView:self preferredEdge:edge];
}

if (moveOnscreen)
if (moveIntoView)
{
moveOnscreen = NO;
moveIntoView = NO;

NSRect (^edgeInsetsInsetRect)(NSRect, NSEdgeInsets) = ^(NSRect rect, NSEdgeInsets insets) {
return NSMakeRect(rect.origin.x + insets.left,
Expand All @@ -2381,26 +2381,26 @@ - (void)selectAnnotation:(id <MGLAnnotation>)annotation atPoint:(NSPoint)gesture
if (CGRectGetMinX(positioningRect) < CGRectGetMinX(bounds))
{
constrainedRect.origin.x = expandedPositioningRect.origin.x;
moveOnscreen = YES;
moveIntoView = YES;
}
else if (CGRectGetMaxX(positioningRect) > CGRectGetMaxX(bounds))
{
constrainedRect.origin.x = CGRectGetMaxX(expandedPositioningRect) - constrainedRect.size.width;
moveOnscreen = YES;
moveIntoView = YES;
}

if (CGRectGetMinY(positioningRect) < CGRectGetMinY(bounds))
{
constrainedRect.origin.y = expandedPositioningRect.origin.y;
moveOnscreen = YES;
moveIntoView = YES;
}
else if (CGRectGetMaxY(positioningRect) > CGRectGetMaxY(bounds))
{
constrainedRect.origin.y = CGRectGetMaxY(expandedPositioningRect) - constrainedRect.size.height;
moveOnscreen = YES;
moveIntoView = YES;
}

if (moveOnscreen)
if (moveIntoView)
{
CGPoint center = CGPointMake(CGRectGetMidX(constrainedRect), CGRectGetMidY(constrainedRect));
CLLocationCoordinate2D centerCoord = [self convertPoint:center toCoordinateFromView:self];
Expand Down