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

#2599 Fix InfoWindow position after style switching #2627

Merged
merged 1 commit into from
Oct 16, 2015

Conversation

ljbade
Copy link
Contributor

@ljbade ljbade commented Oct 15, 2015

Fixes InfoWindow position after style switching (for #2599)

Also incorporates the #2588 commit that @tobrun reverted with the crash fixed.

As part of this #630 was implemented.

@ljbade ljbade added the Android Mapbox Maps SDK for Android label Oct 15, 2015
@ljbade ljbade added this to the android-v2.1.0 milestone Oct 15, 2015
@ljbade
Copy link
Contributor Author

ljbade commented Oct 15, 2015

@jfirebaugh Can you review the changes made to core for implementing #630

@tobrun Can you verify the behaviour of InfoWindow and InfoWindowAdapter. Try changing the style to Emerald and back to see how it now readjusts the InfoWindow position.

@ljbade ljbade force-pushed the 2599-fix-infowindow-style-switch branch 2 times, most recently from fd84fa8 to 991eeca Compare October 15, 2015 06:32
@ljbade
Copy link
Contributor Author

ljbade commented Oct 15, 2015

@tobrun I think there is a bug with InfoWindow and InfoWindowAdapter activities. It does not seem to load the offset.

Going to need to workout why the map finish loading event is not updating the markers on these activities.

@tobrun
Copy link
Member

tobrun commented Oct 15, 2015

@ljbade
To give some more info: When testing my PR yesterday noticed that the MainActivity did not give any problems for adding markers/infowindow on a long press. I'm thinking it could be related to adding markers/infowindow while creating the MapView?

Update: will do a test this evening related to repositioning

@ljbade
Copy link
Contributor Author

ljbade commented Oct 15, 2015

@tobrun Yeah it has something to do with adding Marker during onCreate.

@ljbade
Copy link
Contributor Author

ljbade commented Oct 15, 2015

@jfirebaugh How do I move that notification back to the main thread, since setStyleUrl is async? Is there a function I can use to post an event to the main thread?

@ljbade
Copy link
Contributor Author

ljbade commented Oct 15, 2015

@jfirebaugh or should I check for failure in renderSync?

@jfirebaugh
Copy link
Contributor

I don't think there's a good way to emit MapChangeDidFailLoadingMap right now.

@ljbade
Copy link
Contributor Author

ljbade commented Oct 15, 2015

@jfirebaugh An idea I had was to add loadFailed flag to MapData then set this on fail. renderSync will then check and clear this flag before it checks the loading flag.

@ljbade
Copy link
Contributor Author

ljbade commented Oct 16, 2015

I had to move the set for data.loading to loadStyleJSON() to avoid a false map loaded event right after the map is created (and finishes rendering a blank map) but before it starts loading the new style.

This fixed the problem with adding markers in onCreate.

@ljbade ljbade force-pushed the 2599-fix-infowindow-style-switch branch from 991eeca to 190aa99 Compare October 16, 2015 00:49
@ljbade
Copy link
Contributor Author

ljbade commented Oct 16, 2015

Only Travis failure is a known iOS hang. Other jobs passed.

@@ -92,10 +92,6 @@ void MapContext::triggerUpdate(const TransformState& state, const Update flags)
}

void MapContext::setStyleURL(const std::string& url) {
if (styleURL == url) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You're reverting 074d61a from earlier today. Accidental?

…hing styles

Fixes #2599
Make getTopOffsetPixelsForAnnotationSymbol private
Also implements MapChangeWillStartLoadingMap, MapChangeDidFinishLoadingMap
Paritally implements #630
@ljbade ljbade force-pushed the 2599-fix-infowindow-style-switch branch from 190aa99 to 2ee7b02 Compare October 16, 2015 02:07
@ljbade ljbade merged commit 2ee7b02 into master Oct 16, 2015
@ljbade
Copy link
Contributor Author

ljbade commented Oct 16, 2015

@jfirebaugh I missed your comment before merging. I used git cherry-pick 074d61a55e32e83173ec12b2eb34b78ddaaf0ef1 to replay @brunoabinader's commit on master to restore it.

@ljbade ljbade deleted the 2599-fix-infowindow-style-switch branch October 16, 2015 02:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants