Skip to content

Commit 22b42a6

Browse files
neslihanturanVivek Maskara
authored and
Vivek Maskara
committed
Refactor nearby classes mvp (#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.
1 parent b21640b commit 22b42a6

24 files changed

+1995
-1938
lines changed

app/build.gradle

+3-2
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ dependencies {
3636
implementation 'fr.avianey.com.viewpagerindicator:library:2.4.1.1@aar'
3737
implementation 'com.github.chrisbanes:PhotoView:2.0.0'
3838
implementation 'com.github.pedrovgs:renderers:3.3.3'
39-
implementation 'com.mapbox.mapboxsdk:mapbox-android-plugin-localization:0.6.0'
39+
implementation 'com.mapbox.mapboxsdk:mapbox-android-sdk:7.2.0'
40+
implementation 'com.mapbox.mapboxsdk:mapbox-android-plugin-localization-v7:0.7.0'
4041
implementation 'com.github.deano2390:MaterialShowcaseView:1.2.0'
4142
implementation 'com.dinuscxj:circleprogressbar:1.1.1'
4243
implementation 'com.karumi:dexter:5.0.0'
@@ -162,7 +163,7 @@ android {
162163
versionNameSuffix "-debug-" + getBranchName()
163164
}
164165
}
165-
166+
166167
if (isRunningOnTravisAndIsNotPRBuild) {
167168
// configure keystore based on env vars in Travis for automated alpha builds
168169
signingConfigs.release.storeFile = file("../nr-commons.keystore")

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

+17-29
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@
3434
import fr.free.nrw.commons.R;
3535
import fr.free.nrw.commons.auth.SessionManager;
3636
import fr.free.nrw.commons.location.LocationServiceManager;
37-
import fr.free.nrw.commons.nearby.NearbyFragment;
3837
import fr.free.nrw.commons.nearby.NearbyNotificationCardView;
38+
import fr.free.nrw.commons.nearby.fragments.NearbyParentFragment;
39+
import fr.free.nrw.commons.nearby.presenter.NearbyParentFragmentPresenter;
3940
import fr.free.nrw.commons.notification.Notification;
4041
import fr.free.nrw.commons.notification.NotificationActivity;
4142
import fr.free.nrw.commons.notification.NotificationController;
@@ -50,13 +51,15 @@
5051

5152
public class MainActivity extends NavigationBaseActivity implements FragmentManager.OnBackStackChangedListener {
5253

53-
@Inject
54-
SessionManager sessionManager;
55-
@Inject ContributionController controller;
5654
@BindView(R.id.tab_layout)
5755
TabLayout tabLayout;
5856
@BindView(R.id.pager)
5957
public UnswipableViewPager viewPager;
58+
59+
@Inject
60+
SessionManager sessionManager;
61+
@Inject
62+
ContributionController controller;
6063
@Inject
6164
public LocationServiceManager locationManager;
6265
@Inject
@@ -72,10 +75,9 @@ public class MainActivity extends NavigationBaseActivity implements FragmentMana
7275
public static final int NEARBY_TAB_POSITION = 1;
7376

7477
public boolean isContributionsFragmentVisible = true; // False means nearby fragment is visible
78+
public boolean onOrientationChanged;
7579
private Menu menu;
7680

77-
private boolean onOrientationChanged = false;
78-
7981
private MenuItem notificationsMenuItem;
8082
private TextView notificationCount;
8183

@@ -168,9 +170,7 @@ public void setNumOfUploads(int uploadCount) {
168170
* tab won't change and vice versa. So we have to notify each of them.
169171
*/
170172
private void setTabAndViewPagerSynchronisation() {
171-
//viewPager.canScrollHorizontally(false);
172173
viewPager.setFocusableInTouchMode(true);
173-
174174
viewPager.addOnPageChangeListener(new ViewPager.OnPageChangeListener() {
175175
@Override
176176
public void onPageScrolled(int position, float positionOffset, int positionOffsetPixels) {
@@ -185,15 +185,14 @@ public void onPageSelected(int position) {
185185
tabLayout.getTabAt(CONTRIBUTIONS_TAB_POSITION).select();
186186
isContributionsFragmentVisible = true;
187187
updateMenuItem();
188-
189188
break;
190189
case NEARBY_TAB_POSITION:
191190
Timber.d("Nearby tab selected");
192191
tabLayout.getTabAt(NEARBY_TAB_POSITION).select();
193192
isContributionsFragmentVisible = false;
194193
updateMenuItem();
195194
// Do all permission and GPS related tasks on tab selected, not on create
196-
((NearbyFragment)contributionsActivityPagerAdapter.getItem(1)).onTabSelected(onOrientationChanged);
195+
NearbyParentFragmentPresenter.getInstance().onTabSelected();
197196
break;
198197
default:
199198
tabLayout.getTabAt(CONTRIBUTIONS_TAB_POSITION).select();
@@ -269,15 +268,7 @@ public void onBackPressed() {
269268
}
270269
} else if (getSupportFragmentManager().findFragmentByTag(nearbyFragmentTag) != null && !isContributionsFragmentVisible) {
271270
// Means that nearby fragment is visible (not contributions fragment)
272-
NearbyFragment nearbyFragment = (NearbyFragment) contributionsActivityPagerAdapter.getItem(1);
273-
274-
if(nearbyFragment.isBottomSheetExpanded()) {
275-
// Back should first hide the bottom sheet if it is expanded
276-
nearbyFragment.listOptionMenuItemClicked();
277-
} else {
278-
// Otherwise go back to contributions fragment
279-
viewPager.setCurrentItem(0);
280-
}
271+
NearbyParentFragmentPresenter.getInstance().backButtonClicked();
281272
} else {
282273
super.onBackPressed();
283274
}
@@ -334,12 +325,12 @@ private void updateMenuItem() {
334325
// Display notifications menu item
335326
menu.findItem(R.id.notifications).setVisible(true);
336327
menu.findItem(R.id.list_sheet).setVisible(false);
337-
Timber.d("Contributions activity notifications menu item is visible");
328+
Timber.d("Contributions fragment notifications menu item is visible");
338329
} else {
339330
// Display bottom list menu item
340331
menu.findItem(R.id.notifications).setVisible(false);
341332
menu.findItem(R.id.list_sheet).setVisible(true);
342-
Timber.d("Contributions activity list sheet menu item is visible");
333+
Timber.d("Nearby fragment list sheet menu item is visible");
343334
}
344335
}
345336
}
@@ -353,7 +344,7 @@ public boolean onOptionsItemSelected(MenuItem item) {
353344
return true;
354345
case R.id.list_sheet:
355346
if (contributionsActivityPagerAdapter.getItem(1) != null) {
356-
((NearbyFragment)contributionsActivityPagerAdapter.getItem(1)).listOptionMenuItemClicked();
347+
((NearbyParentFragment)contributionsActivityPagerAdapter.getItem(1)).listOptionMenuItemClicked();
357348
}
358349
return true;
359350
default:
@@ -363,8 +354,6 @@ public boolean onOptionsItemSelected(MenuItem item) {
363354

364355
public class ContributionsActivityPagerAdapter extends FragmentPagerAdapter {
365356
FragmentManager fragmentManager;
366-
private boolean isContributionsListFragment = true; // to know what to put in first tab, Contributions of Media Details
367-
368357

369358
public ContributionsActivityPagerAdapter(FragmentManager fragmentManager) {
370359
super(fragmentManager);
@@ -394,12 +383,12 @@ public Fragment getItem(int position) {
394383
}
395384

396385
case 1:
397-
NearbyFragment retainedNearbyFragment = getNearbyFragment(1);
386+
NearbyParentFragment retainedNearbyFragment = getNearbyFragment(1);
398387
if (retainedNearbyFragment != null) {
399388
return retainedNearbyFragment;
400389
} else {
401390
// If we reach here, retainedNearbyFragment is null
402-
return new NearbyFragment();
391+
return new NearbyParentFragment();
403392
}
404393
default:
405394
return null;
@@ -421,9 +410,9 @@ private ContributionsFragment getContributionsFragment(int position) {
421410
* @param position index of tabs, in our case 0 or 1
422411
* @return
423412
*/
424-
private NearbyFragment getNearbyFragment(int position) {
413+
private NearbyParentFragment getNearbyFragment(int position) {
425414
String tag = makeFragmentName(R.id.pager, position);
426-
return (NearbyFragment)fragmentManager.findFragmentByTag(tag);
415+
return (NearbyParentFragment)fragmentManager.findFragmentByTag(tag);
427416
}
428417

429418
/**
@@ -446,7 +435,6 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {
446435
controller.handleActivityResult(this, requestCode, resultCode, data);
447436
}
448437

449-
@Override
450438
protected void onResume() {
451439
super.onResume();
452440
setNotificationCount();

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

+7-7
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
import fr.free.nrw.commons.explore.recentsearches.RecentSearchesFragment;
1414
import fr.free.nrw.commons.media.MediaDetailFragment;
1515
import fr.free.nrw.commons.media.MediaDetailPagerFragment;
16-
import fr.free.nrw.commons.nearby.NearbyFragment;
17-
import fr.free.nrw.commons.nearby.NearbyListFragment;
18-
import fr.free.nrw.commons.nearby.NearbyMapFragment;
16+
import fr.free.nrw.commons.nearby.fragments.NearbyListFragment;
17+
import fr.free.nrw.commons.nearby.fragments.NearbyMapFragment;
18+
import fr.free.nrw.commons.nearby.fragments.NearbyParentFragment;
1919
import fr.free.nrw.commons.review.ReviewImageFragment;
2020
import fr.free.nrw.commons.settings.SettingsFragment;
2121
import fr.free.nrw.commons.upload.categories.UploadCategoriesFragment;
@@ -38,9 +38,6 @@ public abstract class FragmentBuilderModule {
3838
@ContributesAndroidInjector
3939
abstract NearbyListFragment bindNearbyListFragment();
4040

41-
@ContributesAndroidInjector
42-
abstract NearbyMapFragment bindNearbyMapFragment();
43-
4441
@ContributesAndroidInjector
4542
abstract SettingsFragment bindSettingsFragment();
4643

@@ -63,7 +60,10 @@ public abstract class FragmentBuilderModule {
6360
abstract ContributionsFragment bindContributionsFragment();
6461

6562
@ContributesAndroidInjector
66-
abstract NearbyFragment bindNearbyFragment();
63+
abstract NearbyMapFragment bindNearbyMapFragment();
64+
65+
@ContributesAndroidInjector
66+
abstract NearbyParentFragment bindNearbyParentFragment();
6767

6868
@ContributesAndroidInjector
6969
abstract BookmarkPicturesFragment bindBookmarkPictureListFragment();

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import android.location.LocationListener;
77
import android.location.LocationManager;
88
import android.os.Bundle;
9-
109
import java.util.HashSet;
1110
import java.util.List;
1211
import java.util.Set;
@@ -222,6 +221,7 @@ public enum LocationChangeType{
222221
LOCATION_MEDIUM_CHANGED, //Between slight and significant changes, will be used for nearby card view updates.
223222
LOCATION_NOT_CHANGED,
224223
PERMISSION_JUST_GRANTED,
225-
MAP_UPDATED
224+
MAP_UPDATED,
225+
SEARCH_CUSTOM_AREA
226226
}
227227
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public NearbyBaseMarker[] newArray(int size) {
2323

2424
private Place place;
2525

26-
NearbyBaseMarker() {
26+
public NearbyBaseMarker() {
2727
}
2828

2929
private NearbyBaseMarker(Parcel in) {

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

+28-12
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,10 @@
2828
public class NearbyController {
2929
private static final int MAX_RESULTS = 1000;
3030
private final NearbyPlaces nearbyPlaces;
31-
public static double searchedRadius = 10.0; //in kilometers
32-
public static LatLng currentLocation;
31+
public static double currentLocationSearchRadius = 10.0; //in kilometers
32+
public static LatLng currentLocation; // Users latest fetched location
33+
public static LatLng latestSearchLocation; // Can be current and camera target on search this area button is used
34+
public static double latestSearchRadius = 10.0; // Any last search radius except closest result search
3335

3436
@Inject
3537
public NearbyController(NearbyPlaces nearbyPlaces) {
@@ -41,21 +43,21 @@ public NearbyController(NearbyPlaces nearbyPlaces) {
4143
* Prepares Place list to make their distance information update later.
4244
*
4345
* @param curLatLng current location for user
44-
* @param latLangToSearchAround the location user wants to search around
46+
* @param searchLatLng the location user wants to search around
4547
* @param returnClosestResult if this search is done to find closest result or all results
4648
* @return NearbyPlacesInfo a variable holds Place list without distance information
4749
* and boundary coordinates of current Place List
4850
*/
49-
public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng latLangToSearchAround, boolean returnClosestResult, boolean checkingAroundCurrentLocation) throws IOException {
51+
public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng searchLatLng, boolean returnClosestResult, boolean checkingAroundCurrentLocation) throws IOException {
5052

51-
Timber.d("Loading attractions near %s", latLangToSearchAround);
53+
Timber.d("Loading attractions near %s", searchLatLng);
5254
NearbyPlacesInfo nearbyPlacesInfo = new NearbyPlacesInfo();
5355

54-
if (latLangToSearchAround == null) {
56+
if (searchLatLng == null) {
5557
Timber.d("Loading attractions nearby, but curLatLng is null");
5658
return null;
5759
}
58-
List<Place> places = nearbyPlaces.radiusExpander(latLangToSearchAround, Locale.getDefault().getLanguage(), returnClosestResult);
60+
List<Place> places = nearbyPlaces.radiusExpander(searchLatLng, Locale.getDefault().getLanguage(), returnClosestResult);
5961

6062
if (null != places && places.size() > 0) {
6163
LatLng[] boundaryCoordinates = {places.get(0).location, // south
@@ -91,13 +93,25 @@ public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng lat
9193
}
9294
);
9395
}
96+
nearbyPlacesInfo.curLatLng = curLatLng;
97+
nearbyPlacesInfo.searchLatLng = searchLatLng;
9498
nearbyPlacesInfo.placeList = places;
9599
nearbyPlacesInfo.boundaryCoordinates = boundaryCoordinates;
96-
if (!returnClosestResult && checkingAroundCurrentLocation) {
97-
// Do not update searched radius, if controller is used for nearby card notification
98-
searchedRadius = nearbyPlaces.radius;
99-
currentLocation = curLatLng;
100+
101+
// Returning closes result means we use the controller for nearby card. So no need to set search this area flags
102+
if (!returnClosestResult) {
103+
// To remember latest search either around user or any point on map
104+
latestSearchLocation = searchLatLng;
105+
latestSearchRadius = nearbyPlaces.radius*1000; // to meter
106+
107+
// Our radius searched around us, will be used to understand when user search their own location, we will follow them
108+
if (checkingAroundCurrentLocation) {
109+
currentLocationSearchRadius = nearbyPlaces.radius*1000; // to meter
110+
currentLocation = curLatLng;
111+
}
100112
}
113+
114+
101115
return nearbyPlacesInfo;
102116
}
103117
else {
@@ -112,7 +126,7 @@ public NearbyPlacesInfo loadAttractionsFromLocation(LatLng curLatLng, LatLng lat
112126
* @param placeList list of nearby places in Place data type
113127
* @return Place list that holds nearby places
114128
*/
115-
static List<Place> loadAttractionsFromLocationToPlaces(
129+
public static List<Place> loadAttractionsFromLocationToPlaces(
116130
LatLng curLatLng,
117131
List<Place> placeList) {
118132
placeList = placeList.subList(0, Math.min(placeList.size(), MAX_RESULTS));
@@ -212,5 +226,7 @@ public static List<NearbyBaseMarker> loadAttractionsFromLocationToBaseMarkerOpti
212226
public class NearbyPlacesInfo {
213227
public List<Place> placeList; // List of nearby places
214228
public LatLng[] boundaryCoordinates; // Corners of nearby area
229+
public LatLng curLatLng; // Current location when this places are populated
230+
public LatLng searchLatLng; // Search location for finding this places
215231
}
216232
}

0 commit comments

Comments
 (0)