Refactored MGLUserLocationAnnotationView (redux) #5882

friedbunny · 2016-08-06T00:34:09Z

This is a replacement PR for #5573 that targets master.

Additionally, I’ve rewritten #5816 so that the hit testing layer can be set by any subclass of MGLUserLocationAnnotationView.

/cc @frederoni @1ec5 @incanus @boundsj

friedbunny · 2016-08-07T00:49:22Z

platform/ios/src/MGLMapView.mm

+
+ CGPoint tapPointForUserLocation = self.userLocationAnnotationView.hitTestLayer.superlayer ? [singleTap locationInView:self.userLocationAnnotationView] : tapPoint;
+
+ CALayer *hitLayer = self.userLocationVisible ? [self.userLocationAnnotationView.hitTestLayer hitTest:tapPointForUserLocation] : nil;


This doesn’t need to check again for self.userLocationVisible — will remove.

friedbunny · 2016-08-07T01:36:53Z

Saw some weird behavior just now that I don’t have time to investigate, but the user dot wasn’t being updated/deselected when another annotation became selected. E.g., tapping another annotation while in course tracking mode would leave the puck on screen until the tracking mode was manually changed again.

Will investigate next week.

friedbunny · 2016-08-09T22:10:24Z

Rebased again after v3.3.4 was merged back to master, which included a backport of 7aec17d in #5894.

friedbunny · 2016-08-13T02:18:31Z

380d54a9d5106fb957455c4376e086634ff05f39 fixes the issue where the annotation was not updating immediately.

The didUpdateUserLocation: method has been changed to update, which is again called from -[MGLMapView updateUserLocationAnnotationViewAnimatedWithDuration:].

A separate method that’s called when the user’s location changes shouldn’t be necessary, as the existing userLocation property will already contain this information.

The user location annotation needs updates more frequently than once per location event — it needs to respond to user interaction, viewport changes, and user tracking mode changes.

Very open to naming suggestions for update.

friedbunny · 2016-08-13T02:44:18Z

Updated the docs a bit. Is the @warning too much?

1ec5 · 2016-08-13T03:06:47Z

platform/ios/src/MGLUserLocationAnnotationView.h

+ user interaction, a change in the user’s location, when the user tracking mode
+ changes, or when the viewport changes.
+
+ @warning During user interaction with the map, this method may be called many


No keyword is needed here, although @note is fine if you think it’s sufficiently important. @warning should be reserved for pitfalls that may lead to data loss or similarly catastrophic outcomes.

incanus · 2016-08-15T17:52:34Z

The didUpdateUserLocation: method has been changed to update, which is again called from -[MGLMapView updateUserLocationAnnotationViewAnimatedWithDuration:].

Can you say more about this @friedbunny? didUpdateUserLocation: is akin to how MapKit does things.

friedbunny · 2016-08-15T18:36:15Z

Can you say more about this @friedbunny? didUpdateUserLocation: is akin to how MapKit does things.

@frederoni added -[MGLUserLocationAnnotationView didUpdateUserLocation:], which became the sole way the user location annotation was updated with the removal of [annotationView setupLayers] from -[MGLMapView updateUserLocationAnnotationViewAnimatedWithDuration:] in 91ec7cb#diff-20502b6bc02986a4398d0da84b2d4b94L4683.

I renamed -[MGLUserLocationAnnotationView didUpdateUserLocation:] to -[MGLUserLocationAnnotationView update] and removed its MGLUserLocation value, because this is already always available via MGLUserLocationAnnotationView.userLocation.

I then re-added the call to -[MGLUserLocationAnnotationView update] (changed from a direct call to -setupLayers previously) in -[MGLMapView updateUserLocationAnnotationViewAnimatedWithDuration:], so that the user location annotation can update when the user interacts with the map, when the tracking mode changes, etc.

-[MGLMapViewDelegate mapView:didUpdateUserLocation:] is unchanged.

incanus · 2016-08-15T18:41:58Z

Aha! Thank you.

friedbunny · 2016-08-15T18:49:54Z

This is ready for review 👓 again — thanks y’all.

I should note: this is a fairly pure refactor. The main part that has changed versus v3.3.x is that MGLUserLocationAnnotationView is now available. The functionality of the existing dot/puck now lives in MGLFaux3DUserLocationAnnotationView and is unchanged.

…tionView

And implement this property in MGLFaux3DUserLocationAnnotationView. Generic reimplementation of #5816.

The user location annotation needs to change in response to many factors, not just user location updates. When the user tracking mode changes and when the viewport changes, the dot should respond immediately.

Also remove unnecessary header import.

Because, really, what choice do you have? <3 Xcode.

friedbunny · 2016-08-16T14:42:48Z

Rebased again — I’m going to aim for merging this afternoon.

friedbunny added iOS Mapbox Maps SDK for iOS refactor annotations Annotations on iOS and macOS or markers on Android labels Aug 6, 2016

friedbunny added this to the ios-v3.4.0 milestone Aug 6, 2016

friedbunny assigned frederoni Aug 6, 2016

This was referenced Aug 6, 2016

[ios] #5039 refactored MGLUserLocationAnnotationView #5573

Closed

Tapping annotation within user location accuracy halo selects user location annotation #5571

Closed

friedbunny reviewed Aug 7, 2016
View reviewed changes

This was referenced Aug 7, 2016

[ios] Hit test the user location annotation dot specifically #5893

Closed

[ios] Hit test the user location annotation dot specifically (redone for release branch) #5894

Merged

friedbunny force-pushed the 5039-MGLUserLocationAnnotationView-refactor branch from cd60f2a to 9ba5c91 Compare August 9, 2016 22:07

friedbunny self-assigned this Aug 12, 2016

friedbunny force-pushed the 5039-MGLUserLocationAnnotationView-refactor branch from 9ba5c91 to 71d9687 Compare August 12, 2016 20:54

1ec5 reviewed Aug 13, 2016
View reviewed changes

friedbunny force-pushed the 5039-MGLUserLocationAnnotationView-refactor branch from 1806121 to 824c09f Compare August 15, 2016 18:19

friedbunny added the ✓ ready for review label Aug 15, 2016

1ec5 added the beta blocker Blocks the next beta release label Aug 15, 2016

friedbunny mentioned this pull request Aug 15, 2016

Improve user location annotation performance #6019

Merged

frederoni added 2 commits August 16, 2016 10:34

[ios] #5039 MGLUserLocationAnnotationView now inherits from MGLAnnota…

0793394

…tionView

[ios] WiP refactored user location annotation view

68e2e4c

frederoni and others added 18 commits August 16, 2016 10:37

[ios] allow mbgl to drive animation of all user location view's bounds.

e56ce6c

[ios] updated user location view in the demo app

0ce1195

[ios] removed unused import

ef8cf2a

added MGLUserLocationAnnotationView to jazzy

179aa65

[ios] Add MGLUserLocationAnnotationView.hitTestLayer property

f40c72c

And implement this property in MGLFaux3DUserLocationAnnotationView. Generic reimplementation of #5816.

Clean up hitLayer logic

c863546

Use a more specific test for a custom hit test layer

5903b81

Remove unused code

ff9fd2f

Move haloLayer into the private implementation def

5a65c8b

Only update frame size if not already set

d7e0c5b

Change to a plain update method, call it more frequently

9f06753

The user location annotation needs to change in response to many factors, not just user location updates. When the user tracking mode changes and when the viewport changes, the dot should respond immediately.

Give greater depth to documentation for update method

ca895ec

Be less severe: @warning → @note

652aecb

Fix example's accuracy in dot frame sizing

4687516

Also remove unnecessary header import.

Remove one letter, because it was bothering me...

b6e2a49

Re-add empty implementation of update with subclassing note

c46d79e

Accept Xcode's project file rearrangement

412214d

Because, really, what choice do you have? <3 Xcode.

Add toggle in iosapp for custom user location annotation

634f680

friedbunny force-pushed the 5039-MGLUserLocationAnnotationView-refactor branch from 824c09f to 634f680 Compare August 16, 2016 14:42

Update changelog for customizable user location annotations

3dd2179

friedbunny merged commit f489ec2 into master Aug 16, 2016

friedbunny deleted the 5039-MGLUserLocationAnnotationView-refactor branch August 16, 2016 20:05

friedbunny removed beta blocker Blocks the next beta release ✓ ready for review labels Aug 16, 2016

This was referenced Aug 16, 2016

MGLUserLocationAnnotationView should inherit from MGLAnnotationView #5039

Closed

Custom user location annotation view #5037

Closed

1ec5 mentioned this pull request Aug 20, 2016

-[MGLMapViewDelegate mapView:viewForAnnotation:] causes assertion failure if upgrading from v3.3.x #6100

Closed

aksonov mentioned this pull request Sep 16, 2016

View-backed annotations nitaliano/react-native-mapbox-gl#404

Closed

1ec5 mentioned this pull request Aug 30, 2017

[ios, macos] Add shapes annotations select/deselect delegates. #9755

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactored MGLUserLocationAnnotationView (redux) #5882

Refactored MGLUserLocationAnnotationView (redux) #5882

friedbunny commented Aug 6, 2016

friedbunny Aug 7, 2016 •

edited

Loading

friedbunny commented Aug 7, 2016

friedbunny commented Aug 9, 2016

friedbunny commented Aug 13, 2016 •

edited

Loading

friedbunny commented Aug 13, 2016

1ec5 Aug 13, 2016

incanus commented Aug 15, 2016

friedbunny commented Aug 15, 2016

incanus commented Aug 15, 2016 via email

friedbunny commented Aug 15, 2016 •

edited

Loading

friedbunny commented Aug 16, 2016


		CGPoint tapPointForUserLocation = self.userLocationAnnotationView.hitTestLayer.superlayer ? [singleTap locationInView:self.userLocationAnnotationView] : tapPoint;

		CALayer *hitLayer = self.userLocationVisible ? [self.userLocationAnnotationView.hitTestLayer hitTest:tapPointForUserLocation] : nil;

Refactored MGLUserLocationAnnotationView (redux) #5882

Refactored MGLUserLocationAnnotationView (redux) #5882

Conversation

friedbunny commented Aug 6, 2016

friedbunny Aug 7, 2016 • edited Loading

Choose a reason for hiding this comment

friedbunny commented Aug 7, 2016

friedbunny commented Aug 9, 2016

friedbunny commented Aug 13, 2016 • edited Loading

friedbunny commented Aug 13, 2016

1ec5 Aug 13, 2016

Choose a reason for hiding this comment

incanus commented Aug 15, 2016

friedbunny commented Aug 15, 2016

incanus commented Aug 15, 2016 via email

friedbunny commented Aug 15, 2016 • edited Loading

friedbunny commented Aug 16, 2016

friedbunny Aug 7, 2016 •

edited

Loading

friedbunny commented Aug 13, 2016 •

edited

Loading

friedbunny commented Aug 15, 2016 •

edited

Loading