From 9404ec45da0ae9d7125391f72ed4a0f5cd060960 Mon Sep 17 00:00:00 2001
From: soumyashisPR <37874108+soumyashisPR@users.noreply.github.com>
Date: Fri, 2 Oct 2020 21:20:01 +0530
Subject: [PATCH] Issue 915: Stop/start Location manager based on listeners
registered. (#999)
* Fixed doc descriptions for Camera
* Fixed documentation for Camera
* Maps: location manager start/stop listening fix
* Remove commented lines
* Reaname boolean fields for UserLocation
* fix unit tests
* Fix unit test cases for location manager
* Fix userlocation test
* Add modified docs
* PR review comment addressed
* PR review comment addressed
* PR review commit - reverted renames
* Fix unit test
* typo fix for read to re-add
---
__tests__/components/UserLocation.test.js | 5 +++--
__tests__/modules/location/locationManager.test.js | 11 ++++++++++-
docs/UserLocation.md | 2 +-
docs/docs.json | 4 ++--
javascript/components/UserLocation.js | 12 ++++--------
javascript/modules/location/locationManager.js | 7 +++++++
6 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/__tests__/components/UserLocation.test.js b/__tests__/components/UserLocation.test.js
index d60d1b919..8525da5e4 100644
--- a/__tests__/components/UserLocation.test.js
+++ b/__tests__/components/UserLocation.test.js
@@ -187,7 +187,7 @@ describe('UserLocation', () => {
expect(ul.locationManagerRunning).toStrictEqual(true);
expect(locationManager.start).toHaveBeenCalledTimes(1);
expect(locationManager.getLastKnownLocation).toHaveBeenCalledTimes(1);
- expect(ul.setState).toHaveBeenCalledTimes(1);
+ expect(ul.setState).toHaveBeenCalledTimes(2);
expect(ul.setState).toHaveBeenCalledWith({
coordinates: lastKnownLocation,
heading,
@@ -207,7 +207,8 @@ describe('UserLocation', () => {
expect(ul.locationManagerRunning).toStrictEqual(false);
// only once from start
expect(locationManager.start).toHaveBeenCalledTimes(1);
- expect(locationManager.stop).toHaveBeenCalledTimes(1);
+ // stop should not be called
+ expect(locationManager.stop).not.toHaveBeenCalled();
});
});
diff --git a/__tests__/modules/location/locationManager.test.js b/__tests__/modules/location/locationManager.test.js
index 6e0a929e2..b4ba3e3ad 100644
--- a/__tests__/modules/location/locationManager.test.js
+++ b/__tests__/modules/location/locationManager.test.js
@@ -69,6 +69,7 @@ describe('LocationManager', () => {
describe('#addListener', () => {
const myListener = jest.fn();
+ MapboxGL.LocationCallbackName = {Update: 'MapboxUserLocationUpdate'};
afterEach(() => {
locationManager._listeners = [];
@@ -80,7 +81,7 @@ describe('LocationManager', () => {
expect(locationManager._listeners).toStrictEqual([myListener]);
});
- test('does not readd same listener', () => {
+ test('does not re-add same listener', () => {
locationManager.addListener(myListener);
expect(locationManager._listeners).toStrictEqual([myListener]);
locationManager.addListener(myListener);
@@ -99,6 +100,8 @@ describe('LocationManager', () => {
});
describe('#removeListener', () => {
+ MapboxGLLocationManager.stop = jest.fn();
+
test('removes selected listener', () => {
// just two different functions
const listenerA = jest.fn(() => 'listenerA');
@@ -106,16 +109,22 @@ describe('LocationManager', () => {
locationManager.addListener(listenerA);
expect(locationManager._listeners).toStrictEqual([listenerA]);
+ expect(MapboxGLLocationManager.stop).not.toHaveBeenCalled();
+
locationManager.addListener(listenerB);
expect(locationManager._listeners).toStrictEqual([
listenerA,
listenerB,
]);
+ expect(MapboxGLLocationManager.stop).not.toHaveBeenCalled();
locationManager.removeListener(listenerB);
expect(locationManager._listeners).toStrictEqual([listenerA]);
+ expect(MapboxGLLocationManager.stop).not.toHaveBeenCalled();
+
locationManager.removeListener(listenerA);
expect(locationManager._listeners).toStrictEqual([]);
+ expect(MapboxGLLocationManager.stop).toHaveBeenCalledTimes(1);
});
});
diff --git a/docs/UserLocation.md b/docs/UserLocation.md
index a16c1774b..481f4a2f0 100644
--- a/docs/UserLocation.md
+++ b/docs/UserLocation.md
@@ -18,7 +18,7 @@
### methods
#### setLocationManager({running})
-Whether to start or stop the locationManager
Notice, that locationManager will start automatically when
either `onUpdate` or `visible` are set
+Whether to start or stop listening to the locationManager
Notice, that listening will start automatically when
either `onUpdate` or `visible` are set
##### arguments
| Name | Type | Required | Description |
diff --git a/docs/docs.json b/docs/docs.json
index c16b8cf8f..ddf737411 100644
--- a/docs/docs.json
+++ b/docs/docs.json
@@ -5164,7 +5164,7 @@
"methods": [
{
"name": "setLocationManager",
- "docblock": "Whether to start or stop the locationManager\n\nNotice, that locationManager will start automatically when\neither `onUpdate` or `visible` are set\n\n@async\n@param {Object} running - Object with key `running` and `boolean` value\n@return {Promise}",
+ "docblock": "Whether to start or stop listening to the locationManager\n\nNotice, that listening will start automatically when\neither `onUpdate` or `visible` are set\n\n@async\n@param {Object} running - Object with key `running` and `boolean` value\n@return {Promise}",
"modifiers": [
"async"
],
@@ -5184,7 +5184,7 @@
]
}
},
- "description": "Whether to start or stop the locationManager\n\nNotice, that locationManager will start automatically when\neither `onUpdate` or `visible` are set",
+ "description": "Whether to start or stop listening to the locationManager\n\nNotice, that listening will start automatically when\neither `onUpdate` or `visible` are set",
"examples": []
},
{
diff --git a/javascript/components/UserLocation.js b/javascript/components/UserLocation.js
index 440d18823..ba50c6a2f 100644
--- a/javascript/components/UserLocation.js
+++ b/javascript/components/UserLocation.js
@@ -142,8 +142,6 @@ class UserLocation extends React.Component {
async componentDidMount() {
this._isMounted = true;
- locationManager.addListener(this._onLocationUpdate);
-
await this.setLocationManager({
running: this.needsLocationManagerRunning(),
});
@@ -167,14 +165,13 @@ class UserLocation extends React.Component {
async componentWillUnmount() {
this._isMounted = false;
- locationManager.removeListener(this._onLocationUpdate);
await this.setLocationManager({running: false});
}
/**
- * Whether to start or stop the locationManager
+ * Whether to start or stop listening to the locationManager
*
- * Notice, that locationManager will start automatically when
+ * Notice, that listening will start automatically when
* either `onUpdate` or `visible` are set
*
* @async
@@ -185,12 +182,11 @@ class UserLocation extends React.Component {
if (this.locationManagerRunning !== running) {
this.locationManagerRunning = running;
if (running) {
- locationManager.start();
-
+ locationManager.addListener(this._onLocationUpdate);
const location = await locationManager.getLastKnownLocation();
this._onLocationUpdate(location);
} else {
- locationManager.stop();
+ locationManager.removeListener(this._onLocationUpdate);
}
}
}
diff --git a/javascript/modules/location/locationManager.js b/javascript/modules/location/locationManager.js
index 52e2b6814..9023d6026 100644
--- a/javascript/modules/location/locationManager.js
+++ b/javascript/modules/location/locationManager.js
@@ -38,6 +38,9 @@ class LocationManager {
}
addListener(listener) {
+ if (!this._isListening) {
+ this.start();
+ }
if (!this._listeners.includes(listener)) {
this._listeners.push(listener);
@@ -49,10 +52,14 @@ class LocationManager {
removeListener(listener) {
this._listeners = this._listeners.filter((l) => l !== listener);
+ if (this._listeners.length === 0) {
+ this.stop();
+ }
}
removeAllListeners() {
this._listeners = [];
+ this.stop();
}
start(displacement = 0) {