Skip to content

Conversation

@cammace
Copy link

@cammace cammace commented Apr 2, 2018

Closes #363

When the location layer plugin's disabled, we remove the onMapChange listener.

@cammace cammace added ready for review When your PR has been personally reviewed, its time for an external contributors to approve location-layer-plugin Issues that deal with the location layer module labels Apr 2, 2018
@cammace cammace added this to the location-layer-0.5.0 milestone Apr 2, 2018
@cammace cammace self-assigned this Apr 2, 2018
@cammace cammace requested review from LukasPaczos and tobrun April 2, 2018 20:04
if (mapboxMap != null) {
mapboxMap.removeOnCameraMoveListener(onCameraMoveListener);
}
mapView.removeOnMapChangedListener(onMapChangedListener);
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this one unregister the listener during style switch resulting in disabling the location layer? See

public void onMapChanged(int change) {
if (change == MapView.WILL_START_LOADING_MAP) {
onStop();
} else if (change == MapView.DID_FINISH_LOADING_STYLE) {
locationLayer.initializeComponents(options);
locationLayerCamera.initializeOptions(options);
setRenderMode(locationLayer.getRenderMode());
onStart();
}
}
.

@cammace cammace modified the milestones: location-layer-0.6.0, location-layer-0.5.1, location-layer-0.5.2 May 4, 2018
@cammace
Copy link
Author

cammace commented May 16, 2018

I've added separate onLocationLayerStop and start methods to extract the location layer lifecycle events from the activities which the plugin's contained within. This means when onMapChange gets called we don't remove the listener.

} else if (change == MapView.DID_FINISH_LOADING_STYLE) {
locationLayer.initializeComponents(options);
locationLayerCamera.initializeOptions(options);
setRenderMode(locationLayer.getRenderMode());
Copy link
Contributor

@LukasPaczos LukasPaczos May 17, 2018

Choose a reason for hiding this comment

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

If the plugin is disabled and we change the style this line will draw the puck in the last location even though the plugin is disabled. Could you address it in this PR as well?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, want to make this clear that this is a bug that is in master though, not something I introduced in this PR...

Copy link
Author

Choose a reason for hiding this comment

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

After looking into this, I'm not finding a solution to this fix. Will open a ticket and revisit in a separate PR.

Copy link
Author

Choose a reason for hiding this comment

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

@cammace cammace merged commit 089c450 into master May 17, 2018
@cammace cammace deleted the cam-363 branch May 17, 2018 16:34
@cammace cammace mentioned this pull request May 23, 2018
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

location-layer-plugin Issues that deal with the location layer module ready for review When your PR has been personally reviewed, its time for an external contributors to approve

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants