Puck Location fix #2387
Puck Location fix #2387
Conversation
…vents, including map view resizing
Udumft
requested review from
1ec5,
fabian-guerra,
nishant-karajgikar and
captainbarbosa
| if tracksUserCourse { | ||
| centerUserCourseView() |
There was a problem hiding this comment.
This is the smoothest implementation I found so far, to make user puck slide in place gracefully...
There was a problem hiding this comment.
#2211 removed the line that set the view’s center. Can you check that it doesn’t introduce any regressions related to that PR? Thanks!
There was a problem hiding this comment.
Issue description and reproduction steps for #2211 are a bit cryptic for me, but as I understood, the issue was about incorrect puck positioning during navigation, together with zooming/panning/rotating map view. I checked that user puck is located right on the route regardless of camera position and switching to/from course tracking mode. Is that what #2211 was about?
There was a problem hiding this comment.
Unfortunately, I’m not super clear on it myself; I’m not entirely convinced that #2211 was the right fix for #2145, and it definitely wasn’t a fix for mapbox/mapbox-gl-native#15232 or mapbox/mapbox-gl-native#15233.
The main things to watch out for are #2145 (comment) and #2145 (comment).
There was a problem hiding this comment.
I could not reproduce symptoms described in linked comments...
There was a problem hiding this comment.
If I understand correctly, the Nav SDK is not using the Maps SDK user location annotation, which means it will not account for contentInset changes. I think this pr is trying to update the user location annotation considering the "insets".
|
Team - if we are done - can we close this now with a merge? It is blocking ... |
|
This PR also possibly fixes #2364. |
There was a problem hiding this comment.
After looking at the code in the Maps SDK that updates the user location annotation position I think this is the right approach and is not causing a regression since this does not contrapone #2211, and is reusing code that was already there to position user location annotation.
| if tracksUserCourse { | ||
| centerUserCourseView() |
There was a problem hiding this comment.
If I understand correctly, the Nav SDK is not using the Maps SDK user location annotation, which means it will not account for contentInset changes. I think this pr is trying to update the user location annotation considering the "insets".
Resolves puck location issue, caused by the bottom banner update.
Fixed updating Puck location on various events, including
NavigationMapViewresizing.Edit: Removed internal link.