fix: Make KML/GeoJSON lines and polygons clickable by default (#614)

As discussed in #558, the original intent when combining the KML and GeoJSON renderers was to have GeoJSON and KML lines and polygons clickable by default.

However, there was a regression in #454 that unintentionally disabled clicks for KML and GeoJSON polygons when attempting to allow developers to use GeoJSON custom styles to set clickable.

I investigated both KML and GeoJSON lines and polygons, and here is the current state of the library:

* For KML/GeoJSON polygons - we're currently supporting developer-defined clickable styles for GeoJSON (the change in #454), but that results in a default of not clickable for GeoJSON and KML. And there isn't a way to override the default in KmlLayer.
* For KML/GeoJSON lines - we're currently ignoring developer-defined clickable styles and hard-coding the default clickable style to true.

This PR solves both problems (default of "not clickable" and supporting developer-defined styles) as follows:

* Makes all styles for GeoJSON and KML lines and polygons clickable by default by setting clickable=true in style constructors rather than the renderer. This also allows the developer to set a custom style with clickable=false on GeoJSON lines and polygons (custom programmatic KML styles currently aren't supported by the library).
* Make clickable default to true in the Style superclass - this should hopefully avoid a similar issue in any new implementations that extend Style
* Normalize the GeoJsonLineStringStyle and GeoJsonPolygonStyle clickable implementations - GeoJsonPolygonStyle was missing code related to the clickable attribute
* Add missing <name> attribute to the KML polygon demo file used in the MultiLayerDemoActivity
* Add unit tests:
    * For GeoJSON - test both default and programmatic custom clickable styles for both the style and renderer classes
    * For KML - test the default style for both the style and renderer classes
    * Refactor KML and GeoJSON test utility methods used in more than one test class to new Kml/GeoJsonTestUtil classes
    * Fix GeoJSON polygonStyle.toPolygonOptions().isVisible() test (this previously didn't check PolygonOptions)

I've also added TODO statements in the unit tests where we could expand coverage further (i.e., after adding the layers to the map), but that requires additional test instrumentation to have a live GoogleMap object.

Fixes #558

Refs: 558

Author: barbeau

demo/src/main/res/raw/south_london_square_kml.kml

@@ -2,6 +2,7 @@
 <kml xmlns="http://www.opengis.net/kml/2.2">
     <Document>
         <Placemark>
+            <name>South London Polygon KML</name>
             <ExtendedData></ExtendedData>
             <Polygon>
                 <outerBoundaryIs>

library/src/androidTest/java/com/google/maps/android/data/geojson/GeoJsonLineStringStyleTest.java

@@ -1,13 +1,15 @@
 package com.google.maps.android.data.geojson;
 
-import android.graphics.Color;
-
 import org.junit.Before;
 import org.junit.Test;
 
+import android.graphics.Color;
+
 import java.util.Arrays;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 public class GeoJsonLineStringStyleTest {
     private GeoJsonLineStringStyle lineStringStyle;
@@ -75,6 +77,7 @@ public void testDefaultLineStringStyle() {
         assertTrue(lineStringStyle.isVisible());
         assertEquals(10.0f, lineStringStyle.getWidth(), 0);
         assertEquals(0.0f, lineStringStyle.getZIndex(), 0);
+        assertTrue(lineStringStyle.isClickable());
     }
 
     @Test
@@ -84,5 +87,6 @@ public void testDefaultGetPolylineOptions() {
         assertTrue(lineStringStyle.toPolylineOptions().isVisible());
         assertEquals(10.0f, lineStringStyle.toPolylineOptions().getWidth(), 0);
         assertEquals(0.0f, lineStringStyle.toPolylineOptions().getZIndex(), 0);
+        assertTrue(lineStringStyle.toPolylineOptions().isClickable());
     }
 }

library/src/androidTest/java/com/google/maps/android/data/geojson/GeoJsonParserTest.java

@@ -1,13 +1,15 @@
 package com.google.maps.android.data.geojson;
 
-import android.graphics.Color;
-
 import org.junit.Before;
 import org.junit.Test;
 
+import android.graphics.Color;
+
 import java.util.Arrays;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 public class GeoJsonPolygonStyleTest {
     private GeoJsonPolygonStyle polygonStyle;
@@ -91,6 +93,7 @@ public void testDefaultPolygonStyle() {
         assertEquals(10.0f, polygonStyle.getStrokeWidth(), 0);
         assertTrue(polygonStyle.isVisible());
         assertEquals(0.0f, polygonStyle.getZIndex(), 0);
+        assertTrue(polygonStyle.isClickable());
     }
 
     @Test
@@ -99,7 +102,8 @@ public void testDefaultGetPolygonOptions() {
         assertFalse(polygonStyle.toPolygonOptions().isGeodesic());
         assertEquals(Color.BLACK, polygonStyle.toPolygonOptions().getStrokeColor());
         assertEquals(10.0f, polygonStyle.toPolygonOptions().getStrokeWidth(), 0);
-        assertTrue(polygonStyle.isVisible());
+        assertTrue(polygonStyle.toPolygonOptions().isVisible());
         assertEquals(0.0f, polygonStyle.toPolygonOptions().getZIndex(), 0);
+        assertTrue(polygonStyle.toPolygonOptions().isClickable());
     }
 }

library/src/androidTest/java/com/google/maps/android/data/geojson/GeoJsonPolygonStyleTest.java

@@ -2,8 +2,8 @@
 
 import com.google.android.gms.maps.GoogleMap;
 import com.google.android.gms.maps.model.LatLng;
+import com.google.maps.android.data.Feature;
 
-import org.json.JSONObject;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -13,7 +13,11 @@
 import java.util.HashMap;
 import java.util.Set;
 
-import static org.junit.Assert.*;
+import static com.google.maps.android.data.geojson.GeoJsonTestUtil.createFeatureCollection;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 public class GeoJsonRendererTest {
     private GoogleMap mMap1;
@@ -28,6 +32,10 @@ public void setUp() throws Exception {
         GeoJsonParser parser = new GeoJsonParser(createFeatureCollection());
         HashMap<GeoJsonFeature, Object> geoJsonFeatures = new HashMap<>();
         for (GeoJsonFeature feature : parser.getFeatures()) {
+            // Set default styles
+            feature.setPointStyle(new GeoJsonPointStyle());
+            feature.setLineStringStyle(new GeoJsonLineStringStyle());
+            feature.setPolygonStyle(new GeoJsonPolygonStyle());
             geoJsonFeatures.put(feature, null);
         }
         geoJsonFeaturesSet = geoJsonFeatures.keySet();
@@ -85,42 +93,39 @@ public void testRemoveFeature() {
         assertFalse(mRenderer.getFeatures().contains(mGeoJsonFeature));
     }
 
-    private JSONObject createFeatureCollection() throws Exception {
-        return new JSONObject(
-                "{ \"type\": \"FeatureCollection\",\n"
-                        + "    \"features\": [\n"
-                        + "      { \"type\": \"Feature\",\n"
-                        + "        \"geometry\": {\"type\": \"MultiPoint\", \"coordinates\": [[102.0,"
-                        + " 0.5], [100, 0.5]]},\n"
-                        + "        \"properties\": {\"title\": \"Test MultiPoint\"}\n"
-                        + "        },\n"
-                        + "      { \"type\": \"Feature\",\n"
-                        + "        \"geometry\": {\n"
-                        + "          \"type\": \"MultiLineString\",\n"
-                        + "          \"coordinates\": [\n"
-                        + "            [[100, 0],[101, 1]], [[102, 2], [103, 3]]\n"
-                        + "            ]\n"
-                        + "          },\n"
-                        + "        \"properties\": {\n"
-                        + "          \"title\": \"Test MultiLineString\"\n"
-                        + "          }\n"
-                        + "        },\n"
-                        + "      { \"type\": \"Feature\",\n"
-                        + "         \"geometry\": {\n"
-                        + "           \"type\": \"MultiPolygon\",\n"
-                        + "           \"coordinates\": [\n"
-                        + "             [[[102.0, 2.0], [103.0, 2.0], [103.0, 3.0], [102.0, 3.0],"
-                        + " [102.0, 2.0]]],\n"
-                        + "      [[[100.0, 0.0], [101.0, 0.0], [101.0, 1.0], [100.0, 1.0], [100.0,"
-                        + " 0.0]],\n"
-                        + "       [[100.2, 0.2], [100.8, 0.2], [100.8, 0.8], [100.2, 0.8], [100.2,"
-                        + " 0.2]]],\n"
-                        + "             ]\n"
-                        + "         },\n"
-                        + "         \"properties\": {\n"
-                        + "           \"title\": \"Test MultiPolygon\"}\n"
-                        + "         }\n"
-                        + "       ]\n"
-                        + "     }");
+    @Test
+    public void testDefaultStyleClickable() {
+        // TODO - we should call mRenderer.addLayerToMap() here for a complete end-to-end test, but
+        // that requires an instantiated GoogleMap be passed into GeoJsonRenderer()
+        for (Feature f : mRenderer.getFeatures()) {
+            assertTrue(((GeoJsonFeature)f).getPolylineOptions().isClickable());
+            assertTrue(((GeoJsonFeature)f).getPolygonOptions().isClickable());
+        }
+    }
+
+    @Test
+    public void testCustomStyleNotClickable() throws Exception {
+        GeoJsonParser parser = new GeoJsonParser(createFeatureCollection());
+        HashMap<GeoJsonFeature, Object> geoJsonFeatures = new HashMap<>();
+        for (GeoJsonFeature feature : parser.getFeatures()) {
+            // Set default style for points
+            feature.setPointStyle(new GeoJsonPointStyle());
+            // Set custom style of not clickable for lines and polygons
+            GeoJsonLineStringStyle ls = new GeoJsonLineStringStyle();
+            ls.setClickable(false);
+            feature.setLineStringStyle(ls);
+            GeoJsonPolygonStyle ps = new GeoJsonPolygonStyle();
+            ps.setClickable(false);
+            feature.setPolygonStyle(ps);
+            geoJsonFeatures.put(feature, null);
+        }
+
+        GeoJsonRenderer renderer = new GeoJsonRenderer(mMap1, geoJsonFeatures, null, null, null, null);
+        // TODO - we should call renderer.addLayerToMap() here for a complete end-to-end test, but
+        // that requires an instantiated GoogleMap be passed into GeoJsonRenderer()
+        for (Feature f : renderer.getFeatures()) {
+            assertFalse(((GeoJsonFeature)f).getPolylineOptions().isClickable());
+            assertFalse(((GeoJsonFeature)f).getPolygonOptions().isClickable());
+        }
     }
 }