Refactor nearby classes mvp (commons-app#2969)

* Create parent contract

* Create map child contract and fill methods

* Add javadocs and specific interfaces for list

* Move general method to parent and add javadocs for parent

* Add explanation for keeping an emty View interface under NearbyListContract

* Move constracts under contract package

* Create presenters for map and list and implement user actions accordingly

* Add javadocs

* Add presenter, contract and fragment for parent Fragment of both NearyListFragment and NearbyMapFragment

* Implement missing methods

* Fix typo

* Add main views on fragment

* İmplement child fragment logic and their retain

* Relate parent presenter with parent fragment

* Add all location permission related methods to view contract and implement in fragment. Call them from presenter by passing locationServiceManager parameter

* Define refreshView method as updateMapAndList which is a better naming. Define it at presenter part.

* Define a presenter variable in fragment and call updateMapAndList method from there, if permissions are okay

* Add lock neabry method to unlisten nearby operations during updates

* Add network connection established check on view side, check it from presenter

* try to simplify previous method during refactor

* Add missing methods for NearbyMap

* Connect child fragment and prent fragment with presenter

* Change nearby design, first create views then register listeners

* AddnetworkBroadcatsReceiver on view side, call it from presenter

* Add comments

* Change the old NearbyFragment by our new NearbyParentFragment for first tests

* Prevent crash caused child fragment is actually null by checking if it is attached or not.

* Makes sure that initialize nearby operations method is called just after all views and fragments are ready and attached

* Make sure updateMAoAndList method is called when everything ready

* Rename a method with prepoer name

* Call update map and add required markers

* Find out zoom level problem

* Implement add nearby markers and add current marker methods

* remove unneeded codes copied from previous implementation

* Revert "remove unneeded codes copied from previous implementation"

This reverts commit 4253965.

* add some commits and clear code

* Remove location listener implementation from view, handle them in presenter instead. And add timber debug notes and required method calls

* Style ,issues

* Refactor a variable name to camel case and bind search this area view

* Search this area button action is added

* Mostly implement search this area methods, not tested yet even once

* Make sure everything is called in required order and seach this area method basically works

* Rename methods accordingly and add Javadocs

* Add current location marker and remove circle around it

* Remove unused methods

* Add current location marker with object animator and remove previous marker is exists

* include clear map into add markers method and reorder methods

* Make search this area button appear at correct time

* Try to load un search this area is called

* Clear logs

* Add changes for permission made by Vivek to newly added fragments along with our nearby classes

* Add a view to nearby map ragment and insert map view as an item inside it

* Add logs to uınderstand why on meap ready callback is never called

* Add list item clicked and bottom sheet for list of nearby items is expanded

* Make nearby map ready callback came

* Add required methods to be called after map view is ready

* State: Map ready call is not called but permissions and methods are in correct order

* Remove unused logs

* Try to use SupportMapFragment instead, still no success...

* use SupportMapFragment instead

* Remove unused Near

* Upgrade mapbox sdk versions

* Remove Style import from fragment/NearbyMapFragment

* Remove Style import from **fragments/NearbyParentFragment

* Remove nearby/NearbyMapFragment

* Remove unused/old NearbyFragment and NearbyMapFragment

* Remove import of already removed class

* Make sure you removed everything related with mapbox implementation

* Update mapbox map

* Remove unused classes, do not forget centerMapToPlace doesn't work on this branch

* Remove Style class it required updated version of mapbox map

* Add base codes for new activity

* Prove that our mapbox sdk let us implement the map directly inside the activity

* Add base codes for testing mapbox activity in a single fragment inside activity

* Add codes from mapbox demo repository to test support fragment, map works in a single layer fragment

* Add base codes for test layered fragment activity

* Add Support fragment inside a fragment and proves that layered fragmentw with map works

* Test view pager and tab layout with support map fragment to see it works, it works!

* Move Contributions Fragment related codes to test activity

* Move nearby card methods

* Inject location manager and implement NerabyParentFragmentContract.View on test fragment

* Coppy the content of SupportMapFragment from mapbox repository, use this code to modify later. This method war suggested by mapbox team instead of extending the class

* Implement NearbyMapContract.View on our new SupportMapFragment

* Start to mplement logic of checking permissions

* Fix small dagger issue to inject location manager properly

* Request permission for nearby places if fragment is loaded and tab is selected

* Initialize map operations if map ready and tab is selected

* Markers loads at correct time

* Style the map according to new version of mapbox map

* Add some map elements like FABs and give their actions

* Implement map marker click actions

* Implement nearby Fabs logic with a small issue

* fix FABs are not closing problem

* Unkown problem occurs at map load when I try to use MainActivity again.

* Revert "Unkown problem occurs at map load when I try to use MainActivity again."

This reverts commit 3dc0844.

* Search this area buttons are added but button function does not work and button visibility is problematic

* Fix issue with MainActivity with the help of Ashish

* Fix search this area button visibility issue

* Fix the issues with updating search nearby markers and camera position together

* Fix progress bar visibility issue

* Prevent loding map each time tab selected

* Take toolbar back

* Implement back button with presenter

* Add click actoion to bottom sheet details

* Add nearby list into bottom sheeet

* Make reuse existing fragments if there is any

* Code cleanup

* Cleanup

* Code cleanup

* Add lifecyle codes to prevent leaks

* Cleanup

* Code cleanup

* cleanup

* Make list item clicked and map focus to same place

* Add bookmark from list fragment

* Make nearby card click action work

* Revert "Fix conflicts"

This reverts commit f345174, reversing
changes made to c5d4d55.

* Code cleanup

* Cleanup

* Make recenter button work when list sheet is expanded

* Cleanup

* Code cleanups

* NPE issue is not detected for now, can be solved on seperate PR

* Update map after Wikidata edit

* Cherry picked previously reverted merge, hoping this will fix enourmus amounts of diff files

* Previous merge bringed an file which should be deleted. Delete it.

* Revert irrelevant changes on build.gradle

* Revert irrelevant changes

* Jetbrains annotation is not included to build gradle, this issue caused build issues on my branch so I removed it to be able to build

* Rename ListView

* Use a singleton to access the presenter, instead of passing it to a variable inside fragment

* Fix code style issues

* Move hardcoded colors to colors.xml file

* Make larger methods smaller

* Make current location marker follow

* Do not track users position if user is searching around

* Revert irrelevant shanges at build gradle

* Remove unneeded variable

* Remove mvp directory, add sub directories directly under nearby directory instead

* Remove unneeded public definiton

* Remove unneeded namespace

* Add public defiiton, it is needed to reach the constructor.

Author: neslihanturan

app/build.gradle

@@ -34,7 +34,8 @@ dependencies {
     implementation 'fr.avianey.com.viewpagerindicator:library:2.4.1.1@aar'
     implementation 'com.github.chrisbanes:PhotoView:2.0.0'
     implementation 'com.github.pedrovgs:renderers:3.3.3'
-    implementation 'com.mapbox.mapboxsdk:mapbox-android-plugin-localization:0.6.0'
+    implementation 'com.mapbox.mapboxsdk:mapbox-android-sdk:7.2.0'
+    implementation 'com.mapbox.mapboxsdk:mapbox-android-plugin-localization-v7:0.7.0'
     implementation 'com.github.deano2390:MaterialShowcaseView:1.2.0'
     implementation 'com.dinuscxj:circleprogressbar:1.1.1'
     implementation 'com.karumi:dexter:5.0.0'
@@ -159,7 +160,7 @@ android {
             versionNameSuffix "-debug-" + getBranchName()
         }
     }
-	
+
     if (isRunningOnTravisAndIsNotPRBuild) {
         // configure keystore based on env vars in Travis for automated alpha builds
         signingConfigs.release.storeFile = file("../nr-commons.keystore")

app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java

@@ -31,8 +31,9 @@
 import fr.free.nrw.commons.auth.AuthenticatedActivity;
 import fr.free.nrw.commons.auth.SessionManager;
 import fr.free.nrw.commons.location.LocationServiceManager;
-import fr.free.nrw.commons.nearby.NearbyFragment;
 import fr.free.nrw.commons.nearby.NearbyNotificationCardView;
+import fr.free.nrw.commons.nearby.fragments.NearbyParentFragment;
+import fr.free.nrw.commons.nearby.presenter.NearbyParentFragmentPresenter;
 import fr.free.nrw.commons.notification.Notification;
 import fr.free.nrw.commons.notification.NotificationActivity;
 import fr.free.nrw.commons.notification.NotificationController;
@@ -47,13 +48,15 @@
 
 public class MainActivity extends AuthenticatedActivity implements FragmentManager.OnBackStackChangedListener {
 
-    @Inject
-    SessionManager sessionManager;
-    @Inject ContributionController controller;
     @BindView(R.id.tab_layout)
     TabLayout tabLayout;
     @BindView(R.id.pager)
     public UnswipableViewPager viewPager;
+
+    @Inject
+    SessionManager sessionManager;
+    @Inject
+    ContributionController controller;
     @Inject
     public LocationServiceManager locationManager;
     @Inject
@@ -72,29 +75,16 @@ public class MainActivity extends AuthenticatedActivity implements FragmentManag
     public boolean isContributionsFragmentVisible = true; // False means nearby fragment is visible
     private Menu menu;
 
-    private boolean onOrientationChanged = false;
-
     private MenuItem notificationsMenuItem;
     private TextView notificationCount;
 
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
         setContentView(R.layout.activity_contributions);
         ButterKnife.bind(this);
-
         requestAuthToken();
         initDrawer();
         setTitle(getString(R.string.navigation_item_home)); // Should I create a new string variable with another name instead?
-
-
-        if (savedInstanceState != null ) {
-            onOrientationChanged = true; // Will be used in nearby fragment to determine significant update of map
-
-            //If nearby map was visible, call on Tab Selected to call all nearby operations
-            /*if (savedInstanceState.getInt("viewPagerCurrentItem") == 1) {
-                ((NearbyFragment)contributionsActivityPagerAdapter.getItem(1)).onTabSelected(onOrientationChanged);
-            }*/
-        }
     }
 
     @Override
@@ -161,9 +151,7 @@ public void setNumOfUploads(int uploadCount) {
      * tab won't change and vice versa. So we have to notify each of them.
      */
     private void setTabAndViewPagerSynchronisation() {
-        //viewPager.canScrollHorizontally(false);
         viewPager.setFocusableInTouchMode(true);
-
         viewPager.addOnPageChangeListener(new ViewPager.OnPageChangeListener() {
             @Override
             public void onPageScrolled(int position, float positionOffset, int positionOffsetPixels) {
@@ -178,15 +166,14 @@ public void onPageSelected(int position) {
                         tabLayout.getTabAt(CONTRIBUTIONS_TAB_POSITION).select();
                         isContributionsFragmentVisible = true;
                         updateMenuItem();
-
                         break;
                     case NEARBY_TAB_POSITION:
                         Timber.d("Nearby tab selected");
                         tabLayout.getTabAt(NEARBY_TAB_POSITION).select();
                         isContributionsFragmentVisible = false;
                         updateMenuItem();
                         // Do all permission and GPS related tasks on tab selected, not on create
-                        ((NearbyFragment)contributionsActivityPagerAdapter.getItem(1)).onTabSelected(onOrientationChanged);
+                        NearbyParentFragmentPresenter.getInstance().onTabSelected();
                         break;
                     default:
                         tabLayout.getTabAt(CONTRIBUTIONS_TAB_POSITION).select();
@@ -267,15 +254,7 @@ public void onBackPressed() {
             }
         } else if (getSupportFragmentManager().findFragmentByTag(nearbyFragmentTag) != null && !isContributionsFragmentVisible) {
             // Means that nearby fragment is visible (not contributions fragment)
-            NearbyFragment nearbyFragment = (NearbyFragment) contributionsActivityPagerAdapter.getItem(1);
-
-            if(nearbyFragment.isBottomSheetExpanded()) {
-                // Back should first hide the bottom sheet if it is expanded
-                nearbyFragment.listOptionMenuItemClicked();
-            } else {
-                // Otherwise go back to contributions fragment
-                viewPager.setCurrentItem(0);
-            }
+            NearbyParentFragmentPresenter.getInstance().backButtonClicked();
         } else {
             super.onBackPressed();
         }
@@ -332,12 +311,12 @@ private void updateMenuItem() {
                 // Display notifications menu item
                 menu.findItem(R.id.notifications).setVisible(true);
                 menu.findItem(R.id.list_sheet).setVisible(false);
-                Timber.d("Contributions activity notifications menu item is visible");
+                Timber.d("Contributions fragment notifications menu item is visible");
             } else {
                 // Display bottom list menu item
                 menu.findItem(R.id.notifications).setVisible(false);
                 menu.findItem(R.id.list_sheet).setVisible(true);
-                Timber.d("Contributions activity list sheet menu item is visible");
+                Timber.d("Nearby fragment list sheet menu item is visible");
             }
         }
     }
@@ -351,7 +330,7 @@ public boolean onOptionsItemSelected(MenuItem item) {
                 return true;
             case R.id.list_sheet:
                 if (contributionsActivityPagerAdapter.getItem(1) != null) {
-                    ((NearbyFragment)contributionsActivityPagerAdapter.getItem(1)).listOptionMenuItemClicked();
+                    ((NearbyParentFragment)contributionsActivityPagerAdapter.getItem(1)).listOptionMenuItemClicked();
                 }
                 return true;
             default:
@@ -361,8 +340,6 @@ public boolean onOptionsItemSelected(MenuItem item) {
 
     public class ContributionsActivityPagerAdapter extends FragmentPagerAdapter {
         FragmentManager fragmentManager;
-        private boolean isContributionsListFragment = true; // to know what to put in first tab, Contributions of Media Details
-
 
         public ContributionsActivityPagerAdapter(FragmentManager fragmentManager) {
             super(fragmentManager);
@@ -392,12 +369,12 @@ public Fragment getItem(int position) {
                     }
 
                 case 1:
-                    NearbyFragment retainedNearbyFragment = getNearbyFragment(1);
+                    NearbyParentFragment retainedNearbyFragment = getNearbyFragment(1);
                     if (retainedNearbyFragment != null) {
                         return retainedNearbyFragment;
                     } else {
                         // If we reach here, retainedNearbyFragment is null
-                        return new NearbyFragment();
+                        return new NearbyParentFragment();
                     }
                 default:
                     return null;
@@ -419,9 +396,9 @@ private ContributionsFragment getContributionsFragment(int position) {
          * @param position index of tabs, in our case 0 or 1
          * @return
          */
-        private NearbyFragment getNearbyFragment(int position) {
+        private NearbyParentFragment getNearbyFragment(int position) {
             String tag = makeFragmentName(R.id.pager, position);
-            return (NearbyFragment)fragmentManager.findFragmentByTag(tag);
+            return (NearbyParentFragment)fragmentManager.findFragmentByTag(tag);
         }
 
         /**
@@ -444,7 +421,6 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {
         controller.handleActivityResult(this, requestCode, resultCode, data);
     }
 
-    @Override
     protected void onResume() {
         super.onResume();
         setNotificationCount();

app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.java

@@ -13,9 +13,9 @@
 import fr.free.nrw.commons.explore.recentsearches.RecentSearchesFragment;
 import fr.free.nrw.commons.media.MediaDetailFragment;
 import fr.free.nrw.commons.media.MediaDetailPagerFragment;
-import fr.free.nrw.commons.nearby.NearbyFragment;
-import fr.free.nrw.commons.nearby.NearbyListFragment;
-import fr.free.nrw.commons.nearby.NearbyMapFragment;
+import fr.free.nrw.commons.nearby.fragments.NearbyListFragment;
+import fr.free.nrw.commons.nearby.fragments.NearbyMapFragment;
+import fr.free.nrw.commons.nearby.fragments.NearbyParentFragment;
 import fr.free.nrw.commons.review.ReviewImageFragment;
 import fr.free.nrw.commons.settings.SettingsFragment;
 import fr.free.nrw.commons.upload.categories.UploadCategoriesFragment;
@@ -38,9 +38,6 @@ public abstract class FragmentBuilderModule {
     @ContributesAndroidInjector
     abstract NearbyListFragment bindNearbyListFragment();
 
-    @ContributesAndroidInjector
-    abstract NearbyMapFragment bindNearbyMapFragment();
-
     @ContributesAndroidInjector
     abstract SettingsFragment bindSettingsFragment();
 
@@ -63,7 +60,10 @@ public abstract class FragmentBuilderModule {
     abstract ContributionsFragment bindContributionsFragment();
 
     @ContributesAndroidInjector
-    abstract NearbyFragment bindNearbyFragment();
+    abstract NearbyMapFragment bindNearbyMapFragment();
+
+    @ContributesAndroidInjector
+    abstract NearbyParentFragment bindNearbyParentFragment();
 
     @ContributesAndroidInjector
     abstract BookmarkPicturesFragment bindBookmarkPictureListFragment();

app/src/main/java/fr/free/nrw/commons/location/LocationServiceManager.java

@@ -6,7 +6,6 @@
 import android.location.LocationListener;
 import android.location.LocationManager;
 import android.os.Bundle;
-
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -222,6 +221,7 @@ public enum LocationChangeType{
         LOCATION_MEDIUM_CHANGED,      //Between slight and significant changes, will be used for nearby card view updates.
         LOCATION_NOT_CHANGED,
         PERMISSION_JUST_GRANTED,
-        MAP_UPDATED
+        MAP_UPDATED,
+        SEARCH_CUSTOM_AREA
     }
 }

app/src/main/java/fr/free/nrw/commons/mwapi/UploadResult.java

@@ -1,6 +1,5 @@
 package fr.free.nrw.commons.mwapi;
 
-import org.jetbrains.annotations.NotNull;
 
 import java.util.Date;
 
@@ -36,7 +35,6 @@ public class UploadResult {
         this.imageUrl = imageUrl;
     }
 
-    @NotNull
     @Override
     public String toString() {
         return "UploadResult{" +

app/src/main/java/fr/free/nrw/commons/nearby/NearbyBaseMarker.java

@@ -23,7 +23,7 @@ public NearbyBaseMarker[] newArray(int size) {
 
     private Place place;
 
-    NearbyBaseMarker() {
+    public NearbyBaseMarker() {
     }
 
     private NearbyBaseMarker(Parcel in) {

app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java

@@ -28,8 +28,10 @@
 public class NearbyController {
     private static final int MAX_RESULTS = 1000;
     private final NearbyPlaces nearbyPlaces;
-    public static double searchedRadius = 10.0; //in kilometers
-    public static LatLng currentLocation;
+    public static double currentLocationSearchRadius = 10.0; //in kilometers
+    public static LatLng currentLocation; // Users latest fetched location
+    public static LatLng latestSearchLocation; // Can be current and camera target on search this area button is used
+    public static double latestSearchRadius = 10.0; // Any last search radius except closest result search
 
     @Inject
     public NearbyController(NearbyPlaces nearbyPlaces) {
@@ -41,21 +43,21 @@ public NearbyController(NearbyPlaces nearbyPlaces) {
      * Prepares Place list to make their distance information update later.
      *
      * @param curLatLng current location for user
-     * @param latLangToSearchAround the location user wants to search around
+     * @param searchLatLng the location user wants to search around
      * @param returnClosestResult if this search is done to find closest result or all results
      * @return NearbyPlacesInfo a variable holds Place list without distance information
      * and boundary coordinates of current Place List
      */
-    public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng latLangToSearchAround, boolean returnClosestResult, boolean checkingAroundCurrentLocation) throws IOException {
+    public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng searchLatLng, boolean returnClosestResult, boolean checkingAroundCurrentLocation) throws IOException {
 
-        Timber.d("Loading attractions near %s", latLangToSearchAround);
+        Timber.d("Loading attractions near %s", searchLatLng);
         NearbyPlacesInfo nearbyPlacesInfo = new NearbyPlacesInfo();
 
-        if (latLangToSearchAround == null) {
+        if (searchLatLng == null) {
             Timber.d("Loading attractions nearby, but curLatLng is null");
             return null;
         }
-        List<Place> places = nearbyPlaces.radiusExpander(latLangToSearchAround, Locale.getDefault().getLanguage(), returnClosestResult);
+        List<Place> places = nearbyPlaces.radiusExpander(searchLatLng, Locale.getDefault().getLanguage(), returnClosestResult);
 
         if (null != places && places.size() > 0) {
             LatLng[] boundaryCoordinates = {places.get(0).location,   // south
@@ -91,13 +93,25 @@ public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng lat
                         }
                 );
             }
+            nearbyPlacesInfo.curLatLng = curLatLng;
+            nearbyPlacesInfo.searchLatLng = searchLatLng;
             nearbyPlacesInfo.placeList = places;
             nearbyPlacesInfo.boundaryCoordinates = boundaryCoordinates;
-            if (!returnClosestResult && checkingAroundCurrentLocation) {
-                // Do not update searched radius, if controller is used for nearby card notification
-                searchedRadius = nearbyPlaces.radius;
-                currentLocation = curLatLng;
+
+            // Returning closes result means we use the controller for nearby card. So no need to set search this area flags
+            if (!returnClosestResult) {
+                // To remember latest search either around user or any point on map
+                latestSearchLocation = searchLatLng;
+                latestSearchRadius = nearbyPlaces.radius*1000; // to meter
+
+                // Our radius searched around us, will be used to understand when user search their own location, we will follow them
+                if (checkingAroundCurrentLocation) {
+                    currentLocationSearchRadius = nearbyPlaces.radius*1000; // to meter
+                    currentLocation = curLatLng;
+                }
             }
+
+
             return nearbyPlacesInfo;
         }
         else {
@@ -112,7 +126,7 @@ public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng lat
      * @param placeList list of nearby places in Place data type
      * @return Place list that holds nearby places
      */
-    static List<Place> loadAttractionsFromLocationToPlaces(
+    public static List<Place> loadAttractionsFromLocationToPlaces(
             LatLng curLatLng,
             List<Place> placeList) {
         placeList = placeList.subList(0, Math.min(placeList.size(), MAX_RESULTS));
@@ -212,5 +226,7 @@ public static List<NearbyBaseMarker> loadAttractionsFromLocationToBaseMarkerOpti
     public class NearbyPlacesInfo {
         public List<Place> placeList; // List of nearby places
         public LatLng[] boundaryCoordinates; // Corners of nearby area
+        public LatLng curLatLng; // Current location when this places are populated
+        public LatLng searchLatLng; // Search location for finding this places
     }
 }