From 22dd69cabb2ed10a498d6dfbbb71ed5342be60e7 Mon Sep 17 00:00:00 2001 From: Evangelos Talos <115378448+vtalos@users.noreply.github.com> Date: Sun, 9 Jun 2024 15:07:02 -0400 Subject: [PATCH] Improved Map Marker Visibility Based on App Theme (#5744) * Update map markers * Modify code to use different map marker for themes * Update map markers for bookmarked * Add 2 tests --------- Co-authored-by: Giannis Karyotakis <110292528+karyotakisg@users.noreply.github.com> --- .../explore/map/ExploreMapController.java | 2 +- .../fragments/NearbyParentFragment.java | 34 ++++++++++---- .../res/drawable/ic_custom_map_marker.xml | 45 +++++++++--------- .../drawable/ic_custom_map_marker_blue.xml | 29 ++++++++++++ ...ustom_map_marker_blue_bookmarked_dark.xml} | 2 +- .../drawable/ic_custom_map_marker_dark.xml | 23 ++++++++++ .../drawable/ic_custom_map_marker_green.xml | 43 ++++++++--------- .../ic_custom_map_marker_green_bookmarked.xml | 46 +++++++++---------- ...ustom_map_marker_green_bookmarked_dark.xml | 29 ++++++++++++ .../ic_custom_map_marker_green_dark.xml | 23 ++++++++++ app/src/main/res/layout/dialog_nearby.xml | 4 +- .../main/res/layout/fragment_achievements.xml | 2 +- .../nearby/NearbyParentFragmentUnitTest.kt | 24 ++++++++++ 13 files changed, 225 insertions(+), 81 deletions(-) create mode 100644 app/src/main/res/drawable/ic_custom_map_marker_blue.xml rename app/src/main/res/drawable/{ic_custom_map_marker_blue_bookmarked.xml => ic_custom_map_marker_blue_bookmarked_dark.xml} (98%) create mode 100644 app/src/main/res/drawable/ic_custom_map_marker_dark.xml create mode 100644 app/src/main/res/drawable/ic_custom_map_marker_green_bookmarked_dark.xml create mode 100644 app/src/main/res/drawable/ic_custom_map_marker_green_dark.xml diff --git a/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapController.java b/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapController.java index 99c755814..c944f75a1 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapController.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapController.java @@ -143,7 +143,7 @@ public class ExploreMapController extends MapController { VectorDrawableCompat vectorDrawable = null; try { vectorDrawable = VectorDrawableCompat.create( - context.getResources(), R.drawable.ic_custom_map_marker, context.getTheme()); + context.getResources(), R.drawable.ic_custom_map_marker_dark, context.getTheme()); } catch (Resources.NotFoundException e) { // ignore when running tests. diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java index d3f555b4d..877f22f5a 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java @@ -68,7 +68,6 @@ import fr.free.nrw.commons.di.CommonsDaggerSupportFragment; import fr.free.nrw.commons.kvstore.JsonKvStore; import fr.free.nrw.commons.location.LocationPermissionsHelper; import fr.free.nrw.commons.location.LocationPermissionsHelper.LocationPermissionCallback; -import fr.free.nrw.commons.location.LatLng; import fr.free.nrw.commons.location.LocationServiceManager; import fr.free.nrw.commons.location.LocationUpdateListener; import fr.free.nrw.commons.nearby.CheckBoxTriStates; @@ -1731,9 +1730,10 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment * * @param place where marker is to be added * @param isBookmarked true if place is bookmarked + * @param isDarkTheme true if app uses dark theme * @return returns the drawable of marker according to the place information */ - private @DrawableRes int getIconFor(Place place, Boolean isBookmarked) { + private @DrawableRes int getIconFor(Place place, Boolean isBookmarked , Boolean isDarkTheme) { if (nearestPlace != null) { if (place.name.equals(nearestPlace.name)) { // Highlight nearest place only when user clicks on the home nearby banner @@ -1746,17 +1746,31 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment if (place.isMonument()) { return R.drawable.ic_custom_map_marker_monuments; } else if (!place.pic.trim().isEmpty()) { - return (isBookmarked ? - R.drawable.ic_custom_map_marker_green_bookmarked : - R.drawable.ic_custom_map_marker_green); + if (isDarkTheme) { //different icon for light and dark theme + return (isBookmarked ? + R.drawable.ic_custom_map_marker_green_bookmarked_dark : + R.drawable.ic_custom_map_marker_green_dark); + } + else{ + return (isBookmarked ? + R.drawable.ic_custom_map_marker_green_bookmarked_dark : + R.drawable.ic_custom_map_marker_green); + } } else if (!place.exists) { // Means that the topic of the Wikidata item does not exist in the real world anymore, for instance it is a past event, or a place that was destroyed return (isBookmarked ? R.drawable.ic_custom_map_marker_grey_bookmarked : R.drawable.ic_custom_map_marker_grey); } else { - return (isBookmarked ? - R.drawable.ic_custom_map_marker_blue_bookmarked : - R.drawable.ic_custom_map_marker); + if (isDarkTheme) { //different icon for light and dark theme + return (isBookmarked ? + R.drawable.ic_custom_map_marker_blue_bookmarked_dark : + R.drawable.ic_custom_map_marker_dark); + } + else{ + return (isBookmarked ? + R.drawable.ic_custom_map_marker_blue_bookmarked_dark : + R.drawable.ic_custom_map_marker); + } } } @@ -1768,7 +1782,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment */ private void addMarkerToMap(Place place, Boolean isBookMarked) { ArrayList items = new ArrayList<>(); - Drawable icon = ContextCompat.getDrawable(getContext(), getIconFor(place, isBookMarked)); + Drawable icon = ContextCompat.getDrawable(getContext(), getIconFor(place, isBookMarked, isDarkTheme)); GeoPoint point = new GeoPoint(place.location.getLatitude(), place.location.getLongitude()); OverlayItem item = new OverlayItem(place.name, containsParentheses(place.getLongDescription()) ? getTextBetweenParentheses( @@ -1811,7 +1825,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment ArrayList items = new ArrayList<>(); for (int i = 0; i < nearbyBaseMarkers.size(); i++) { Drawable icon = ContextCompat.getDrawable(getContext(), - getIconFor(nearbyBaseMarkers.get(i).getPlace(), false)); + getIconFor(nearbyBaseMarkers.get(i).getPlace(), false, isDarkTheme)); GeoPoint point = new GeoPoint( nearbyBaseMarkers.get(i).getPlace().location.getLatitude(), nearbyBaseMarkers.get(i).getPlace().location.getLongitude()); diff --git a/app/src/main/res/drawable/ic_custom_map_marker.xml b/app/src/main/res/drawable/ic_custom_map_marker.xml index 58d4af211..9b2966e98 100644 --- a/app/src/main/res/drawable/ic_custom_map_marker.xml +++ b/app/src/main/res/drawable/ic_custom_map_marker.xml @@ -1,23 +1,24 @@ + - - - - + android:width="@dimen/half_standard_height" + android:height="28dp" + android:viewportWidth="24.0" + android:viewportHeight="28.0"> + + + + \ No newline at end of file diff --git a/app/src/main/res/drawable/ic_custom_map_marker_blue.xml b/app/src/main/res/drawable/ic_custom_map_marker_blue.xml new file mode 100644 index 000000000..c4290a922 --- /dev/null +++ b/app/src/main/res/drawable/ic_custom_map_marker_blue.xml @@ -0,0 +1,29 @@ + + + + + + \ No newline at end of file diff --git a/app/src/main/res/drawable/ic_custom_map_marker_blue_bookmarked.xml b/app/src/main/res/drawable/ic_custom_map_marker_blue_bookmarked_dark.xml similarity index 98% rename from app/src/main/res/drawable/ic_custom_map_marker_blue_bookmarked.xml rename to app/src/main/res/drawable/ic_custom_map_marker_blue_bookmarked_dark.xml index f24ad7f33..0237d6049 100644 --- a/app/src/main/res/drawable/ic_custom_map_marker_blue_bookmarked.xml +++ b/app/src/main/res/drawable/ic_custom_map_marker_blue_bookmarked_dark.xml @@ -16,7 +16,7 @@ + + + + diff --git a/app/src/main/res/drawable/ic_custom_map_marker_green.xml b/app/src/main/res/drawable/ic_custom_map_marker_green.xml index 6c7eb95fe..8cf28626c 100644 --- a/app/src/main/res/drawable/ic_custom_map_marker_green.xml +++ b/app/src/main/res/drawable/ic_custom_map_marker_green.xml @@ -1,23 +1,24 @@ + - - - + android:width="@dimen/half_standard_height" + android:height="28dp" + android:viewportWidth="24.0" + android:viewportHeight="28.0"> + + + \ No newline at end of file diff --git a/app/src/main/res/drawable/ic_custom_map_marker_green_bookmarked.xml b/app/src/main/res/drawable/ic_custom_map_marker_green_bookmarked.xml index 545e140c8..7b28ebb1a 100644 --- a/app/src/main/res/drawable/ic_custom_map_marker_green_bookmarked.xml +++ b/app/src/main/res/drawable/ic_custom_map_marker_green_bookmarked.xml @@ -1,29 +1,29 @@ + android:width="24dp" + android:height="28dp" + android:viewportWidth="24" + android:viewportHeight="28"> + android:pathData="M6.072,22.223a6.031,3.672 0,1 0,12.062 0a6.031,3.672 0,1 0,-12.062 0z" + android:strokeAlpha="0.1" + android:strokeWidth="1" + android:fillColor="#000000" + android:fillAlpha="0.1"/> + android:pathData="M11.575,11.62C10.689,11.462 9.902,10.759 9.625,9.878 9.553,9.65 9.535,9.499 9.538,9.14c0.004,-0.397 0.019,-0.492 0.13,-0.787 0.236,-0.631 0.646,-1.099 1.212,-1.382 0.386,-0.193 0.709,-0.272 1.116,-0.272 0.676,0 1.263,0.247 1.744,0.734 0.355,0.359 0.541,0.682 0.657,1.136 0.327,1.278 -0.442,2.611 -1.723,2.987 -0.282,0.083 -0.817,0.114 -1.099,0.063z" + android:strokeWidth="1" + android:fillColor="#1F7123"/> + android:pathData="M11.617,21.707C10.518,20.424 9.338,18.864 8.395,17.449 6.524,14.641 5.455,12.305 5.102,10.255 5.014,9.744 5.006,8.628 5.088,8.137 5.348,6.561 6.043,5.221 7.158,4.148 9.148,2.231 12.016,1.668 14.593,2.688c2.043,0.809 3.607,2.581 4.162,4.719 0.174,0.67 0.204,0.933 0.203,1.761 -0.001,0.81 -0.035,1.098 -0.22,1.857 -0.614,2.524 -2.571,5.977 -5.383,9.501 -0.645,0.809 -1.321,1.61 -1.358,1.61 -0.008,0 -0.179,-0.193 -0.381,-0.428zM12.617,11.603c0.783,-0.188 1.457,-0.795 1.738,-1.564 0.516,-1.415 -0.317,-2.962 -1.783,-3.312 -0.216,-0.052 -0.317,-0.059 -0.661,-0.047 -0.354,0.012 -0.441,0.025 -0.682,0.104 -0.673,0.221 -1.205,0.695 -1.506,1.344 -0.176,0.38 -0.218,0.584 -0.217,1.054 0.001,0.324 0.014,0.452 0.064,0.635 0.266,0.97 1.077,1.689 2.079,1.844 0.243,0.038 0.68,0.012 0.968,-0.057z" + android:strokeWidth="1" + android:fillColor="#1F7123" + android:strokeColor="#003b59" + android:fillAlpha="1"/> + android:pathData="M17.9025,7.0798 L14.1612,6.7552 12.7003,3.3154c-0.2628,-0.6261 -1.1595,-0.6261 -1.4223,0L9.817,6.7629 6.0835,7.0798C5.4032,7.134 5.125,7.9842 5.6429,8.4326l2.8369,2.4581 -0.8503,3.6485c-0.1546,0.6648 0.5643,1.1904 1.1518,0.8348l3.2079,-1.9325 3.2079,1.9402c0.5875,0.3556 1.3064,-0.1701 1.1518,-0.8348L15.4985,10.8907 18.3354,8.4326C18.8533,7.9842 18.5827,7.134 17.9025,7.0798Z" + android:strokeAlpha="1" + android:strokeWidth="1" + android:fillColor="#f84d4d" + android:fillAlpha="1" + android:strokeColor="#003b59"/> diff --git a/app/src/main/res/drawable/ic_custom_map_marker_green_bookmarked_dark.xml b/app/src/main/res/drawable/ic_custom_map_marker_green_bookmarked_dark.xml new file mode 100644 index 000000000..545e140c8 --- /dev/null +++ b/app/src/main/res/drawable/ic_custom_map_marker_green_bookmarked_dark.xml @@ -0,0 +1,29 @@ + + + + + + diff --git a/app/src/main/res/drawable/ic_custom_map_marker_green_dark.xml b/app/src/main/res/drawable/ic_custom_map_marker_green_dark.xml new file mode 100644 index 000000000..6c7eb95fe --- /dev/null +++ b/app/src/main/res/drawable/ic_custom_map_marker_green_dark.xml @@ -0,0 +1,23 @@ + + + + + \ No newline at end of file diff --git a/app/src/main/res/layout/dialog_nearby.xml b/app/src/main/res/layout/dialog_nearby.xml index 46a35d477..fb322b30f 100644 --- a/app/src/main/res/layout/dialog_nearby.xml +++ b/app/src/main/res/layout/dialog_nearby.xml @@ -25,7 +25,7 @@ android:layout_height="wrap_content" android:layout_weight="0" android:scaleType="fitCenter" - app:srcCompat="@drawable/ic_custom_map_marker" /> + app:srcCompat="@drawable/ic_custom_map_marker_dark" /> + app:srcCompat="@drawable/ic_custom_map_marker_green_dark" /> + app:srcCompat="@drawable/ic_custom_map_marker_dark" /> (fragment, "getIconFor", place, true, false) + Assert.assertEquals(R.drawable.ic_custom_map_marker_blue_bookmarked_dark, icon) + } + + @Test @Ignore + @Throws(Exception::class) + fun `test getIconFor non-bookmarked monument place`() { + val place = mock(Place::class.java).apply { + `when`(isMonument()).thenReturn(true) + } + + val icon = Whitebox.invokeMethod(fragment, "getIconFor", place, false, false) + Assert.assertEquals(R.drawable.ic_custom_map_marker_monuments, icon) + } + @Test @Ignore @Throws(Exception::class) fun testOnToggleChipsClickedCaseVisible() {