From 7b291535e0a16e16ba1455a87cff97fc2b07af87 Mon Sep 17 00:00:00 2001 From: Ifeoluwa Andrew Omole Date: Thu, 30 Jan 2025 13:58:00 +0100 Subject: [PATCH] Feat: Make it smoother to switch between nearby and explore maps (#6164) * Nearby: Add 'Show in Explore' 3-dots menu item * MainActivity: Add methods to pass extras between Nearby and Explore * MainActivity: Extend loadFragment() to support passing fragment arguments * Nearby: Add ability to navigate to Explore fragment on 'Show in Explore' click * Explore: Read fragment arguments for Nearby map data and update Explore map if present * Explore: Add 'Show in Nearby' 3-dots menu item. Only visible when Map tab is selected * Explore: On 'Show in Nearby' click, navigate to Nearby fragment, passing map data as fragment args * Nearby: Read fragment arguments for Explore map data and update Nearby map if present * MainActivity: Fix memory leaks when navigating between bottom nav destinations * Explore: Fix crashes caused by unattached map fragment * Refactor code to pass unit tests * Explore: Format javadocs --------- Co-authored-by: Nicolas Raoul --- .gitignore | 3 +- .../commons/contributions/MainActivity.java | 64 ++++ .../nrw/commons/explore/ExploreFragment.java | 85 ++++- .../explore/ExploreMapRootFragment.java | 19 +- .../explore/map/ExploreMapFragment.java | 318 +++++++++++------- .../fragments/NearbyParentFragment.java | 132 ++++++-- .../main/res/menu/explore_fragment_menu.xml | 19 ++ .../main/res/menu/nearby_fragment_menu.xml | 6 + app/src/main/res/values/strings.xml | 2 + .../explore/ExploreFragmentUnitTest.kt | 14 +- 10 files changed, 510 insertions(+), 152 deletions(-) create mode 100644 app/src/main/res/menu/explore_fragment_menu.xml diff --git a/.gitignore b/.gitignore index e54ea2551..7fa4767a7 100644 --- a/.gitignore +++ b/.gitignore @@ -46,4 +46,5 @@ captures/* # Test and other output app/jacoco.exec -app/CommonsContributions \ No newline at end of file +app/CommonsContributions +app/.* diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java b/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java index 03027f287..047943721 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java @@ -207,6 +207,9 @@ public class MainActivity extends BaseActivity private boolean loadFragment(Fragment fragment, boolean showBottom) { //showBottom so that we do not show the bottom tray again when constructing //from the saved instance state. + + freeUpFragments(); + if (fragment instanceof ContributionsFragment) { if (activeFragment == ActiveFragment.CONTRIBUTIONS) { // scroll to top if already on the Contributions tab @@ -256,6 +259,31 @@ public class MainActivity extends BaseActivity return false; } + /** + * loadFragment() overload that supports passing extras to fragments + **/ + private boolean loadFragment(Fragment fragment, boolean showBottom, Bundle args) { + if (fragment != null && args != null) { + fragment.setArguments(args); + } + + return loadFragment(fragment, showBottom); + } + + /** + * Old implementation of loadFragment() was causing memory leaks, due to MainActivity holding + * references to cleared fragments. This function frees up all fragment references. + *

+ * Called in loadFragment() before doing the actual loading. + **/ + public void freeUpFragments() { + // free all fragments except contributionsFragment because several tests depend on it. + // hence, contributionsFragment is probably still a leak + nearbyParentFragment = null; + exploreFragment = null; + bookmarkFragment = null; + } + public void hideTabs() { binding.fragmentMainNavTabLayout.setVisibility(View.GONE); } @@ -432,6 +460,42 @@ public class MainActivity extends BaseActivity }); } + /** + * Launch the Explore fragment from Nearby fragment. This method is called when a user clicks + * the 'Show in Explore' option in the 3-dots menu in Nearby. + * + * @param zoom current zoom of Nearby map + * @param latitude current latitude of Nearby map + * @param longitude current longitude of Nearby map + **/ + public void loadExploreMapFromNearby(double zoom, double latitude, double longitude) { + Bundle bundle = new Bundle(); + bundle.putDouble("prev_zoom", zoom); + bundle.putDouble("prev_latitude", latitude); + bundle.putDouble("prev_longitude", longitude); + + loadFragment(ExploreFragment.newInstance(), false, bundle); + setSelectedItemId(NavTab.EXPLORE.code()); + } + + /** + * Launch the Nearby fragment from Explore fragment. This method is called when a user clicks + * the 'Show in Nearby' option in the 3-dots menu in Explore. + * + * @param zoom current zoom of Explore map + * @param latitude current latitude of Explore map + * @param longitude current longitude of Explore map + **/ + public void loadNearbyMapFromExplore(double zoom, double latitude, double longitude) { + Bundle bundle = new Bundle(); + bundle.putDouble("prev_zoom", zoom); + bundle.putDouble("prev_latitude", latitude); + bundle.putDouble("prev_longitude", longitude); + + loadFragment(NearbyParentFragment.newInstance(), false, bundle); + setSelectedItemId(NavTab.NEARBY.code()); + } + @Override protected void onResume() { super.onResume(); diff --git a/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java b/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java index d444148d4..223d028dc 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/ExploreFragment.java @@ -1,5 +1,7 @@ package fr.free.nrw.commons.explore; +import static androidx.viewpager.widget.ViewPager.SCROLL_STATE_IDLE; + import android.os.Bundle; import android.view.LayoutInflater; import android.view.Menu; @@ -42,9 +44,13 @@ public class ExploreFragment extends CommonsDaggerSupportFragment { @Named("default_preferences") public JsonKvStore applicationKvStore; - public void setScroll(boolean canScroll){ - if (binding != null) - { + // Nearby map state (for if we came from Nearby fragment) + private double prevZoom; + private double prevLatitude; + private double prevLongitude; + + public void setScroll(boolean canScroll) { + if (binding != null) { binding.viewPager.setCanScroll(canScroll); } } @@ -60,6 +66,7 @@ public class ExploreFragment extends CommonsDaggerSupportFragment { public View onCreateView(LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) { super.onCreate(savedInstanceState); + loadNearbyMapData(); binding = FragmentExploreBinding.inflate(inflater, container, false); viewPagerAdapter = new ViewPagerAdapter(getChildFragmentManager()); @@ -89,6 +96,11 @@ public class ExploreFragment extends CommonsDaggerSupportFragment { }); setTabs(); setHasOptionsMenu(true); + + // if we came from 'Show in Explore' in Nearby, jump to Map tab + if (isCameFromNearbyMap()) { + binding.viewPager.setCurrentItem(2); + } return binding.getRoot(); } @@ -108,6 +120,13 @@ public class ExploreFragment extends CommonsDaggerSupportFragment { Bundle mapArguments = new Bundle(); mapArguments.putString("categoryName", EXPLORE_MAP); + // if we came from 'Show in Explore' in Nearby, pass on zoom and center to Explore map root + if (isCameFromNearbyMap()) { + mapArguments.putDouble("prev_zoom", prevZoom); + mapArguments.putDouble("prev_latitude", prevLatitude); + mapArguments.putDouble("prev_longitude", prevLongitude); + } + featuredRootFragment = new ExploreListRootFragment(featuredArguments); mobileRootFragment = new ExploreListRootFragment(mobileArguments); mapRootFragment = new ExploreMapRootFragment(mapArguments); @@ -120,13 +139,35 @@ public class ExploreFragment extends CommonsDaggerSupportFragment { fragmentList.add(mapRootFragment); titleList.add(getString(R.string.explore_tab_title_map).toUpperCase(Locale.ROOT)); - ((MainActivity)getActivity()).showTabs(); + ((MainActivity) getActivity()).showTabs(); ((BaseActivity) getActivity()).getSupportActionBar().setDisplayHomeAsUpEnabled(false); viewPagerAdapter.setTabData(fragmentList, titleList); viewPagerAdapter.notifyDataSetChanged(); } + /** + * Fetch Nearby map camera data from fragment arguments if any. + */ + public void loadNearbyMapData() { + // get fragment arguments + if (getArguments() != null) { + prevZoom = getArguments().getDouble("prev_zoom"); + prevLatitude = getArguments().getDouble("prev_latitude"); + prevLongitude = getArguments().getDouble("prev_longitude"); + } + } + + /** + * Checks if fragment arguments contain data from Nearby map. if present, then the user + * navigated from Nearby using 'Show in Explore'. + * + * @return true if user navigated from Nearby map + **/ + public boolean isCameFromNearbyMap() { + return prevZoom != 0.0 || prevLatitude != 0.0 || prevLongitude != 0.0; + } + public boolean onBackPressed() { if (binding.tabLayout.getSelectedTabPosition() == 0) { if (featuredRootFragment.backPressed()) { @@ -155,7 +196,38 @@ public class ExploreFragment extends CommonsDaggerSupportFragment { */ @Override public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) { - inflater.inflate(R.menu.menu_search, menu); + // if logged in 'Show in Nearby' menu item is visible + if (applicationKvStore.getBoolean("login_skipped") == false) { + inflater.inflate(R.menu.explore_fragment_menu, menu); + + MenuItem others = menu.findItem(R.id.list_item_show_in_nearby); + + if (binding.viewPager.getCurrentItem() == 2) { + others.setVisible(true); + } + + // if on Map tab, show all menu options, else only show search + binding.viewPager.addOnPageChangeListener(new OnPageChangeListener() { + @Override + public void onPageScrolled(int position, float positionOffset, + int positionOffsetPixels) { + } + + @Override + public void onPageSelected(int position) { + others.setVisible((position == 2)); + } + + @Override + public void onPageScrollStateChanged(int state) { + if (state == SCROLL_STATE_IDLE && binding.viewPager.getCurrentItem() == 2) { + onPageSelected(2); + } + } + }); + } else { + inflater.inflate(R.menu.menu_search, menu); + } super.onCreateOptionsMenu(menu, inflater); } @@ -171,6 +243,9 @@ public class ExploreFragment extends CommonsDaggerSupportFragment { case R.id.action_search: ActivityUtils.startActivityWithFlags(getActivity(), SearchActivity.class); return true; + case R.id.list_item_show_in_nearby: + mapRootFragment.loadNearbyMapFromExplore(); + return true; default: return super.onOptionsItemSelected(item); } diff --git a/app/src/main/java/fr/free/nrw/commons/explore/ExploreMapRootFragment.java b/app/src/main/java/fr/free/nrw/commons/explore/ExploreMapRootFragment.java index 2653b4409..abf02758d 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/ExploreMapRootFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/ExploreMapRootFragment.java @@ -39,10 +39,22 @@ public class ExploreMapRootFragment extends CommonsDaggerSupportFragment impleme } public ExploreMapRootFragment(Bundle bundle) { + // get fragment arguments String title = bundle.getString("categoryName"); + double zoom = bundle.getDouble("prev_zoom"); + double latitude = bundle.getDouble("prev_latitude"); + double longitude = bundle.getDouble("prev_longitude"); + mapFragment = new ExploreMapFragment(); Bundle featuredArguments = new Bundle(); featuredArguments.putString("categoryName", title); + + // if we came from 'Show in Explore' in Nearby, pass on zoom and center + if (zoom != 0.0 || latitude != 0.0 || longitude != 0.0) { + featuredArguments.putDouble("prev_zoom", zoom); + featuredArguments.putDouble("prev_latitude", latitude); + featuredArguments.putDouble("prev_longitude", longitude); + } mapFragment.setArguments(featuredArguments); } @@ -198,7 +210,8 @@ public class ExploreMapRootFragment extends CommonsDaggerSupportFragment impleme ((MainActivity) getActivity()).showTabs(); return true; - } if (mapFragment != null && mapFragment.isVisible()) { + } + if (mapFragment != null && mapFragment.isVisible()) { if (mapFragment.backButtonClicked()) { // Explore map fragment handled the event no further action required. return true; @@ -213,6 +226,10 @@ public class ExploreMapRootFragment extends CommonsDaggerSupportFragment impleme return false; } + public void loadNearbyMapFromExplore() { + mapFragment.loadNearbyMapFromExplore(); + } + @Override public void onDestroy() { super.onDestroy(); diff --git a/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.java b/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.java index ad42d9518..e64b96190 100644 --- a/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapFragment.java @@ -38,6 +38,7 @@ import fr.free.nrw.commons.Media; import fr.free.nrw.commons.R; import fr.free.nrw.commons.Utils; import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao; +import fr.free.nrw.commons.contributions.MainActivity; import fr.free.nrw.commons.databinding.FragmentExploreMapBinding; import fr.free.nrw.commons.di.CommonsDaggerSupportFragment; import fr.free.nrw.commons.explore.ExploreMapRootFragment; @@ -115,6 +116,11 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment SystemThemeUtils systemThemeUtils; LocationPermissionsHelper locationPermissionsHelper; + // Nearby map state (if we came from Nearby) + private double prevZoom; + private double prevLatitude; + private double prevLongitude; + private ExploreMapPresenter presenter; public FragmentExploreMapBinding binding; @@ -160,6 +166,7 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment ViewGroup container, Bundle savedInstanceState ) { + loadNearbyMapData(); binding = FragmentExploreMapBinding.inflate(getLayoutInflater()); return binding.getRoot(); } @@ -169,12 +176,14 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment super.onViewCreated(view, savedInstanceState); setSearchThisAreaButtonVisibility(false); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - binding.tvAttribution.setText(Html.fromHtml(getString(R.string.map_attribution), Html.FROM_HTML_MODE_LEGACY)); + binding.tvAttribution.setText( + Html.fromHtml(getString(R.string.map_attribution), Html.FROM_HTML_MODE_LEGACY)); } else { binding.tvAttribution.setText(Html.fromHtml(getString(R.string.map_attribution))); } initNetworkBroadCastReceiver(); - locationPermissionsHelper = new LocationPermissionsHelper(getActivity(),locationManager,this); + locationPermissionsHelper = new LocationPermissionsHelper(getActivity(), locationManager, + this); if (presenter == null) { presenter = new ExploreMapPresenter(bookmarkLocationDao); } @@ -204,9 +213,14 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment scaleBarOverlay.setBackgroundPaint(barPaint); scaleBarOverlay.enableScaleBar(); binding.mapView.getOverlays().add(scaleBarOverlay); - binding.mapView.getZoomController().setVisibility(CustomZoomButtonsController.Visibility.NEVER); + binding.mapView.getZoomController() + .setVisibility(CustomZoomButtonsController.Visibility.NEVER); binding.mapView.setMultiTouchControls(true); - binding.mapView.getController().setZoom(ZOOM_LEVEL); + + if (!isCameFromNearbyMap()) { + binding.mapView.getController().setZoom(ZOOM_LEVEL); + } + performMapReadyActions(); binding.mapView.getOverlays().add(new MapEventsOverlay(new MapEventsReceiver() { @@ -295,7 +309,7 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment unregisterNetworkReceiver(); } - + /** * Unregisters the networkReceiver */ @@ -328,11 +342,51 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment isPermissionDenied = true; } lastKnownLocation = MapUtils.getDefaultLatLng(); - moveCameraToPosition( - new GeoPoint(lastKnownLocation.getLatitude(), lastKnownLocation.getLongitude())); + + // if we came from 'Show in Explore' in Nearby, load Nearby map center and zoom + if (isCameFromNearbyMap()) { + moveCameraToPosition( + new GeoPoint(prevLatitude, prevLongitude), + prevZoom, + 1L + ); + } else { + moveCameraToPosition( + new GeoPoint(lastKnownLocation.getLatitude(), lastKnownLocation.getLongitude())); + } presenter.onMapReady(exploreMapController); } + /** + * Fetch Nearby map camera data from fragment arguments if any. + */ + public void loadNearbyMapData() { + // get fragment arguments + if (getArguments() != null) { + prevZoom = getArguments().getDouble("prev_zoom"); + prevLatitude = getArguments().getDouble("prev_latitude"); + prevLongitude = getArguments().getDouble("prev_longitude"); + } + } + + /** + * Checks if fragment arguments contain data from Nearby map, indicating that the user navigated + * from Nearby using 'Show in Explore'. + * + * @return true if user navigated from Nearby map + **/ + public boolean isCameFromNearbyMap() { + return prevZoom != 0.0 || prevLatitude != 0.0 || prevLongitude != 0.0; + } + + public void loadNearbyMapFromExplore() { + ((MainActivity) getContext()).loadNearbyMapFromExplore( + binding.mapView.getZoomLevelDouble(), + binding.mapView.getMapCenter().getLatitude(), + binding.mapView.getMapCenter().getLongitude() + ); + } + private void initViews() { Timber.d("init views called"); initBottomSheets(); @@ -346,7 +400,8 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment */ @SuppressLint("ClickableViewAccessibility") private void initBottomSheets() { - bottomSheetDetailsBehavior = BottomSheetBehavior.from(binding.bottomSheetDetailsBinding.getRoot()); + bottomSheetDetailsBehavior = BottomSheetBehavior.from( + binding.bottomSheetDetailsBinding.getRoot()); bottomSheetDetailsBehavior.setState(BottomSheetBehavior.STATE_HIDDEN); binding.bottomSheetDetailsBinding.getRoot().setVisibility(View.VISIBLE); } @@ -404,7 +459,8 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment if (currentLatLng == null) { return; } - if (currentLatLng.equals(getLastMapFocus())) { // Means we are checking around current location + if (currentLatLng.equals( + getLastMapFocus())) { // Means we are checking around current location nearbyPlacesInfoObservable = presenter.loadAttractionsFromLocation(currentLatLng, getLastMapFocus(), true); } else { @@ -416,11 +472,12 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment .observeOn(AndroidSchedulers.mainThread()) .subscribe(explorePlacesInfo -> { mediaList = explorePlacesInfo.mediaList; - if(mediaList == null) { + if (mediaList == null) { showResponseMessage(getString(R.string.no_pictures_in_this_area)); } updateMapMarkers(explorePlacesInfo); - lastMapFocus = new GeoPoint(currentLatLng.getLatitude(), currentLatLng.getLongitude()); + lastMapFocus = new GeoPoint(currentLatLng.getLatitude(), + currentLatLng.getLongitude()); }, throwable -> { Timber.d(throwable); @@ -474,9 +531,9 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment locationManager.requestLocationUpdatesFromProvider(LocationManager.NETWORK_PROVIDER); locationManager.requestLocationUpdatesFromProvider(LocationManager.GPS_PROVIDER); setProgressBarVisibility(true); - } - else { - locationPermissionsHelper.showLocationOffDialog(getActivity(), R.string.ask_to_turn_location_on_text); + } else { + locationPermissionsHelper.showLocationOffDialog(getActivity(), + R.string.ask_to_turn_location_on_text); } presenter.onMapReady(exploreMapController); registerUnregisterLocationListener(false); @@ -508,7 +565,8 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment recenterToUserLocation = true; return; } - recenterMarkerToPosition(new GeoPoint(currentLatLng.getLatitude(), currentLatLng.getLongitude())); + recenterMarkerToPosition( + new GeoPoint(currentLatLng.getLatitude(), currentLatLng.getLongitude())); binding.mapView.getController() .animateTo(new GeoPoint(currentLatLng.getLatitude(), currentLatLng.getLongitude())); if (lastMapFocus != null) { @@ -549,7 +607,8 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment view -> Utils.handleGeoCoordinates(getActivity(), place.getLocation(), binding.mapView.getZoomLevelDouble())); - binding.bottomSheetDetailsBinding.commonsButton.setVisibility(place.hasCommonsLink() ? View.VISIBLE : View.GONE); + binding.bottomSheetDetailsBinding.commonsButton.setVisibility( + place.hasCommonsLink() ? View.VISIBLE : View.GONE); binding.bottomSheetDetailsBinding.commonsButton.setOnClickListener( view -> Utils.handleWebUrl(getContext(), place.siteLinks.getCommonsLink())); @@ -563,7 +622,8 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment } index++; } - binding.bottomSheetDetailsBinding.title.setText(place.name.substring(5, place.name.lastIndexOf("."))); + binding.bottomSheetDetailsBinding.title.setText( + place.name.substring(5, place.name.lastIndexOf("."))); binding.bottomSheetDetailsBinding.category.setText(place.distance); // Remove label since it is double information String descriptionText = place.getLongDescription() @@ -641,40 +701,43 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment * @param nearbyBaseMarker The NearbyBaseMarker object representing the marker to be added. */ private void addMarkerToMap(BaseMarker nearbyBaseMarker) { - ArrayList items = new ArrayList<>(); - Bitmap icon = nearbyBaseMarker.getIcon(); - Drawable d = new BitmapDrawable(getResources(), icon); - GeoPoint point = new GeoPoint( - nearbyBaseMarker.getPlace().location.getLatitude(), - nearbyBaseMarker.getPlace().location.getLongitude()); - OverlayItem item = new OverlayItem(nearbyBaseMarker.getPlace().name, null, - point); - item.setMarker(d); - items.add(item); - ItemizedOverlayWithFocus overlay = new ItemizedOverlayWithFocus(items, - new OnItemGestureListener() { - @Override - public boolean onItemSingleTapUp(int index, OverlayItem item) { - final Place place = nearbyBaseMarker.getPlace(); - if (clickedMarker != null) { - removeMarker(clickedMarker); - addMarkerToMap(clickedMarker); - bottomSheetDetailsBehavior.setState(BottomSheetBehavior.STATE_HIDDEN); - bottomSheetDetailsBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED); + if (isAttachedToActivity()) { + ArrayList items = new ArrayList<>(); + Bitmap icon = nearbyBaseMarker.getIcon(); + Drawable d = new BitmapDrawable(getResources(), icon); + GeoPoint point = new GeoPoint( + nearbyBaseMarker.getPlace().location.getLatitude(), + nearbyBaseMarker.getPlace().location.getLongitude()); + OverlayItem item = new OverlayItem(nearbyBaseMarker.getPlace().name, null, + point); + item.setMarker(d); + items.add(item); + ItemizedOverlayWithFocus overlay = new ItemizedOverlayWithFocus(items, + new OnItemGestureListener() { + @Override + public boolean onItemSingleTapUp(int index, OverlayItem item) { + final Place place = nearbyBaseMarker.getPlace(); + if (clickedMarker != null) { + removeMarker(clickedMarker); + addMarkerToMap(clickedMarker); + bottomSheetDetailsBehavior.setState(BottomSheetBehavior.STATE_HIDDEN); + bottomSheetDetailsBehavior.setState( + BottomSheetBehavior.STATE_COLLAPSED); + } + clickedMarker = nearbyBaseMarker; + passInfoToSheet(place); + return true; } - clickedMarker = nearbyBaseMarker; - passInfoToSheet(place); - return true; - } - @Override - public boolean onItemLongPress(int index, OverlayItem item) { - return false; - } - }, getContext()); + @Override + public boolean onItemLongPress(int index, OverlayItem item) { + return false; + } + }, getContext()); - overlay.setFocusItemsOnTap(true); - binding.mapView.getOverlays().add(overlay); // Add the overlay to the map + overlay.setFocusItemsOnTap(true); + binding.mapView.getOverlays().add(overlay); // Add the overlay to the map + } } /** @@ -708,68 +771,72 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment */ @Override public void clearAllMarkers() { - binding.mapView.getOverlayManager().clear(); - GeoPoint geoPoint = mapCenter; - if (geoPoint != null) { - List overlays = binding.mapView.getOverlays(); - ScaleDiskOverlay diskOverlay = - new ScaleDiskOverlay(this.getContext(), - geoPoint, 2000, GeoConstants.UnitOfMeasure.foot); - Paint circlePaint = new Paint(); - circlePaint.setColor(Color.rgb(128, 128, 128)); - circlePaint.setStyle(Paint.Style.STROKE); - circlePaint.setStrokeWidth(2f); - diskOverlay.setCirclePaint2(circlePaint); - Paint diskPaint = new Paint(); - diskPaint.setColor(Color.argb(40, 128, 128, 128)); - diskPaint.setStyle(Paint.Style.FILL_AND_STROKE); - diskOverlay.setCirclePaint1(diskPaint); - diskOverlay.setDisplaySizeMin(900); - diskOverlay.setDisplaySizeMax(1700); - binding.mapView.getOverlays().add(diskOverlay); - org.osmdroid.views.overlay.Marker startMarker = new org.osmdroid.views.overlay.Marker( - binding.mapView); - startMarker.setPosition(geoPoint); - startMarker.setAnchor(org.osmdroid.views.overlay.Marker.ANCHOR_CENTER, - org.osmdroid.views.overlay.Marker.ANCHOR_BOTTOM); - startMarker.setIcon( - ContextCompat.getDrawable(this.getContext(), R.drawable.current_location_marker)); - startMarker.setTitle("Your Location"); - startMarker.setTextLabelFontSize(24); - binding.mapView.getOverlays().add(startMarker); - } - ScaleBarOverlay scaleBarOverlay = new ScaleBarOverlay(binding.mapView); - scaleBarOverlay.setScaleBarOffset(15, 25); - Paint barPaint = new Paint(); - barPaint.setARGB(200, 255, 250, 250); - scaleBarOverlay.setBackgroundPaint(barPaint); - scaleBarOverlay.enableScaleBar(); - binding.mapView.getOverlays().add(scaleBarOverlay); - binding.mapView.getOverlays().add(new MapEventsOverlay(new MapEventsReceiver() { - @Override - public boolean singleTapConfirmedHelper(GeoPoint p) { - if (clickedMarker != null) { - removeMarker(clickedMarker); - addMarkerToMap(clickedMarker); - binding.mapView.invalidate(); - } else { - Timber.e("CLICKED MARKER IS NULL"); - } - if (bottomSheetDetailsBehavior.getState() == BottomSheetBehavior.STATE_EXPANDED) { - // Back should first hide the bottom sheet if it is expanded - bottomSheetDetailsBehavior.setState(BottomSheetBehavior.STATE_HIDDEN); - } else if (isDetailsBottomSheetVisible()) { - hideBottomDetailsSheet(); - } - return true; + if (isAttachedToActivity()) { + binding.mapView.getOverlayManager().clear(); + GeoPoint geoPoint = mapCenter; + if (geoPoint != null) { + List overlays = binding.mapView.getOverlays(); + ScaleDiskOverlay diskOverlay = + new ScaleDiskOverlay(this.getContext(), + geoPoint, 2000, GeoConstants.UnitOfMeasure.foot); + Paint circlePaint = new Paint(); + circlePaint.setColor(Color.rgb(128, 128, 128)); + circlePaint.setStyle(Paint.Style.STROKE); + circlePaint.setStrokeWidth(2f); + diskOverlay.setCirclePaint2(circlePaint); + Paint diskPaint = new Paint(); + diskPaint.setColor(Color.argb(40, 128, 128, 128)); + diskPaint.setStyle(Paint.Style.FILL_AND_STROKE); + diskOverlay.setCirclePaint1(diskPaint); + diskOverlay.setDisplaySizeMin(900); + diskOverlay.setDisplaySizeMax(1700); + binding.mapView.getOverlays().add(diskOverlay); + org.osmdroid.views.overlay.Marker startMarker = new org.osmdroid.views.overlay.Marker( + binding.mapView); + startMarker.setPosition(geoPoint); + startMarker.setAnchor(org.osmdroid.views.overlay.Marker.ANCHOR_CENTER, + org.osmdroid.views.overlay.Marker.ANCHOR_BOTTOM); + startMarker.setIcon( + ContextCompat.getDrawable(this.getContext(), + R.drawable.current_location_marker)); + startMarker.setTitle("Your Location"); + startMarker.setTextLabelFontSize(24); + binding.mapView.getOverlays().add(startMarker); } + ScaleBarOverlay scaleBarOverlay = new ScaleBarOverlay(binding.mapView); + scaleBarOverlay.setScaleBarOffset(15, 25); + Paint barPaint = new Paint(); + barPaint.setARGB(200, 255, 250, 250); + scaleBarOverlay.setBackgroundPaint(barPaint); + scaleBarOverlay.enableScaleBar(); + binding.mapView.getOverlays().add(scaleBarOverlay); + binding.mapView.getOverlays().add(new MapEventsOverlay(new MapEventsReceiver() { + @Override + public boolean singleTapConfirmedHelper(GeoPoint p) { + if (clickedMarker != null) { + removeMarker(clickedMarker); + addMarkerToMap(clickedMarker); + binding.mapView.invalidate(); + } else { + Timber.e("CLICKED MARKER IS NULL"); + } + if (bottomSheetDetailsBehavior.getState() + == BottomSheetBehavior.STATE_EXPANDED) { + // Back should first hide the bottom sheet if it is expanded + bottomSheetDetailsBehavior.setState(BottomSheetBehavior.STATE_HIDDEN); + } else if (isDetailsBottomSheetVisible()) { + hideBottomDetailsSheet(); + } + return true; + } - @Override - public boolean longPressHelper(GeoPoint p) { - return false; - } - })); - binding.mapView.setMultiTouchControls(true); + @Override + public boolean longPressHelper(GeoPoint p) { + return false; + } + })); + binding.mapView.setMultiTouchControls(true); + } } /** @@ -826,6 +893,18 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment binding.mapView.getController().animateTo(geoPoint); } + /** + * Moves the camera of the map view to the specified GeoPoint at specified zoom level and speed + * using an animation. + * + * @param geoPoint The GeoPoint representing the new camera position for the map. + * @param zoom Zoom level of the map camera + * @param speed Speed of animation + */ + private void moveCameraToPosition(GeoPoint geoPoint, double zoom, long speed) { + binding.mapView.getController().animateTo(geoPoint, zoom, speed); + } + @Override public fr.free.nrw.commons.location.LatLng getLastMapFocus() { return lastMapFocus == null ? getMapCenter() : new fr.free.nrw.commons.location.LatLng( @@ -851,14 +930,17 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment -0.07483536015053005, 1f); } } - moveCameraToPosition(new GeoPoint(latLnge.getLatitude(),latLnge.getLongitude())); + if (!isCameFromNearbyMap()) { + moveCameraToPosition(new GeoPoint(latLnge.getLatitude(), latLnge.getLongitude())); + } return latLnge; } @Override public fr.free.nrw.commons.location.LatLng getMapFocus() { fr.free.nrw.commons.location.LatLng mapFocusedLatLng = new fr.free.nrw.commons.location.LatLng( - binding.mapView.getMapCenter().getLatitude(), binding.mapView.getMapCenter().getLongitude(), 100); + binding.mapView.getMapCenter().getLatitude(), + binding.mapView.getMapCenter().getLongitude(), 100); return mapFocusedLatLng; } @@ -911,9 +993,19 @@ public class ExploreMapFragment extends CommonsDaggerSupportFragment }; } - @Override - public void onLocationPermissionDenied(String toastMessage) {} + /** + * helper function to confirm that this fragment has been attached. + **/ + public boolean isAttachedToActivity() { + boolean attached = isVisible() && getActivity() != null; + return attached; + } @Override - public void onLocationPermissionGranted() {} + public void onLocationPermissionDenied(String toastMessage) { + } + + @Override + public void onLocationPermissionGranted() { + } } 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 ee15d13ac..95f19f699 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 @@ -233,6 +233,11 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment private Place nearestPlace; private volatile boolean stopQuery; + // Explore map data (for if we came from Explore) + private double prevZoom; + private double prevLatitude; + private double prevLongitude; + private final Handler searchHandler = new Handler(); private Runnable searchRunnable; @@ -247,27 +252,28 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment private final ActivityResultLauncher galleryPickLauncherForResult = registerForActivityResult(new StartActivityForResult(), - result -> { - controller.handleActivityResultWithCallback(requireActivity(), callbacks -> { - controller.onPictureReturnedFromGallery(result, requireActivity(), callbacks); + result -> { + controller.handleActivityResultWithCallback(requireActivity(), callbacks -> { + controller.onPictureReturnedFromGallery(result, requireActivity(), callbacks); + }); }); - }); private final ActivityResultLauncher customSelectorLauncherForResult = registerForActivityResult(new StartActivityForResult(), - result -> { - controller.handleActivityResultWithCallback(requireActivity(), callbacks -> { - controller.onPictureReturnedFromCustomSelector(result, requireActivity(), callbacks); + result -> { + controller.handleActivityResultWithCallback(requireActivity(), callbacks -> { + controller.onPictureReturnedFromCustomSelector(result, requireActivity(), + callbacks); + }); }); - }); private final ActivityResultLauncher cameraPickLauncherForResult = registerForActivityResult(new StartActivityForResult(), - result -> { - controller.handleActivityResultWithCallback(requireActivity(), callbacks -> { - controller.onPictureReturnedFromCamera(result, requireActivity(), callbacks); + result -> { + controller.handleActivityResultWithCallback(requireActivity(), callbacks -> { + controller.onPictureReturnedFromCamera(result, requireActivity(), callbacks); + }); }); - }); private ActivityResultLauncher inAppCameraLocationPermissionLauncher = registerForActivityResult( new RequestMultiplePermissions(), @@ -337,12 +343,15 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment @Override public View onCreateView(@NonNull final LayoutInflater inflater, final ViewGroup container, final Bundle savedInstanceState) { + loadExploreMapData(); + binding = FragmentNearbyParentBinding.inflate(inflater, container, false); view = binding.getRoot(); initNetworkBroadCastReceiver(); scope = LifecycleOwnerKt.getLifecycleScope(getViewLifecycleOwner()); - presenter = new NearbyParentFragmentPresenter(bookmarkLocationDao, placesRepository, nearbyController); + presenter = new NearbyParentFragmentPresenter(bookmarkLocationDao, placesRepository, + nearbyController); progressDialog = new ProgressDialog(getActivity()); progressDialog.setCancelable(false); progressDialog.setMessage("Saving in progress..."); @@ -359,6 +368,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment inflater.inflate(R.menu.nearby_fragment_menu, menu); MenuItem refreshButton = menu.findItem(R.id.item_refresh); MenuItem listMenu = menu.findItem(R.id.list_sheet); + MenuItem showInExploreButton = menu.findItem(R.id.list_item_show_in_explore); MenuItem saveAsGPXButton = menu.findItem(R.id.list_item_gpx); MenuItem saveAsKMLButton = menu.findItem(R.id.list_item_kml); refreshButton.setOnMenuItemClickListener(new OnMenuItemClickListener() { @@ -379,6 +389,17 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment return false; } }); + showInExploreButton.setOnMenuItemClickListener(new OnMenuItemClickListener() { + @Override + public boolean onMenuItemClick(@NonNull MenuItem item) { + ((MainActivity) getContext()).loadExploreMapFromNearby( + binding.map.getZoomLevelDouble(), + binding.map.getMapCenter().getLatitude(), + binding.map.getMapCenter().getLongitude() + ); + return false; + } + }); saveAsGPXButton.setOnMenuItemClickListener(new OnMenuItemClickListener() { @Override @@ -467,6 +488,14 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment binding.map.getOverlays().add(scaleBarOverlay); binding.map.getZoomController().setVisibility(Visibility.NEVER); binding.map.getController().setZoom(ZOOM_LEVEL); + // if we came from Explore map using 'Show in Nearby', load Explore map camera position + if (isCameFromExploreMap()) { + moveCameraToPosition( + new GeoPoint(prevLatitude, prevLongitude), + prevZoom, + 1L + ); + } binding.map.getOverlays().add(mapEventsOverlay); binding.map.addMapListener(new MapListener() { @@ -489,11 +518,14 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment } initNearbyFilter(); addCheckBoxCallback(); - moveCameraToPosition(lastMapFocus); + if (!isCameFromExploreMap()) { + moveCameraToPosition(lastMapFocus); + } initRvNearbyList(); onResume(); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - binding.tvAttribution.setText(Html.fromHtml(getString(R.string.map_attribution), Html.FROM_HTML_MODE_LEGACY)); + binding.tvAttribution.setText( + Html.fromHtml(getString(R.string.map_attribution), Html.FROM_HTML_MODE_LEGACY)); } else { //noinspection deprecation binding.tvAttribution.setText(Html.fromHtml(getString(R.string.map_attribution))); @@ -545,6 +577,28 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment } } + /** + * Fetch Explore map camera data from fragment arguments if any. + */ + public void loadExploreMapData() { + // get fragment arguments + if (getArguments() != null) { + prevZoom = getArguments().getDouble("prev_zoom"); + prevLatitude = getArguments().getDouble("prev_latitude"); + prevLongitude = getArguments().getDouble("prev_longitude"); + } + } + + /** + * Checks if fragment arguments contain data from Explore map. if present, then the user + * navigated from Explore using 'Show in Nearby'. + * + * @return true if user navigated from Explore map + **/ + public boolean isCameFromExploreMap() { + return prevZoom != 0.0 || prevLatitude != 0.0 || prevLongitude != 0.0; + } + /** * Initialise background based on theme, this should be doe ideally via styles, that would need * another refactor @@ -625,7 +679,9 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment mapCenter = targetP; binding.map.getController().setCenter(targetP); recenterMarkerToPosition(targetP); - moveCameraToPosition(targetP); + if (!isCameFromExploreMap()) { + moveCameraToPosition(targetP); + } } else if (locationManager.isGPSProviderEnabled() || locationManager.isNetworkProviderEnabled()) { locationManager.requestLocationUpdatesFromProvider(LocationManager.NETWORK_PROVIDER); @@ -669,7 +725,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment } else { lastKnownLocation = MapUtils.getDefaultLatLng(); } - if (binding.map != null) { + if (binding.map != null && !isCameFromExploreMap()) { moveCameraToPosition( new GeoPoint(lastKnownLocation.getLatitude(), lastKnownLocation.getLongitude())); } @@ -739,8 +795,8 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment } /** - * Determines the number of spans (columns) in the RecyclerView based on device orientation - * and adapter item count. + * Determines the number of spans (columns) in the RecyclerView based on device orientation and + * adapter item count. * * @return The number of spans to be used in the RecyclerView. */ @@ -1175,7 +1231,6 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment /** * Clears the Nearby local cache and then calls for pin details to be fetched afresh. - * */ private void emptyCache() { // reload the map once the cache is cleared @@ -1338,7 +1393,8 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment } /** - * Fetches and updates the data for a specific place, then updates the corresponding marker on the map. + * Fetches and updates the data for a specific place, then updates the corresponding marker on + * the map. * * @param entity The entity ID of the place. * @param place The Place object containing the initial place data. @@ -1469,9 +1525,8 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment } /** - * Stops any ongoing queries and clears all disposables. - * This method sets the stopQuery flag to true and clears the compositeDisposable - * to prevent any further processing. + * Stops any ongoing queries and clears all disposables. This method sets the stopQuery flag to + * true and clears the compositeDisposable to prevent any further processing. */ @Override public void stopQuery() { @@ -1624,7 +1679,8 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment new Builder(getContext()) .setMessage(R.string.login_alert_message) .setCancelable(false) - .setNegativeButton(R.string.cancel, (dialog, which) -> {}) + .setNegativeButton(R.string.cancel, (dialog, which) -> { + }) .setPositiveButton(R.string.login, (dialog, which) -> { // logout of the app BaseLogoutListener logoutListener = new BaseLogoutListener(getActivity()); @@ -1743,7 +1799,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment final boolean filterForPlaceState, final boolean filterForAllNoneType) { final boolean displayExists = false; - final boolean displayNeedsPhoto= false; + final boolean displayNeedsPhoto = false; final boolean displayWlm = false; if (selectedLabels == null || selectedLabels.size() == 0) { replaceMarkerOverlays(NearbyController.markerLabelList); @@ -1903,8 +1959,8 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment /** * Adds multiple markers representing places to the map and handles item gestures. * - * @param markerPlaceGroups The list of marker place groups containing the places and - * their bookmarked status + * @param markerPlaceGroups The list of marker place groups containing the places and their + * bookmarked status */ @Override public void replaceMarkerOverlays(final List markerPlaceGroups) { @@ -1913,7 +1969,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment for (int i = markerPlaceGroups.size() - 1; i >= 0; i--) { newMarkers.add( convertToMarker(markerPlaceGroups.get(i).getPlace(), - markerPlaceGroups.get(i).getIsBookmarked()) + markerPlaceGroups.get(i).getIsBookmarked()) ); } clearAllMarkers(); @@ -2103,7 +2159,8 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment if (binding.fabCamera.isShown()) { Timber.d("Camera button tapped. Place: %s", selectedPlace.toString()); storeSharedPrefs(selectedPlace); - controller.initiateCameraPick(getActivity(), inAppCameraLocationPermissionLauncher, cameraPickLauncherForResult); + controller.initiateCameraPick(getActivity(), inAppCameraLocationPermissionLauncher, + cameraPickLauncherForResult); } }); @@ -2121,7 +2178,8 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment if (binding.fabCustomGallery.isShown()) { Timber.d("Gallery button tapped. Place: %s", selectedPlace.toString()); storeSharedPrefs(selectedPlace); - controller.initiateCustomGalleryPickWithPermission(getActivity(), customSelectorLauncherForResult); + controller.initiateCustomGalleryPickWithPermission(getActivity(), + customSelectorLauncherForResult); } }); } @@ -2296,6 +2354,18 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment binding.map.getController().animateTo(geoPoint); } + /** + * Moves the camera of the map view to the specified GeoPoint at specified zoom level and speed + * using an animation. + * + * @param geoPoint The GeoPoint representing the new camera position for the map. + * @param zoom Zoom level of the map camera + * @param speed Speed of animation + */ + private void moveCameraToPosition(GeoPoint geoPoint, double zoom, long speed) { + binding.map.getController().animateTo(geoPoint, zoom, speed); + } + @Override public void onBottomSheetItemClick(@Nullable View view, int position) { BottomSheetItem item = dataList.get(position); diff --git a/app/src/main/res/menu/explore_fragment_menu.xml b/app/src/main/res/menu/explore_fragment_menu.xml new file mode 100644 index 000000000..4dce1c57c --- /dev/null +++ b/app/src/main/res/menu/explore_fragment_menu.xml @@ -0,0 +1,19 @@ + +

+ + + + \ No newline at end of file diff --git a/app/src/main/res/menu/nearby_fragment_menu.xml b/app/src/main/res/menu/nearby_fragment_menu.xml index fe049cde4..e7c23ed89 100644 --- a/app/src/main/res/menu/nearby_fragment_menu.xml +++ b/app/src/main/res/menu/nearby_fragment_menu.xml @@ -12,6 +12,12 @@ android:icon="@drawable/ic_list_white_24dp" /> + + Caption copied to clipboard Congratulations, all pictures in this album have been either uploaded or marked as not for upload. + Show in Explore + Show in Nearby diff --git a/app/src/test/kotlin/fr/free/nrw/commons/explore/ExploreFragmentUnitTest.kt b/app/src/test/kotlin/fr/free/nrw/commons/explore/ExploreFragmentUnitTest.kt index 41c999791..e2ef3bb92 100644 --- a/app/src/test/kotlin/fr/free/nrw/commons/explore/ExploreFragmentUnitTest.kt +++ b/app/src/test/kotlin/fr/free/nrw/commons/explore/ExploreFragmentUnitTest.kt @@ -11,16 +11,19 @@ import androidx.fragment.app.FragmentManager import androidx.fragment.app.FragmentTransaction import androidx.test.core.app.ApplicationProvider import com.google.android.material.tabs.TabLayout +import com.nhaarman.mockitokotlin2.eq import fr.free.nrw.commons.OkHttpConnectionFactory import fr.free.nrw.commons.R import fr.free.nrw.commons.TestCommonsApplication import fr.free.nrw.commons.contributions.MainActivity import fr.free.nrw.commons.createTestClient import org.junit.Assert +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Ignore import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor import org.mockito.Mock import org.mockito.Mockito.verify import org.mockito.Mockito.`when` @@ -34,6 +37,7 @@ import org.robolectric.annotation.LooperMode import org.robolectric.fakes.RoboMenu import org.robolectric.fakes.RoboMenuItem + @RunWith(RobolectricTestRunner::class) @Config(sdk = [21], application = TestCommonsApplication::class) @LooperMode(LooperMode.Mode.PAUSED) @@ -151,6 +155,14 @@ class ExploreFragmentUnitTest { Shadows.shadowOf(getMainLooper()).idle() val menu: Menu = RoboMenu(context) fragment.onCreateOptionsMenu(menu, inflater) - verify(inflater).inflate(R.menu.menu_search, menu) + + val captor = ArgumentCaptor.forClass( + Int::class.java + ) + verify(inflater).inflate(captor.capture(), eq(menu)) + + val capturedLayout = captor.value + assertTrue(capturedLayout == R.menu.menu_search || capturedLayout == R.menu.explore_fragment_menu) + } }