[android] - remove timer from compassView, make fading when facing north optional. (#6786)

save state for recreating map view

fix review comments

unit test covering default value for fading and setting a new value

fixup setter, fix default value

11 files changed, 165 insertions, 76 deletions

diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/constants/MapboxConstants.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/constants/MapboxConstants.java
index 10ebecee27..10d5c55044 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/constants/MapboxConstants.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/constants/MapboxConstants.java
@@ -115,6 +115,7 @@ public class MapboxConstants {
     public static final String STATE_COMPASS_MARGIN_TOP = "compassMarginTop";
     public static final String STATE_COMPASS_MARGIN_RIGHT = "compassMarginRight";
     public static final String STATE_COMPASS_MARGIN_BOTTOM = "compassMarginBottom";
+    public static final String STATE_COMPASS_FADE_WHEN_FACING_NORTH = "compassFade";
     public static final String STATE_LOGO_GRAVITY = "logoGravity";
     public static final String STATE_LOGO_MARGIN_LEFT = "logoMarginLeft";
     public static final String STATE_LOGO_MARGIN_TOP = "logoMarginTop";
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/CompassViewSettings.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/CompassViewSettings.java
new file mode 100644
index 0000000000..3e1b14d641
--- /dev/null
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/CompassViewSettings.java
@@ -0,0 +1,21 @@
+package com.mapbox.mapboxsdk.maps;
+
+/**
+ * Settings for the overlain views of a MapboxMap. Used by UiSettings.
+ */
+class CompassViewSettings extends ViewSettings{
+
+    private boolean fadeFacingNorth = true;
+
+    public CompassViewSettings() {
+        super();
+    }
+
+    public boolean isFadeFacingNorth() {
+        return fadeFacingNorth;
+    }
+
+    public void setFadeFacingNorth(boolean fadeFacingNorth) {
+        this.fadeFacingNorth = fadeFacingNorth;
+    }
+}
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
index 920cafcb8c..32b327e8d0 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
@@ -329,6 +329,7 @@ public class MapView extends FrameLayout {
             int tenDp = (int) getResources().getDimension(R.dimen.ten_dp);
             uiSettings.setCompassMargins(tenDp, tenDp, tenDp, tenDp);
         }
+        uiSettings.setCompassFadeFacingNorth(options.getCompassFadeFacingNorth());
 
         // Logo
         uiSettings.setLogoEnabled(options.getLogoEnabled());
@@ -414,6 +415,7 @@ public class MapView extends FrameLayout {
                     savedInstanceState.getInt(MapboxConstants.STATE_COMPASS_MARGIN_TOP),
                     savedInstanceState.getInt(MapboxConstants.STATE_COMPASS_MARGIN_RIGHT),
                     savedInstanceState.getInt(MapboxConstants.STATE_COMPASS_MARGIN_BOTTOM));
+            uiSettings.setCompassFadeFacingNorth(savedInstanceState.getBoolean(MapboxConstants.STATE_COMPASS_FADE_WHEN_FACING_NORTH));
 
             // Logo
             uiSettings.setLogoEnabled(savedInstanceState.getBoolean(MapboxConstants.STATE_LOGO_ENABLED));
@@ -551,6 +553,7 @@ public class MapView extends FrameLayout {
         outState.putInt(MapboxConstants.STATE_COMPASS_MARGIN_TOP, uiSettings.getCompassMarginTop());
         outState.putInt(MapboxConstants.STATE_COMPASS_MARGIN_BOTTOM, uiSettings.getCompassMarginBottom());
         outState.putInt(MapboxConstants.STATE_COMPASS_MARGIN_RIGHT, uiSettings.getCompassMarginRight());
+        outState.putBoolean(MapboxConstants.STATE_COMPASS_FADE_WHEN_FACING_NORTH, uiSettings.isCompassFadeWhenFacingNorth());
 
         // UiSettings - Logo
         outState.putInt(MapboxConstants.STATE_LOGO_GRAVITY, uiSettings.getLogoGravity());
@@ -2736,6 +2739,10 @@ public class MapView extends FrameLayout {
         setWidgetMargins(compassView, left, top, right, bottom);
     }
 
+    void setCompassFadeFacingNorth(boolean compassFadeFacingNorth) {
+        compassView.fadeCompassViewFacingNorth(compassFadeFacingNorth);
+    }
+
     //
     // Logo
     //
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMapOptions.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMapOptions.java
index 352b3ad12a..a54d246913 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMapOptions.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMapOptions.java
@@ -44,6 +44,7 @@ public class MapboxMapOptions implements Parcelable {
     private boolean debugActive;
 
     private boolean compassEnabled = true;
+    private boolean fadeCompassFacingNorth = true;
     private int compassGravity = Gravity.TOP | Gravity.END;
     private int[] compassMargins;
 
@@ -98,6 +99,7 @@ public class MapboxMapOptions implements Parcelable {
         compassEnabled = in.readByte() != 0;
         compassGravity = in.readInt();
         compassMargins = in.createIntArray();
+        fadeCompassFacingNorth = in.readByte() != 0;
 
         logoEnabled = in.readByte() != 0;
         logoGravity = in.readInt();
@@ -193,6 +195,7 @@ public class MapboxMapOptions implements Parcelable {
                     , ((int) typedArray.getDimension(R.styleable.MapView_compass_margin_top, DIMENSION_TEN_DP * screenDensity))
                     , ((int) typedArray.getDimension(R.styleable.MapView_compass_margin_right, DIMENSION_TEN_DP * screenDensity))
                     , ((int) typedArray.getDimension(R.styleable.MapView_compass_margin_bottom, DIMENSION_TEN_DP * screenDensity))});
+            mapboxMapOptions.compassFadesWhenFacingNorth(typedArray.getBoolean(R.styleable.MapView_compass_fade_facing_north, true));
 
             mapboxMapOptions.logoEnabled(typedArray.getBoolean(R.styleable.MapView_logo_enabled, true));
             mapboxMapOptions.logoGravity(typedArray.getInt(R.styleable.MapView_logo_gravity, Gravity.BOTTOM | Gravity.START));
@@ -359,6 +362,20 @@ public class MapboxMapOptions implements Parcelable {
     }
 
     /**
+     * Specifies if the compass fades to invisible when facing north.
+     * <p>
+     * By default this value is true.
+     * </p>
+     *
+     * @param compassFadeWhenFacingNorth true is compass fades to invisble
+     * @return This
+     */
+    public MapboxMapOptions compassFadesWhenFacingNorth(boolean compassFadeWhenFacingNorth) {
+        this.fadeCompassFacingNorth = compassFadeWhenFacingNorth;
+        return this;
+    }
+
+    /**
      * Specifies the visibility state of a logo for a map view.
      *
      * @param enabled True and logo is shown
@@ -680,6 +697,15 @@ public class MapboxMapOptions implements Parcelable {
     }
 
     /**
+     * Get the current configured state for fading the compass when facing north.
+     *
+     * @return True if compass fades to invisible when facing north
+     */
+    public boolean getCompassFadeFacingNorth() {
+        return fadeCompassFacingNorth;
+    }
+
+    /**
      * Get the current configured visibility state for compass for a map view.
      *
      * @return Visibility state of the compass
@@ -934,6 +960,7 @@ public class MapboxMapOptions implements Parcelable {
         dest.writeByte((byte) (compassEnabled ? 1 : 0));
         dest.writeInt(compassGravity);
         dest.writeIntArray(compassMargins);
+        dest.writeByte((byte) (fadeCompassFacingNorth ? 1 : 0));
 
         dest.writeByte((byte) (logoEnabled ? 1 : 0));
         dest.writeInt(logoGravity);
@@ -979,6 +1006,7 @@ public class MapboxMapOptions implements Parcelable {
 
         if (debugActive != options.debugActive) return false;
         if (compassEnabled != options.compassEnabled) return false;
+        if (fadeCompassFacingNorth != options.fadeCompassFacingNorth) return false;
         if (compassGravity != options.compassGravity) return false;
         if (logoEnabled != options.logoEnabled) return false;
         if (logoGravity != options.logoGravity) return false;
@@ -1022,6 +1050,7 @@ public class MapboxMapOptions implements Parcelable {
         int result = cameraPosition != null ? cameraPosition.hashCode() : 0;
         result = 31 * result + (debugActive ? 1 : 0);
         result = 31 * result + (compassEnabled ? 1 : 0);
+        result = 31 * result + (fadeCompassFacingNorth ? 1 : 0);
         result = 31 * result + compassGravity;
         result = 31 * result + Arrays.hashCode(compassMargins);
         result = 31 * result + (logoEnabled ? 1 : 0);
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/UiSettings.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/UiSettings.java
index a06f449309..d6ad80d11e 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/UiSettings.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/UiSettings.java
@@ -15,7 +15,7 @@ public class UiSettings {
 
     private MapView mapView;
 
-    private ViewSettings compassSettings;
+    private CompassViewSettings compassSettings;
     private ViewSettings logoSettings;
     private ViewSettings attributionSettings;
 
@@ -39,7 +39,7 @@ public class UiSettings {
 
     UiSettings(@NonNull MapView mapView) {
         this.mapView = mapView;
-        this.compassSettings = new ViewSettings();
+        this.compassSettings = new CompassViewSettings();
         this.logoSettings = new ViewSettings();
         this.attributionSettings = new ViewSettings();
     }
@@ -86,6 +86,28 @@ public class UiSettings {
     }
 
     /**
+     * Enables or disables fading of the compass when facing north.
+     * <p>
+     * By default this feature is enabled
+     * </p>
+     *
+     * @param compassFadeFacingNorth True to enable the fading animation; false to disable it
+     */
+    public void setCompassFadeFacingNorth(boolean compassFadeFacingNorth) {
+        compassSettings.setFadeFacingNorth(compassFadeFacingNorth);
+        mapView.setCompassFadeFacingNorth(compassFadeFacingNorth);
+    }
+
+    /**
+     * Returns whether the compass performs a fading animation out when facing north.
+     *
+     * @return True if the compass will fade, false if it remains visible
+     */
+    public boolean isCompassFadeWhenFacingNorth(){
+        return compassSettings.isFadeFacingNorth();
+    }
+
+    /**
      * Returns the gravity value of the CompassView
      *
      * @return The gravity
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/widgets/CompassView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/widgets/CompassView.java
index e469eb14ab..163c7a610a 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/widgets/CompassView.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/widgets/CompassView.java
@@ -15,18 +15,24 @@ import com.mapbox.mapboxsdk.R;
 import com.mapbox.mapboxsdk.maps.MapboxMap;
 
 import java.lang.ref.WeakReference;
-import java.util.Timer;
-import java.util.TimerTask;
 
 /**
- * UI element overlaid on a map to show the map's bearing
- * when it isn't true north (0.0). Tapping the compass resets the bearing to true
- * north and hides the compass.
+ * UI element overlaid on a map to show the map's bearing when it isn't true north (0.0). Tapping
+ * the compass resets the bearing to true north and hides the compass.
+ * <p>
+ * You can change the behaviour of this View during initialisation with
+ * {@link com.mapbox.mapboxsdk.maps.MapboxMapOptions}, and xml attributes. While running you can
+ * use {@link com.mapbox.mapboxsdk.maps.UiSettings}.
+ * </p>
  */
-public final class CompassView extends ImageView {
+public final class CompassView extends ImageView implements Runnable {
 
-    private Timer northTimer;
-    private double direction = 0.0f;
+    private static final long TIME_WAIT_IDLE = 500;
+    private static final long TIME_FADE_ANIMATION = TIME_WAIT_IDLE;
+    private static final long TIME_MAP_NORTH_ANIMATION = 150;
+
+    private double direction = 0.0;
+    private boolean fadeCompassViewFacingNorth = true;
     private ViewPropertyAnimatorCompat fadeAnimator;
 
     public CompassView(Context context) {
@@ -45,8 +51,6 @@ public final class CompassView extends ImageView {
     }
 
     private void initialize(Context context) {
-
-        // View configuration
         setImageDrawable(ContextCompat.getDrawable(getContext(), R.drawable.compass));
         setContentDescription(getResources().getString(R.string.compassContentDescription));
         setEnabled(false);
@@ -57,105 +61,98 @@ public final class CompassView extends ImageView {
         setLayoutParams(lp);
     }
 
-    public void setMapboxMap(@NonNull MapboxMap mapboxMap){
-        setOnClickListener(new CompassClickListener(mapboxMap));
+    public void setMapboxMap(@NonNull MapboxMap mapboxMap) {
+        setOnClickListener(new CompassClickListener(mapboxMap, this));
+    }
+
+    private void resetAnimation() {
+        if (fadeAnimator != null) {
+            fadeAnimator.cancel();
+        }
+        fadeAnimator = null;
+    }
+
+    public boolean isHidden() {
+        return fadeCompassViewFacingNorth && isFacingNorth();
+    }
+
+    public boolean isFacingNorth() {
+        // increase range more than just 0.0
+        return direction >= 359.0 || direction <= 1.0;
     }
 
     @Override
     public void setEnabled(boolean enabled) {
         super.setEnabled(enabled);
-        if (enabled) {
-            if (direction != 0.0) {
-                if (northTimer != null) {
-                    northTimer.cancel();
-                    northTimer = null;
-                }
-                if (fadeAnimator != null) {
-                    fadeAnimator.cancel();
-                }
-                fadeAnimator = null;
-                setAlpha(1.0f);
-                setVisibility(View.VISIBLE);
-            }
+        if (enabled && !isHidden()) {
+            resetAnimation();
+            setAlpha(1.0f);
+            setVisibility(View.VISIBLE);
         } else {
-            if (northTimer != null) {
-                northTimer.cancel();
-                northTimer = null;
-            }
-            if (fadeAnimator != null) {
-                fadeAnimator.cancel();
-            }
-            fadeAnimator = null;
+            resetAnimation();
+            setAlpha(0.0f);
             setVisibility(View.INVISIBLE);
         }
     }
 
-    public void update(double direction) {
+    public void update(final double direction) {
         this.direction = direction;
-        setRotation((float) direction);
 
         if (!isEnabled()) {
             return;
         }
 
-        if (direction == 0.0) {
-            if (getVisibility() == View.INVISIBLE) {
+        if (isHidden()) {
+            if (getVisibility() == View.INVISIBLE || fadeAnimator != null) {
                 return;
             }
-
-            if (northTimer == null) {
-                if (fadeAnimator != null) {
-                    fadeAnimator.cancel();
-                }
-                fadeAnimator = null;
-
-                northTimer = new Timer("CompassView North timer");
-                northTimer.schedule(new TimerTask() {
-                    @Override
-                    public void run() {
-                        post(new Runnable() {
-                            @Override
-                            public void run() {
-                                setAlpha(1.0f);
-                                fadeAnimator = ViewCompat.animate(CompassView.this).alpha(0.0f).setDuration(1000).withLayer();
-                                fadeAnimator.setListener(new ViewPropertyAnimatorListenerAdapter() {
-                                    @Override
-                                    public void onAnimationEnd(View view) {
-                                        setVisibility(View.INVISIBLE);
-                                        northTimer = null;
-                                    }
-                                });
-                            }
-                        });
-                    }
-                }, 1000);
-            }
+            postDelayed(this, TIME_WAIT_IDLE);
+            return;
         } else {
-            if (northTimer != null) {
-                northTimer.cancel();
-                northTimer = null;
-            }
-            if (fadeAnimator != null) {
-                fadeAnimator.cancel();
-            }
+            resetAnimation();
             setAlpha(1.0f);
             setVisibility(View.VISIBLE);
         }
+
+        setRotation((float) direction);
+    }
+
+    public void fadeCompassViewFacingNorth(boolean compassFadeFacingNorth) {
+        fadeCompassViewFacingNorth = compassFadeFacingNorth;
+    }
+
+    @Override
+    public void run() {
+        if (isFacingNorth() && fadeCompassViewFacingNorth) {
+            resetAnimation();
+            fadeAnimator = ViewCompat.animate(CompassView.this).alpha(0.0f).setDuration(TIME_FADE_ANIMATION).withLayer();
+            fadeAnimator.setListener(new ViewPropertyAnimatorListenerAdapter() {
+                @Override
+                public void onAnimationEnd(View view) {
+                    setVisibility(View.INVISIBLE);
+                    resetAnimation();
+                }
+            });
+        }
     }
 
     static class CompassClickListener implements View.OnClickListener {
 
         private WeakReference<MapboxMap> mapboxMap;
+        private WeakReference<CompassView> compassView;
 
-        public CompassClickListener(final MapboxMap mapboxMap) {
+        CompassClickListener(final MapboxMap mapboxMap, CompassView compassView) {
             this.mapboxMap = new WeakReference<>(mapboxMap);
+            this.compassView = new WeakReference<>(compassView);
         }
 
         @Override
         public void onClick(View view) {
             final MapboxMap mapboxMap = this.mapboxMap.get();
-            if (mapboxMap != null) {
+            final CompassView compassView = this.compassView.get();
+            if (mapboxMap != null && compassView != null) {
                 mapboxMap.resetNorth();
+                compassView.postDelayed(compassView, TIME_WAIT_IDLE + TIME_MAP_NORTH_ANIMATION);
             }
         }
     }
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/res-public/values/public.xml b/platform/android/MapboxGLAndroidSDK/src/main/res-public/values/public.xml
index 777d879d48..4fbf3d5fe1 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/res-public/values/public.xml
+++ b/platform/android/MapboxGLAndroidSDK/src/main/res-public/values/public.xml
@@ -29,6 +29,7 @@
     <public name="compass_margin_top" type="attr" />
     <public name="compass_margin_right" type="attr" />
     <public name="compass_margin_bottom" type="attr" />
+    <public name="compass_fade_facing_north" type="attr" />
     <public name="logo_gravity" type="attr" />
     <public name="logo_margin_left" type="attr" />
     <public name="logo_margin_top" type="attr" />
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/res/values/attrs.xml b/platform/android/MapboxGLAndroidSDK/src/main/res/values/attrs.xml
index f0740a4e53..458056c14e 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/res/values/attrs.xml
+++ b/platform/android/MapboxGLAndroidSDK/src/main/res/values/attrs.xml
@@ -64,6 +64,7 @@
         <attr name="compass_margin_top" format="dimension" />
         <attr name="compass_margin_right" format="dimension" />
         <attr name="compass_margin_bottom" format="dimension" />
+        <attr name="compass_fade_facing_north" format="boolean" />
 
         <!--Logo-->
         <attr name="logo_gravity">
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/annotation/PolygonActivity.java b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/annotation/PolygonActivity.java
index d8c3cb6011..dd80b2ba5a 100644
--- a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/annotation/PolygonActivity.java
+++ b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/annotation/PolygonActivity.java
@@ -58,6 +58,7 @@ public class PolygonActivity extends AppCompatActivity implements OnMapReadyCall
                 .attributionTintColor(RED_COLOR)
                 // deprecated feature!
                 .textureMode(true)
+                .compassFadesWhenFacingNorth(false)
                 .styleUrl(Style.MAPBOX_STREETS)
                 .camera(new CameraPosition.Builder()
                         .target(new LatLng(45.520486, -122.673541))
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/main/res/layout/activity_dynamic_marker.xml b/platform/android/MapboxGLAndroidSDKTestApp/src/main/res/layout/activity_dynamic_marker.xml
index 1b1dca5327..d6507559ec 100644
--- a/platform/android/MapboxGLAndroidSDKTestApp/src/main/res/layout/activity_dynamic_marker.xml
+++ b/platform/android/MapboxGLAndroidSDKTestApp/src/main/res/layout/activity_dynamic_marker.xml
@@ -28,6 +28,7 @@
                 android:layout_height="match_parent"
                 app:center_latitude="51.506675"
                 app:center_longitude="-0.128699"
+                app:compass_fade_facing_north="false"
                 app:direction="90"
                 app:tilt="40"
                 app:zoom="10" />
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/maps/UiSettingsTest.java b/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/maps/UiSettingsTest.java
index 300ed1d73a..14fc84723d 100644
--- a/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/maps/UiSettingsTest.java
+++ b/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/maps/UiSettingsTest.java
@@ -7,6 +7,7 @@ import org.junit.Test;
 import org.mockito.InjectMocks;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
@@ -57,6 +58,13 @@ public class UiSettingsTest {
     }
 
     @Test
+    public void testCompassFadeWhenFacingNorth(){
+        assertTrue("Compass should fade when facing north by default.", uiSettings.isCompassFadeWhenFacingNorth());
+        uiSettings.setCompassFadeFacingNorth(false);
+        assertFalse("Compass fading should be disabled", uiSettings.isCompassFadeWhenFacingNorth());
+    }
+
+    @Test
     public void testLogoEnabled() {
         uiSettings.setLogoEnabled(true);
         assertEquals("Logo should be enabled", true, uiSettings.isLogoEnabled());