From c452a7da90fefe617ae4f1f2cbb317d33a594660 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Fri, 20 Mar 2020 16:00:45 +0530 Subject: [PATCH 01/16] Move static methods and final variables to the top --- .../gnd/ui/map/gms/GoogleMapsMapAdapter.java | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapAdapter.java b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapAdapter.java index 9717cd7e97..81db56252f 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapAdapter.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapAdapter.java @@ -54,6 +54,9 @@ class GoogleMapsMapAdapter implements MapAdapter { private final GoogleMap map; private final Context context; private final MarkerIconFactory markerIconFactory; + private final PublishSubject markerClickSubject = PublishSubject.create(); + private final PublishSubject dragInteractionSubject = PublishSubject.create(); + private final BehaviorSubject cameraMoves = BehaviorSubject.create(); /** * References to Google Maps SDK Markers present on the map. Used to sync and update markers with @@ -61,10 +64,6 @@ class GoogleMapsMapAdapter implements MapAdapter { */ private Set markers = new HashSet<>(); - private final PublishSubject markerClickSubject = PublishSubject.create(); - private final PublishSubject dragInteractionSubject = PublishSubject.create(); - private final BehaviorSubject cameraMoves = BehaviorSubject.create(); - @Nullable private LatLng cameraTargetBeforeDrag; public GoogleMapsMapAdapter(GoogleMap map, Context context, MarkerIconFactory markerIconFactory) { @@ -86,6 +85,14 @@ public GoogleMapsMapAdapter(GoogleMap map, Context context, MarkerIconFactory ma onCameraMove(); } + private static Point fromLatLng(LatLng latLng) { + return Point.newBuilder().setLatitude(latLng.latitude).setLongitude(latLng.longitude).build(); + } + + private static LatLng toLatLng(Point point) { + return new LatLng(point.getLatitude(), point.getLongitude()); + } + private boolean onMarkerClick(Marker marker) { if (map.getUiSettings().isZoomGesturesEnabled()) { markerClickSubject.onNext((MapPin) marker.getTag()); @@ -187,14 +194,6 @@ public void setMapPins(ImmutableSet updatedPins) { stream(pinsToAdd).forEach(this::addMapPin); } - private static Point fromLatLng(LatLng latLng) { - return Point.newBuilder().setLatitude(latLng.latitude).setLongitude(latLng.longitude).build(); - } - - private static LatLng toLatLng(Point point) { - return new LatLng(point.getLatitude(), point.getLongitude()); - } - private void removeMarker(Marker marker) { Timber.v("Removing marker %s", marker.getId()); marker.remove(); From e970f971e5f5c59bee34bded71d14636aaa434b2 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Fri, 20 Mar 2020 16:02:25 +0530 Subject: [PATCH 02/16] Log -> Timber --- .../ui/home/mapcontainer/MapContainerFragment.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java index dae8936e1f..10c8984397 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java @@ -21,7 +21,6 @@ import android.content.res.ColorStateList; import android.os.Bundle; -import android.util.Log; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; @@ -48,10 +47,11 @@ import com.google.android.material.floatingactionbutton.FloatingActionButton; import io.reactivex.Single; import javax.inject.Inject; +import timber.log.Timber; /** Main app view, displaying the map and related controls (center cross-hairs, add button, etc). */ public class MapContainerFragment extends AbstractFragment { - private static final String TAG = MapContainerFragment.class.getSimpleName(); + private static final String MAP_FRAGMENT_KEY = MapProvider.class.getName() + "#fragment"; @Inject MapProvider mapProvider; @@ -123,7 +123,7 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat } private void onMapReady(MapAdapter map) { - Log.d(TAG, "MapAdapter ready. Updating subscriptions"); + Timber.d("MapAdapter ready. Updating subscriptions"); // Observe events emitted by the ViewModel. mapContainerViewModel.getMapPins().observe(this, map::setMapPins); mapContainerViewModel @@ -157,7 +157,7 @@ private void onFeatureSheetStateChange(FeatureSheetState state, MapAdapter map) map.enable(); break; default: - Log.e(TAG, "Unhandled visibility: " + state.getVisibility()); + Timber.e("Unhandled visibility: %s", state.getVisibility()); break; } } @@ -189,11 +189,11 @@ private void disableAddFeatureBtn() { private void onLocationLockStateChange(BooleanOrError result, MapAdapter map) { result.error().ifPresent(this::onLocationLockError); if (result.isTrue()) { - Log.d(TAG, "Location lock enabled"); + Timber.d("Location lock enabled"); map.enableCurrentLocationIndicator(); locationLockBtn.setImageResource(R.drawable.ic_gps_blue); } else { - Log.d(TAG, "Location lock disabled"); + Timber.d("Location lock disabled"); locationLockBtn.setImageResource(R.drawable.ic_gps_grey600); } } @@ -213,7 +213,7 @@ private void showUserActionFailureMessage(int resId) { } private void onCameraUpdate(MapContainerViewModel.CameraUpdate update, MapAdapter map) { - Log.v(TAG, "Update camera: " + update); + Timber.v("Update camera: %s", update); if (update.getMinZoomLevel().isPresent()) { map.moveCamera( update.getCenter(), Math.max(update.getMinZoomLevel().get(), map.getCurrentZoomLevel())); From d49d91191d8d821eb3ff103dba703ebcea621070 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Fri, 20 Mar 2020 16:09:52 +0530 Subject: [PATCH 03/16] Add map type button --- .../main/res/layout/map_container_frag.xml | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/gnd/src/main/res/layout/map_container_frag.xml b/gnd/src/main/res/layout/map_container_frag.xml index d310b3e8c4..6bc7e79534 100644 --- a/gnd/src/main/res/layout/map_container_frag.xml +++ b/gnd/src/main/res/layout/map_container_frag.xml @@ -16,17 +16,16 @@ ~ limitations under the License. --> - + type="com.google.android.gnd.ui.home.mapcontainer.MapContainerViewModel" /> + type="com.google.android.gnd.ui.home.HomeScreenViewModel" /> + android:layout_height="match_parent" /> + android:src="@drawable/ic_crosshairs" /> + app:useCompatPadding="true" /> + app:useCompatPadding="true" /> + android:tint="@color/colorBackground" /> + + \ No newline at end of file From 7f4419826934c5156ef99c33aeab5a80d19c177a Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Fri, 20 Mar 2020 16:37:51 +0530 Subject: [PATCH 04/16] Add getter/setter for map type --- .../java/com/google/android/gnd/ui/map/MapAdapter.java | 6 ++++++ .../com/google/android/gnd/ui/map/MapProvider.java | 4 ++++ .../android/gnd/ui/map/gms/GoogleMapsMapAdapter.java | 10 ++++++++++ .../android/gnd/ui/map/gms/GoogleMapsMapProvider.java | 10 ++++++++++ 4 files changed, 30 insertions(+) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/MapAdapter.java b/gnd/src/main/java/com/google/android/gnd/ui/map/MapAdapter.java index ff83d64ee0..8c2238c0fb 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/MapAdapter.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/MapAdapter.java @@ -70,6 +70,12 @@ public interface MapAdapter { /** Update map pins shown on map. */ void setMapPins(ImmutableSet pins); + /** Get current map type. */ + int getMapType(); + + /** Update map type. */ + void setMapType(int mapType); + /** Returns the bounds of the currently visibly viewport. */ LatLngBounds getViewport(); } diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java b/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java index 874809aaec..76969d79cc 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java @@ -26,4 +26,8 @@ public interface MapProvider { Fragment getFragment(); Single getMapAdapter(); + + int getMapType(); + + void setMapType(int mapType); } diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapAdapter.java b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapAdapter.java index 81db56252f..e6f1629823 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapAdapter.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapAdapter.java @@ -194,6 +194,16 @@ public void setMapPins(ImmutableSet updatedPins) { stream(pinsToAdd).forEach(this::addMapPin); } + @Override + public int getMapType() { + return map.getMapType(); + } + + @Override + public void setMapType(int mapType) { + map.setMapType(mapType); + } + private void removeMarker(Marker marker) { Timber.v("Removing marker %s", marker.getId()); marker.remove(); diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java index 8c6fd12421..ade16cbdad 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java @@ -68,4 +68,14 @@ public Fragment getFragment() { public Single getMapAdapter() { return map; } + + @Override + public int getMapType() { + return map.getValue().getMapType(); + } + + @Override + public void setMapType(int mapType) { + map.getValue().setMapType(mapType); + } } From ce8db1df1d0223ef2c5e2c2e4b00c62e7fc62895 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Fri, 20 Mar 2020 16:38:20 +0530 Subject: [PATCH 05/16] Create alert dialog on button click and display map types --- .../mapcontainer/MapContainerFragment.java | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java index 10c8984397..571f45d259 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java @@ -28,9 +28,12 @@ import android.widget.Toast; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.appcompat.app.AlertDialog; import androidx.core.view.ViewCompat; import androidx.core.view.WindowInsetsCompat; import butterknife.BindView; +import butterknife.OnClick; +import com.google.android.gms.maps.GoogleMap; import com.google.android.gnd.MainViewModel; import com.google.android.gnd.R; import com.google.android.gnd.databinding.MapContainerFragBinding; @@ -54,6 +57,10 @@ public class MapContainerFragment extends AbstractFragment { private static final String MAP_FRAGMENT_KEY = MapProvider.class.getName() + "#fragment"; + private static final CharSequence[] MAP_TYPE_ITEMS = { + "None", "Normal", "Satellite", "Terrain", "Hybrid" + }; + @Inject MapProvider mapProvider; @BindView(R.id.hamburger_btn) @@ -72,6 +79,47 @@ public class MapContainerFragment extends AbstractFragment { private HomeScreenViewModel homeScreenViewModel; private MainViewModel mainViewModel; + @OnClick(R.id.map_type_btn) + void onClick(ImageButton view) { + showMapTypeSelectorDialog(); + } + + private void showMapTypeSelectorDialog() { + final String dialogTitle = "Select Map Type"; + AlertDialog.Builder builder = new AlertDialog.Builder(getContext()); + builder.setTitle(dialogTitle); + int checkItem = mapProvider.getMapType(); + builder.setSingleChoiceItems( + MAP_TYPE_ITEMS, + checkItem, + (dialog, item) -> { + switch (item) { + case 0: + mapProvider.setMapType(GoogleMap.MAP_TYPE_NONE); + break; + case 1: + mapProvider.setMapType(GoogleMap.MAP_TYPE_NORMAL); + break; + case 2: + mapProvider.setMapType(GoogleMap.MAP_TYPE_SATELLITE); + break; + case 3: + mapProvider.setMapType(GoogleMap.MAP_TYPE_TERRAIN); + break; + case 4: + mapProvider.setMapType(GoogleMap.MAP_TYPE_HYBRID); + break; + default: + Timber.e("Unknown type: %d", item); + } + dialog.dismiss(); + }); + + AlertDialog alertDialog = builder.create(); + alertDialog.setCanceledOnTouchOutside(true); + alertDialog.show(); + } + @Override public void onCreate(@Nullable Bundle savedInstanceState) { super.onCreate(savedInstanceState); From a184c310659d38d4a45964228f88c229884461cf Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Fri, 20 Mar 2020 17:08:00 +0530 Subject: [PATCH 06/16] Store list of map types in GoogleMapsMapProvider --- .../mapcontainer/MapContainerFragment.java | 35 ++++--------------- .../android/gnd/ui/map/MapProvider.java | 3 ++ .../gnd/ui/map/gms/GoogleMapsMapProvider.java | 13 +++++++ gnd/src/main/res/values/strings.xml | 1 + 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java index 571f45d259..f4efae5c57 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java @@ -33,7 +33,6 @@ import androidx.core.view.WindowInsetsCompat; import butterknife.BindView; import butterknife.OnClick; -import com.google.android.gms.maps.GoogleMap; import com.google.android.gnd.MainViewModel; import com.google.android.gnd.R; import com.google.android.gnd.databinding.MapContainerFragBinding; @@ -49,6 +48,7 @@ import com.google.android.gnd.ui.map.MapProvider; import com.google.android.material.floatingactionbutton.FloatingActionButton; import io.reactivex.Single; +import java.util.HashMap; import javax.inject.Inject; import timber.log.Timber; @@ -57,10 +57,6 @@ public class MapContainerFragment extends AbstractFragment { private static final String MAP_FRAGMENT_KEY = MapProvider.class.getName() + "#fragment"; - private static final CharSequence[] MAP_TYPE_ITEMS = { - "None", "Normal", "Satellite", "Terrain", "Hybrid" - }; - @Inject MapProvider mapProvider; @BindView(R.id.hamburger_btn) @@ -85,38 +81,21 @@ void onClick(ImageButton view) { } private void showMapTypeSelectorDialog() { - final String dialogTitle = "Select Map Type"; + HashMap mapTypes = mapProvider.getMapTypes(); + AlertDialog.Builder builder = new AlertDialog.Builder(getContext()); - builder.setTitle(dialogTitle); + builder.setTitle(R.string.select_map_type); int checkItem = mapProvider.getMapType(); builder.setSingleChoiceItems( - MAP_TYPE_ITEMS, + mapTypes.values().toArray(new String[0]), checkItem, (dialog, item) -> { - switch (item) { - case 0: - mapProvider.setMapType(GoogleMap.MAP_TYPE_NONE); - break; - case 1: - mapProvider.setMapType(GoogleMap.MAP_TYPE_NORMAL); - break; - case 2: - mapProvider.setMapType(GoogleMap.MAP_TYPE_SATELLITE); - break; - case 3: - mapProvider.setMapType(GoogleMap.MAP_TYPE_TERRAIN); - break; - case 4: - mapProvider.setMapType(GoogleMap.MAP_TYPE_HYBRID); - break; - default: - Timber.e("Unknown type: %d", item); - } + mapProvider.setMapType(item); dialog.dismiss(); }); AlertDialog alertDialog = builder.create(); - alertDialog.setCanceledOnTouchOutside(true); + alertDialog.setCancelable(true); alertDialog.show(); } diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java b/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java index 76969d79cc..5e4a7f0327 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java @@ -18,6 +18,7 @@ import androidx.fragment.app.Fragment; import io.reactivex.Single; +import java.util.HashMap; /** Common interface for various map provider libraries. */ public interface MapProvider { @@ -30,4 +31,6 @@ public interface MapProvider { int getMapType(); void setMapType(int mapType); + + HashMap getMapTypes(); } diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java index ade16cbdad..fef34f4cca 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java @@ -18,11 +18,13 @@ import androidx.annotation.Nullable; import androidx.fragment.app.Fragment; +import com.google.android.gms.maps.GoogleMap; import com.google.android.gnd.ui.MarkerIconFactory; import com.google.android.gnd.ui.map.MapAdapter; import com.google.android.gnd.ui.map.MapProvider; import io.reactivex.Single; import io.reactivex.subjects.SingleSubject; +import java.util.HashMap; /** Ground map adapter implementation for Google Maps API. */ public class GoogleMapsMapProvider implements MapProvider { @@ -78,4 +80,15 @@ public int getMapType() { public void setMapType(int mapType) { map.getValue().setMapType(mapType); } + + @Override + public HashMap getMapTypes() { + HashMap hashMap = new HashMap<>(); + hashMap.put(GoogleMap.MAP_TYPE_NONE, "None"); + hashMap.put(GoogleMap.MAP_TYPE_NORMAL, "Normal"); + hashMap.put(GoogleMap.MAP_TYPE_SATELLITE, "Satellite"); + hashMap.put(GoogleMap.MAP_TYPE_TERRAIN, "Terrain"); + hashMap.put(GoogleMap.MAP_TYPE_HYBRID, "Hybrid"); + return hashMap; + } } diff --git a/gnd/src/main/res/values/strings.xml b/gnd/src/main/res/values/strings.xml index 7fefb34ded..8554d72c3f 100644 --- a/gnd/src/main/res/values/strings.xml +++ b/gnd/src/main/res/values/strings.xml @@ -140,5 +140,6 @@ Cancel Join project Photo preview + Select Map Type From 45bc8d505c7c35720738aeff4a312091ec58a260 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Fri, 20 Mar 2020 17:13:42 +0530 Subject: [PATCH 07/16] Cleanup and null checks --- .../mapcontainer/MapContainerFragment.java | 29 ++++++++----------- .../gnd/ui/map/gms/GoogleMapsMapProvider.java | 11 +++++-- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java index f4efae5c57..0a2b41c1d0 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java @@ -48,7 +48,6 @@ import com.google.android.gnd.ui.map.MapProvider; import com.google.android.material.floatingactionbutton.FloatingActionButton; import io.reactivex.Single; -import java.util.HashMap; import javax.inject.Inject; import timber.log.Timber; @@ -81,22 +80,18 @@ void onClick(ImageButton view) { } private void showMapTypeSelectorDialog() { - HashMap mapTypes = mapProvider.getMapTypes(); - - AlertDialog.Builder builder = new AlertDialog.Builder(getContext()); - builder.setTitle(R.string.select_map_type); - int checkItem = mapProvider.getMapType(); - builder.setSingleChoiceItems( - mapTypes.values().toArray(new String[0]), - checkItem, - (dialog, item) -> { - mapProvider.setMapType(item); - dialog.dismiss(); - }); - - AlertDialog alertDialog = builder.create(); - alertDialog.setCancelable(true); - alertDialog.show(); + new AlertDialog.Builder(getContext()) + .setTitle(R.string.select_map_type) + .setSingleChoiceItems( + mapProvider.getMapTypes().values().toArray(new String[0]), + mapProvider.getMapType(), + (dialog, which) -> { + mapProvider.setMapType(which); + dialog.dismiss(); + }) + .setCancelable(true) + .create() + .show(); } @Override diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java index fef34f4cca..f5795c9f44 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java @@ -73,12 +73,19 @@ public Single getMapAdapter() { @Override public int getMapType() { - return map.getValue().getMapType(); + if (map != null) { + return map.getValue().getMapType(); + } + throw new IllegalStateException("MapAdapter is null"); } @Override public void setMapType(int mapType) { - map.getValue().setMapType(mapType); + if (map != null) { + map.getValue().setMapType(mapType); + } else { + throw new IllegalStateException("MapAdapter is null"); + } } @Override From 14442defbacb41a95562ce869c5f184554426e71 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Fri, 20 Mar 2020 17:37:12 +0530 Subject: [PATCH 08/16] Add map layer icon --- gnd/src/main/res/drawable/map_layers.xml | 29 ++++++++++++++ .../main/res/layout/map_container_frag.xml | 38 ++++++++++++------- 2 files changed, 53 insertions(+), 14 deletions(-) create mode 100644 gnd/src/main/res/drawable/map_layers.xml diff --git a/gnd/src/main/res/drawable/map_layers.xml b/gnd/src/main/res/drawable/map_layers.xml new file mode 100644 index 0000000000..16b5562c43 --- /dev/null +++ b/gnd/src/main/res/drawable/map_layers.xml @@ -0,0 +1,29 @@ + + + + + + + + + \ No newline at end of file diff --git a/gnd/src/main/res/layout/map_container_frag.xml b/gnd/src/main/res/layout/map_container_frag.xml index 6bc7e79534..9a2852894c 100644 --- a/gnd/src/main/res/layout/map_container_frag.xml +++ b/gnd/src/main/res/layout/map_container_frag.xml @@ -58,15 +58,27 @@ android:src="@drawable/ic_crosshairs" /> - + android:layout_marginBottom="@dimen/map_btn_margin_bottom"> + + - + - - \ No newline at end of file From dcb09e68422f21c8c4d549545e04fe892d36717b Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Fri, 20 Mar 2020 18:07:55 +0530 Subject: [PATCH 09/16] Replace HashMap with interface Map Invert if conditions for simplification --- .../android/gnd/ui/map/MapProvider.java | 4 +-- .../gnd/ui/map/gms/GoogleMapsMapProvider.java | 28 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java b/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java index 5e4a7f0327..fad8cb8e95 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java @@ -18,7 +18,7 @@ import androidx.fragment.app.Fragment; import io.reactivex.Single; -import java.util.HashMap; +import java.util.Map; /** Common interface for various map provider libraries. */ public interface MapProvider { @@ -32,5 +32,5 @@ public interface MapProvider { void setMapType(int mapType); - HashMap getMapTypes(); + Map getMapTypes(); } diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java index f5795c9f44..d42a39eaf9 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java @@ -25,6 +25,7 @@ import io.reactivex.Single; import io.reactivex.subjects.SingleSubject; import java.util.HashMap; +import java.util.Map; /** Ground map adapter implementation for Google Maps API. */ public class GoogleMapsMapProvider implements MapProvider { @@ -73,29 +74,28 @@ public Single getMapAdapter() { @Override public int getMapType() { - if (map != null) { - return map.getValue().getMapType(); + if (map == null) { + throw new IllegalStateException("MapAdapter is null"); } - throw new IllegalStateException("MapAdapter is null"); + return map.getValue().getMapType(); } @Override public void setMapType(int mapType) { - if (map != null) { - map.getValue().setMapType(mapType); - } else { + if (map == null) { throw new IllegalStateException("MapAdapter is null"); } + map.getValue().setMapType(mapType); } @Override - public HashMap getMapTypes() { - HashMap hashMap = new HashMap<>(); - hashMap.put(GoogleMap.MAP_TYPE_NONE, "None"); - hashMap.put(GoogleMap.MAP_TYPE_NORMAL, "Normal"); - hashMap.put(GoogleMap.MAP_TYPE_SATELLITE, "Satellite"); - hashMap.put(GoogleMap.MAP_TYPE_TERRAIN, "Terrain"); - hashMap.put(GoogleMap.MAP_TYPE_HYBRID, "Hybrid"); - return hashMap; + public Map getMapTypes() { + Map map = new HashMap<>(); + map.put(GoogleMap.MAP_TYPE_NONE, "None"); + map.put(GoogleMap.MAP_TYPE_NORMAL, "Normal"); + map.put(GoogleMap.MAP_TYPE_SATELLITE, "Satellite"); + map.put(GoogleMap.MAP_TYPE_TERRAIN, "Terrain"); + map.put(GoogleMap.MAP_TYPE_HYBRID, "Hybrid"); + return map; } } From 076d3d0be28277f77a3fac2c2c58f9b9ac794cc6 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Fri, 20 Mar 2020 23:50:53 +0530 Subject: [PATCH 10/16] Replace Map with ImmutableMap --- .../java/com/google/android/gnd/ui/map/MapProvider.java | 4 ++-- .../android/gnd/ui/map/gms/GoogleMapsMapProvider.java | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java b/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java index fad8cb8e95..21d6f7e2b0 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java @@ -17,8 +17,8 @@ package com.google.android.gnd.ui.map; import androidx.fragment.app.Fragment; +import com.google.common.collect.ImmutableMap; import io.reactivex.Single; -import java.util.Map; /** Common interface for various map provider libraries. */ public interface MapProvider { @@ -32,5 +32,5 @@ public interface MapProvider { void setMapType(int mapType); - Map getMapTypes(); + ImmutableMap getMapTypes(); } diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java index d42a39eaf9..efe01dd649 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/gms/GoogleMapsMapProvider.java @@ -22,6 +22,7 @@ import com.google.android.gnd.ui.MarkerIconFactory; import com.google.android.gnd.ui.map.MapAdapter; import com.google.android.gnd.ui.map.MapProvider; +import com.google.common.collect.ImmutableMap; import io.reactivex.Single; import io.reactivex.subjects.SingleSubject; import java.util.HashMap; @@ -89,13 +90,14 @@ public void setMapType(int mapType) { } @Override - public Map getMapTypes() { + public ImmutableMap getMapTypes() { + // TODO: i18n Map map = new HashMap<>(); map.put(GoogleMap.MAP_TYPE_NONE, "None"); map.put(GoogleMap.MAP_TYPE_NORMAL, "Normal"); map.put(GoogleMap.MAP_TYPE_SATELLITE, "Satellite"); map.put(GoogleMap.MAP_TYPE_TERRAIN, "Terrain"); map.put(GoogleMap.MAP_TYPE_HYBRID, "Hybrid"); - return map; + return ImmutableMap.copyOf(map); } } From f531eb72201fcaa61c22c49fd7dd3a166e40f6e5 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Tue, 24 Mar 2020 00:00:53 +0530 Subject: [PATCH 11/16] Don't use Title casing --- gnd/src/main/res/values/strings.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gnd/src/main/res/values/strings.xml b/gnd/src/main/res/values/strings.xml index 8554d72c3f..b3eef6c4c8 100644 --- a/gnd/src/main/res/values/strings.xml +++ b/gnd/src/main/res/values/strings.xml @@ -140,6 +140,6 @@ Cancel Join project Photo preview - Select Map Type + Select map type From 4a1323dd8d46da5bcdafa132cc9cd4dd09380f2e Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Tue, 24 Mar 2020 00:53:39 +0530 Subject: [PATCH 12/16] Replace ButterKnife with DataBinding --- .../mapcontainer/MapContainerFragment.java | 10 ++++----- .../mapcontainer/MapContainerViewModel.java | 22 ++++++++++++++++--- .../main/res/layout/map_container_frag.xml | 1 + 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java index 0e3fd6c8eb..0d2c08cdbe 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java @@ -32,7 +32,6 @@ import androidx.core.view.ViewCompat; import androidx.core.view.WindowInsetsCompat; import butterknife.BindView; -import butterknife.OnClick; import com.google.android.gnd.MainViewModel; import com.google.android.gnd.R; import com.google.android.gnd.databinding.MapContainerFragBinding; @@ -74,11 +73,6 @@ public class MapContainerFragment extends AbstractFragment { private HomeScreenViewModel homeScreenViewModel; private MainViewModel mainViewModel; - @OnClick(R.id.map_type_btn) - void onClick(ImageButton view) { - showMapTypeSelectorDialog(); - } - private void showMapTypeSelectorDialog() { new AlertDialog.Builder(getContext()) .setTitle(R.string.select_map_type) @@ -142,6 +136,10 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat } else { mapProvider.restore(restoreChildFragment(savedInstanceState, MAP_FRAGMENT_KEY)); } + + mapContainerViewModel + .getMapLayerUpdateRequests() + .observe(getViewLifecycleOwner(), __ -> showMapTypeSelectorDialog()); } private void onMapReady(MapAdapter map) { diff --git a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java index 8a45b66110..95eda06929 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java @@ -16,10 +16,10 @@ package com.google.android.gnd.ui.home.mapcontainer; +import static androidx.lifecycle.LiveDataReactiveStreams.fromPublisher; import static com.google.android.gnd.util.ImmutableSetCollector.toImmutableSet; import static java8.util.stream.StreamSupport.stream; -import android.util.Log; import androidx.lifecycle.LiveData; import androidx.lifecycle.LiveDataReactiveStreams; import androidx.lifecycle.MutableLiveData; @@ -29,7 +29,9 @@ import com.google.android.gnd.repository.FeatureRepository; import com.google.android.gnd.repository.ProjectRepository; import com.google.android.gnd.rx.BooleanOrError; +import com.google.android.gnd.rx.Event; import com.google.android.gnd.rx.Loadable; +import com.google.android.gnd.rx.Nil; import com.google.android.gnd.system.LocationManager; import com.google.android.gnd.ui.common.AbstractViewModel; import com.google.android.gnd.ui.common.SharedViewModel; @@ -37,26 +39,30 @@ import com.google.common.collect.ImmutableSet; import io.reactivex.BackpressureStrategy; import io.reactivex.Flowable; +import io.reactivex.Single; +import io.reactivex.processors.PublishProcessor; import io.reactivex.subjects.PublishSubject; import io.reactivex.subjects.Subject; import java.util.concurrent.TimeUnit; import java8.util.Optional; import javax.inject.Inject; +import timber.log.Timber; @SharedViewModel public class MapContainerViewModel extends AbstractViewModel { - private static final String TAG = MapContainerViewModel.class.getSimpleName(); private static final float DEFAULT_ZOOM_LEVEL = 20.0f; private final LiveData> activeProject; private final LiveData> mapPins; private final LiveData locationLockState; private final LiveData cameraUpdateRequests; private final MutableLiveData cameraPosition; + private final LiveData> mapLayerUpdateRequests; private final LocationManager locationManager; private final FeatureRepository featureRepository; private final Subject locationLockChangeRequests; private final Subject cameraUpdateSubject; + private final PublishProcessor mapLayerTypeClicks = PublishProcessor.create(); @Inject MapContainerViewModel( @@ -88,6 +94,8 @@ public class MapContainerViewModel extends AbstractViewModel { .map(Loadable::value) .switchMap(this::getFeaturesStream) .map(MapContainerViewModel::toMapPins)); + this.mapLayerUpdateRequests = + fromPublisher(mapLayerTypeClicks.switchMapSingle(__ -> Single.just(Event.create(Nil.NIL)))); } private Flowable createCameraUpdateFlowable( @@ -131,6 +139,10 @@ private Flowable> getFeaturesStream(Optional acti .orElse(Flowable.just(ImmutableSet.of())); } + public void onMapTypeButtonClicked() { + mapLayerTypeClicks.onNext(Nil.NIL); + } + private static ImmutableSet toMapPins(ImmutableSet features) { return stream(features).map(MapContainerViewModel::toMapPin).collect(toImmutableSet()); } @@ -174,7 +186,7 @@ public void onCameraMove(Point newCameraPosition) { public void onMapDrag(Point newCameraPosition) { if (isLocationLockEnabled()) { - Log.d(TAG, "User dragged map. Disabling location lock"); + Timber.d("User dragged map. Disabling location lock"); locationLockChangeRequests.onNext(false); } } @@ -191,6 +203,10 @@ public void onLocationLockClick() { locationLockChangeRequests.onNext(!isLocationLockEnabled()); } + public LiveData> getMapLayerUpdateRequests() { + return mapLayerUpdateRequests; + } + static class CameraUpdate { private Point center; diff --git a/gnd/src/main/res/layout/map_container_frag.xml b/gnd/src/main/res/layout/map_container_frag.xml index 9a2852894c..67ba51bf9f 100644 --- a/gnd/src/main/res/layout/map_container_frag.xml +++ b/gnd/src/main/res/layout/map_container_frag.xml @@ -72,6 +72,7 @@ android:layout_width="wrap_content" android:layout_height="wrap_content" android:layout_marginTop="32dp" + android:onClick="@{() -> viewModel.onMapTypeButtonClicked()}" android:tint="@color/colorGrey500" app:backgroundTint="@color/colorBackground" app:fabSize="mini" From 8bdd8d4bb39ef7700943a300bdce5c31b3281a0f Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Tue, 24 Mar 2020 02:06:12 +0530 Subject: [PATCH 13/16] Fix pmd issues --- .../gnd/ui/home/mapcontainer/MapContainerViewModel.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java index 95eda06929..8781a7e3bf 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java @@ -16,7 +16,6 @@ package com.google.android.gnd.ui.home.mapcontainer; -import static androidx.lifecycle.LiveDataReactiveStreams.fromPublisher; import static com.google.android.gnd.util.ImmutableSetCollector.toImmutableSet; import static java8.util.stream.StreamSupport.stream; @@ -62,7 +61,7 @@ public class MapContainerViewModel extends AbstractViewModel { private final FeatureRepository featureRepository; private final Subject locationLockChangeRequests; private final Subject cameraUpdateSubject; - private final PublishProcessor mapLayerTypeClicks = PublishProcessor.create(); + private final PublishProcessor mapLayerTypeClicks; @Inject MapContainerViewModel( @@ -73,6 +72,7 @@ public class MapContainerViewModel extends AbstractViewModel { this.locationManager = locationManager; this.locationLockChangeRequests = PublishSubject.create(); this.cameraUpdateSubject = PublishSubject.create(); + this.mapLayerTypeClicks = PublishProcessor.create(); Flowable locationLockStateFlowable = createLocationLockStateFlowable().share(); this.locationLockState = @@ -95,7 +95,8 @@ public class MapContainerViewModel extends AbstractViewModel { .switchMap(this::getFeaturesStream) .map(MapContainerViewModel::toMapPins)); this.mapLayerUpdateRequests = - fromPublisher(mapLayerTypeClicks.switchMapSingle(__ -> Single.just(Event.create(Nil.NIL)))); + LiveDataReactiveStreams.fromPublisher( + mapLayerTypeClicks.switchMapSingle(__ -> Single.just(Event.create(Nil.NIL)))); } private Flowable createCameraUpdateFlowable( From 55c175284dee5381a6d73d7fd86cd47cdf7ad476 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Thu, 26 Mar 2020 22:28:46 +0530 Subject: [PATCH 14/16] Add newline --- gnd/src/main/res/drawable/map_layers.xml | 2 +- gnd/src/main/res/layout/map_container_frag.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gnd/src/main/res/drawable/map_layers.xml b/gnd/src/main/res/drawable/map_layers.xml index 16b5562c43..3e7aabd215 100644 --- a/gnd/src/main/res/drawable/map_layers.xml +++ b/gnd/src/main/res/drawable/map_layers.xml @@ -26,4 +26,4 @@ - \ No newline at end of file + diff --git a/gnd/src/main/res/layout/map_container_frag.xml b/gnd/src/main/res/layout/map_container_frag.xml index 67ba51bf9f..c85cbdfc4e 100644 --- a/gnd/src/main/res/layout/map_container_frag.xml +++ b/gnd/src/main/res/layout/map_container_frag.xml @@ -122,4 +122,4 @@ android:tint="@color/colorBackground" /> - \ No newline at end of file + From 372a3604c7ff3112380ff08a20b74ded333f16b7 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Sat, 28 Mar 2020 14:51:35 +0530 Subject: [PATCH 15/16] Add comments and TODO --- .../java/com/google/android/gnd/ui/map/MapProvider.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java b/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java index 21d6f7e2b0..3ab0530eb3 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/map/MapProvider.java @@ -20,7 +20,12 @@ import com.google.common.collect.ImmutableMap; import io.reactivex.Single; -/** Common interface for various map provider libraries. */ +/** + * Common interface for various map provider libraries. + * + *

Map Type refers to the basemap shown below map features and offline satellite imagery. It's + * called "map styles" in Mapbox and "basemaps" in Leaflet. + */ public interface MapProvider { void restore(Fragment fragment); @@ -30,6 +35,8 @@ public interface MapProvider { int getMapType(); + // TODO: Use ENUM instead of int with a superset of basemap types. + // https://github.com/google/ground-android/pull/406#discussion_r398726351 void setMapType(int mapType); ImmutableMap getMapTypes(); From a1b26df99db02f9e719f0bffa27bf0c761f329c6 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Mon, 30 Mar 2020 11:14:10 +0530 Subject: [PATCH 16/16] Convert LiveData into MutableLiveData and cleanup --- .../home/mapcontainer/MapContainerFragment.java | 2 +- .../home/mapcontainer/MapContainerViewModel.java | 15 ++++----------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java index 0d2c08cdbe..b9f2fad9a1 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerFragment.java @@ -138,7 +138,7 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat } mapContainerViewModel - .getMapLayerUpdateRequests() + .getShowMapTypeSelectorRequests() .observe(getViewLifecycleOwner(), __ -> showMapTypeSelectorDialog()); } diff --git a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java index 8781a7e3bf..049941fa8b 100644 --- a/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java +++ b/gnd/src/main/java/com/google/android/gnd/ui/home/mapcontainer/MapContainerViewModel.java @@ -38,8 +38,6 @@ import com.google.common.collect.ImmutableSet; import io.reactivex.BackpressureStrategy; import io.reactivex.Flowable; -import io.reactivex.Single; -import io.reactivex.processors.PublishProcessor; import io.reactivex.subjects.PublishSubject; import io.reactivex.subjects.Subject; import java.util.concurrent.TimeUnit; @@ -56,12 +54,11 @@ public class MapContainerViewModel extends AbstractViewModel { private final LiveData locationLockState; private final LiveData cameraUpdateRequests; private final MutableLiveData cameraPosition; - private final LiveData> mapLayerUpdateRequests; private final LocationManager locationManager; private final FeatureRepository featureRepository; private final Subject locationLockChangeRequests; private final Subject cameraUpdateSubject; - private final PublishProcessor mapLayerTypeClicks; + private final MutableLiveData> showMapTypeSelectorRequests = new MutableLiveData<>(); @Inject MapContainerViewModel( @@ -72,7 +69,6 @@ public class MapContainerViewModel extends AbstractViewModel { this.locationManager = locationManager; this.locationLockChangeRequests = PublishSubject.create(); this.cameraUpdateSubject = PublishSubject.create(); - this.mapLayerTypeClicks = PublishProcessor.create(); Flowable locationLockStateFlowable = createLocationLockStateFlowable().share(); this.locationLockState = @@ -94,9 +90,6 @@ public class MapContainerViewModel extends AbstractViewModel { .map(Loadable::value) .switchMap(this::getFeaturesStream) .map(MapContainerViewModel::toMapPins)); - this.mapLayerUpdateRequests = - LiveDataReactiveStreams.fromPublisher( - mapLayerTypeClicks.switchMapSingle(__ -> Single.just(Event.create(Nil.NIL)))); } private Flowable createCameraUpdateFlowable( @@ -141,7 +134,7 @@ private Flowable> getFeaturesStream(Optional acti } public void onMapTypeButtonClicked() { - mapLayerTypeClicks.onNext(Nil.NIL); + showMapTypeSelectorRequests.setValue(Event.create(Nil.NIL)); } private static ImmutableSet toMapPins(ImmutableSet features) { @@ -204,8 +197,8 @@ public void onLocationLockClick() { locationLockChangeRequests.onNext(!isLocationLockEnabled()); } - public LiveData> getMapLayerUpdateRequests() { - return mapLayerUpdateRequests; + LiveData> getShowMapTypeSelectorRequests() { + return showMapTypeSelectorRequests; } static class CameraUpdate {