Issue 915: Stop/start Location manager based on listeners registered. (…

…rnmapbox#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

@@ -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();
       });
     });
 

__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,23 +100,31 @@ describe('LocationManager', () => {
     });
 
     describe('#removeListener', () => {
+      MapboxGLLocationManager.stop = jest.fn();
+
       test('removes selected listener', () => {
         // just two different functions
         const listenerA = jest.fn(() => 'listenerA');
         const listenerB = () => 'listenerB';
 
         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);
       });
     });
 

docs/UserLocation.md

@@ -18,7 +18,7 @@
 ### methods
 #### setLocationManager({running})
 
-Whether to start or stop the locationManager<br/><br/>Notice, that locationManager will start automatically when<br/>either `onUpdate` or `visible` are set
+Whether to start or stop listening to the locationManager<br/><br/>Notice, that listening will start automatically when<br/>either `onUpdate` or `visible` are set
 
 ##### arguments
 | Name | Type | Required | Description  |

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<void>}",
+        "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<void>}",
         "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": []
       },
       {

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);
       }
     }
   }

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) {