From 784ee8b506d96fbb0edc7eb19736d716b38f19da Mon Sep 17 00:00:00 2001 From: neslihanturan Date: Wed, 19 Dec 2018 20:49:08 +0200 Subject: [PATCH] Fix #1828: Add missing Javadocs and tidy up code in nearby package (#2133) * Add Javadocs * Remove unused variables, methods, fragments, imports, classes --- .../nrw/commons/di/FragmentBuilderModule.java | 4 - .../nrw/commons/nearby/NearbyController.java | 4 +- .../nrw/commons/nearby/NearbyFragment.java | 48 +++++++++- .../commons/nearby/NearbyListFragment.java | 16 +++- .../nrw/commons/nearby/NearbyMapFragment.java | 32 +++++-- .../NearbyMaterialShowcaseSequence.java | 18 ---- .../nearby/NearbyNoificationCardView.java | 92 ++----------------- .../free/nrw/commons/nearby/NearbyPlaces.java | 28 ++++++ .../commons/nearby/NoPermissionsFragment.java | 38 -------- 9 files changed, 122 insertions(+), 158 deletions(-) delete mode 100644 app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java delete mode 100644 app/src/main/java/fr/free/nrw/commons/nearby/NoPermissionsFragment.java diff --git a/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.java b/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.java index b664d49e1..14bef3ab4 100644 --- a/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.java +++ b/app/src/main/java/fr/free/nrw/commons/di/FragmentBuilderModule.java @@ -16,7 +16,6 @@ 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.NoPermissionsFragment; import fr.free.nrw.commons.settings.SettingsFragment; @Module @@ -38,9 +37,6 @@ public abstract class FragmentBuilderModule { @ContributesAndroidInjector abstract NearbyMapFragment bindNearbyMapFragment(); - @ContributesAndroidInjector - abstract NoPermissionsFragment bindNoPermissionsFragment(); - @ContributesAndroidInjector abstract SettingsFragment bindSettingsFragment(); diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java index abf02362f..e6ac27c90 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java @@ -5,7 +5,6 @@ import android.content.SharedPreferences; import android.content.res.Resources; import android.graphics.Bitmap; import android.support.graphics.drawable.VectorDrawableCompat; -import android.util.Log; import com.mapbox.mapboxsdk.annotations.IconFactory; @@ -179,6 +178,9 @@ public class NearbyController { return baseMarkerOptions; } + /** + * We pass this variable as a group of placeList and boundaryCoordinates + */ public class NearbyPlacesInfo { public List placeList; // List of nearby places public LatLng[] boundaryCoordinates; // Corners of nearby area diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java index 5a1c48edf..c1960f82d 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyFragment.java @@ -14,7 +14,6 @@ import android.support.design.widget.BottomSheetBehavior; import android.support.design.widget.Snackbar; import android.support.v4.app.FragmentTransaction; import android.support.v7.app.AlertDialog; -import android.util.Log; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; @@ -211,6 +210,10 @@ public class NearbyFragment extends CommonsDaggerSupportFragment bottomSheetBehaviorForDetails.setState(BottomSheetBehavior.STATE_HIDDEN); } + /** + * Sets camera position, zoom level according to sheet positions + * @param bottomSheetState expanded, collapsed or hidden + */ public void prepareViewsForSheetPosition(int bottomSheetState) { // TODO } @@ -243,7 +246,7 @@ public class NearbyFragment extends CommonsDaggerSupportFragment /** * This method should be the single point to load/refresh nearby places * - * @param locationChangeType defines if location shanged significantly or slightly + * @param locationChangeType defines if location changed significantly or slightly */ public void refreshView(LocationServiceManager.LocationChangeType locationChangeType) { Timber.d("Refreshing nearby places"); @@ -359,7 +362,6 @@ public class NearbyFragment extends CommonsDaggerSupportFragment * @param nearbyPlacesInfo This variable has place list information and distances. */ private void populatePlacesFromCustomLocation(NearbyController.NearbyPlacesInfo nearbyPlacesInfo) { - //NearbyMapFragment nearbyMapFragment = getMapFragment(); if (nearbyMapFragment != null) { nearbyMapFragment.searchThisAreaButtonProgressBar.setVisibility(View.GONE); } @@ -374,6 +376,11 @@ public class NearbyFragment extends CommonsDaggerSupportFragment } } + /** + * Turns nearby place lists and boundary coordinates into gson and update map and list fragments + * accordingly + * @param nearbyPlacesInfo a variable holds both nearby place list and boundary coordinates + */ private void populatePlaces(NearbyController.NearbyPlacesInfo nearbyPlacesInfo) { Timber.d("Populating nearby places"); List placeList = nearbyPlacesInfo.placeList; @@ -390,7 +397,6 @@ public class NearbyFragment extends CommonsDaggerSupportFragment } bundle.putString("PlaceList", gsonPlaceList); - //bundle.putString("CurLatLng", gsonCurLatLng); bundle.putString("BoundaryCoord", gsonBoundaryCoordinates); // First time to init fragments @@ -412,7 +418,7 @@ public class NearbyFragment extends CommonsDaggerSupportFragment /** * Lock nearby view updates while updating map or list. Because we don't want new update calls * when we already updating for old location update. - * @param lock + * @param lock true if we should lock nearby map */ private void lockNearbyView(boolean lock) { if (lock) { @@ -426,6 +432,18 @@ public class NearbyFragment extends CommonsDaggerSupportFragment } } + /** + * Updates map fragment, + * For slight update: camera follows users location + * For significant update: nearby markers are removed and new markers added again + * Slight updates stop if user is checking another area of map + * + * @param updateViaButton search this area button is clicked + * @param isSlightUpdate Means no need to update markers, just follow user location with camera + * @param customLatLng Will be used for updates for other locations than users current location. + * Ie. when we use search this area feature + * @param nearbyPlacesInfo Includes nearby places list and boundary coordinates + */ private void updateMapFragment(boolean updateViaButton, boolean isSlightUpdate, @Nullable LatLng customLatLng, @Nullable NearbyController.NearbyPlacesInfo nearbyPlacesInfo) { if (nearbyMapFragment.searchThisAreaModeOn) { @@ -499,6 +517,10 @@ public class NearbyFragment extends CommonsDaggerSupportFragment } } + /** + * Updates already existing list fragment with bundle includes nearby places and boundary + * coordinates + */ private void updateListFragment() { nearbyListFragment.setBundleForUpdates(bundle); nearbyListFragment.updateNearbyListSignificantly(); @@ -537,6 +559,9 @@ public class NearbyFragment extends CommonsDaggerSupportFragment fragmentTransaction.commitAllowingStateLoss(); } + /** + * Hides progress bar + */ private void hideProgressBar() { if (progressBar != null) { progressBar.setVisibility(View.GONE); @@ -576,12 +601,18 @@ public class NearbyFragment extends CommonsDaggerSupportFragment } } + /** + * Requests location permission if activity is not null + */ private void requestLocationPermissions() { if (!getActivity().isFinishing()) { locationManager.requestPermissions(getActivity()); } } + /** + * Will warn user if location is denied + */ private void showLocationPermissionDeniedErrorDialog() { new AlertDialog.Builder(getActivity()) .setMessage(R.string.nearby_needs_permissions) @@ -671,6 +702,9 @@ public class NearbyFragment extends CommonsDaggerSupportFragment ViewUtil.showLongToast(getActivity(), message); } + /** + * Adds network broadcast receiver to recognize connection established + */ private void addNetworkBroadcastReceiver() { if (!FragmentUtils.isFragmentUIActive(this)) { return; @@ -708,6 +742,10 @@ public class NearbyFragment extends CommonsDaggerSupportFragment resumeFragment(); } + /** + * Perform nearby operations on nearby tab selected + * @param onOrientationChanged pass orientation changed info to fragment + */ public void onTabSelected(boolean onOrientationChanged) { Timber.d("On nearby tab selected"); this.onOrientationChanged = onOrientationChanged; diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyListFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyListFragment.java index c6423f8ab..4320ce2d7 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyListFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyListFragment.java @@ -92,6 +92,9 @@ public class NearbyListFragment extends DaggerFragment { recyclerView.setAdapter(adapterFactory.create(getPlaceListFromBundle(bundle))); } + /** + * Updates nearby list elements all together + */ public void updateNearbyListSignificantly() { try { adapterFactory.updateAdapterData(getPlaceListFromBundle(bundleForUpdates), (RVRendererAdapter) recyclerView.getAdapter()); @@ -103,7 +106,7 @@ public class NearbyListFragment extends DaggerFragment { /** * While nearby updates for current location held with bundle, automatically, custom updates are * done by calling this methods, triddered by search this are button input from user. - * @param placeList + * @param placeList List of nearby places to be added list fragment */ public void updateNearbyListSignificantlyForCustomLocation(List placeList) { try { @@ -113,6 +116,13 @@ public class NearbyListFragment extends DaggerFragment { } } + /** + * When user moved too much, we need to update nearby list too. This operation is made by passing + * a bundle from NearbyFragment to NearbyListFragment and NearbyMapFragment. This method extracts + * place list from bundle to a list variable. + * @param bundle Bundle passed from NearbyFragment on users significant moving + * @return List of new nearby places + */ private List getPlaceListFromBundle(Bundle bundle) { List placeList = Collections.emptyList(); @@ -176,6 +186,10 @@ public class NearbyListFragment extends DaggerFragment { } } + /** + * Sets bundles for updates in map. Ie. user is moved too much so we need to update nearby markers. + * @param bundleForUpdates includes new calculated nearby places. + */ public void setBundleForUpdates(Bundle bundleForUpdates) { this.bundleForUpdates = bundleForUpdates; } diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMapFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMapFragment.java index 8d8f4989f..4b3dfb17e 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMapFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMapFragment.java @@ -15,7 +15,6 @@ import android.support.design.widget.BottomSheetBehavior; import android.support.design.widget.CoordinatorLayout; import android.support.design.widget.FloatingActionButton; import android.support.v7.app.AlertDialog; -import android.util.Log; import android.view.Gravity; import android.view.KeyEvent; import android.view.LayoutInflater; @@ -62,7 +61,7 @@ import fr.free.nrw.commons.Utils; import fr.free.nrw.commons.auth.LoginActivity; import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao; import fr.free.nrw.commons.contributions.ContributionController; -import fr.free.nrw.commons.location.LocationServiceManager; + import fr.free.nrw.commons.utils.LocationUtils; import fr.free.nrw.commons.utils.PlaceUtils; import fr.free.nrw.commons.utils.UriDeserializer; @@ -130,7 +129,6 @@ public class NearbyMapFragment extends DaggerFragment { private final double CAMERA_TARGET_SHIFT_FACTOR_PORTRAIT = 0.06; private final double CAMERA_TARGET_SHIFT_FACTOR_LANDSCAPE = 0.04; - private boolean isMapReady; public boolean searchThisAreaModeOn = false; private Bundle bundleForUpdtes;// Carry information from activity about changed nearby places and current location @@ -361,7 +359,10 @@ public class NearbyMapFragment extends DaggerFragment { } } } - + + /** + * Initialize all views. TODO: Use bind view instead. + */ private void initViews() { Timber.d("initViews called"); bottomSheetList = ((NearbyFragment)getParentFragment()).view.findViewById(R.id.bottom_sheet); @@ -406,6 +407,9 @@ public class NearbyMapFragment extends DaggerFragment { } + /** + * Sets click listeners of FABs, and 2 bottom sheets + */ private void setListeners() { fabPlus.setOnClickListener(view -> { if (applicationPrefs.getBoolean("login_skipped", false)) { @@ -512,6 +516,10 @@ public class NearbyMapFragment extends DaggerFragment { } } + /** + * Sets up map view of first time it created, it passes MapBoxMap options and style assets. + * @param savedInstanceState bundle coming from Nearby Fragment + */ private void setupMapView(Bundle savedInstanceState) { Timber.d("setupMapView called"); MapboxMapOptions options = new MapboxMapOptions() @@ -541,6 +549,10 @@ public class NearbyMapFragment extends DaggerFragment { } } + /** + * Adds map movement listener to understand swiping with fingers. So that we can display search + * this area button to search nearby places for other locations + */ private void addMapMovementListeners() { mapboxMap.addOnCameraMoveListener(new MapboxMap.OnCameraMoveListener() { @@ -793,9 +805,7 @@ public class NearbyMapFragment extends DaggerFragment { addAnchorToSmallFABs(fabGallery, ((NearbyFragment)getParentFragment()).view.findViewById(R.id.empty_view).getId()); addAnchorToSmallFABs(fabCamera, ((NearbyFragment)getParentFragment()).view.findViewById(R.id.empty_view1).getId()); - - isMapReady = true; - } + } /* @@ -946,6 +956,10 @@ public class NearbyMapFragment extends DaggerFragment { Utils.handleWebUrl(getContext(), link); } + /** + * Starts animation of fab plus (turning on opening) and other FABs + * @param isFabOpen state of FAB buttons, open when clicked on fab button, closed on other click + */ private void animateFAB(boolean isFabOpen) { this.isFabOpen = !isFabOpen; if (fabPlus.isShown()){ @@ -966,6 +980,10 @@ public class NearbyMapFragment extends DaggerFragment { } } + /** + * Hides camera and gallery FABs, turn back plus FAB + * @param isFabOpen + */ private void closeFabs ( boolean isFabOpen){ if (isFabOpen) { fabPlus.startAnimation(rotate_backward); diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java deleted file mode 100644 index c6e46611d..000000000 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyMaterialShowcaseSequence.java +++ /dev/null @@ -1,18 +0,0 @@ -package fr.free.nrw.commons.nearby; - -import android.app.Activity; - -import uk.co.deanwild.materialshowcaseview.MaterialShowcaseSequence; -import uk.co.deanwild.materialshowcaseview.ShowcaseConfig; - - -public class NearbyMaterialShowcaseSequence extends MaterialShowcaseSequence { - - public NearbyMaterialShowcaseSequence(Activity activity, String sequenceID) { - super(activity, sequenceID); - ShowcaseConfig config = new ShowcaseConfig(); - config.setDelay(500); // half second between each showcase view - this.setConfig(config); - this.singleUse(sequenceID); // Display tutorial only once - } -} diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyNoificationCardView.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyNoificationCardView.java index c9f0e0ff2..09e15081d 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyNoificationCardView.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyNoificationCardView.java @@ -1,14 +1,10 @@ package fr.free.nrw.commons.nearby; import android.content.Context; -import android.content.Intent; -import android.content.res.Resources; -import android.os.Handler; + import android.support.annotation.NonNull; import android.support.annotation.Nullable; -import android.support.v7.app.AlertDialog; import android.util.AttributeSet; -import android.view.MotionEvent; import android.view.View; import android.widget.Button; import android.widget.ImageView; @@ -40,8 +36,6 @@ public class NearbyNoificationCardView extends SwipableCardView { public PermissionType permissionType; - float x1,x2; - public NearbyNoificationCardView(@NonNull Context context) { super(context); this.context = context; @@ -63,6 +57,9 @@ public class NearbyNoificationCardView extends SwipableCardView { init(); } + /** + * Initializes views and action listeners + */ private void init() { View rootView = inflate(context, R.layout.nearby_card_view, this); @@ -105,88 +102,15 @@ public class NearbyNoificationCardView extends SwipableCardView { } /** - * Sets permission request button visible and content layout invisible, then adds correct - * permission request actions to permission request button according to PermissionType enum - * @param isPermissionRequestButtonNeeded true if permissions missing + * Time is up, data for card view is not ready, so do not display it */ - public void displayPermissionRequestButton(boolean isPermissionRequestButtonNeeded) { - if (isPermissionRequestButtonNeeded) { - cardViewVisibilityState = CardViewVisibilityState.ASK_PERMISSION; - contentLayout.setVisibility(GONE); - permissionRequestButton.setVisibility(VISIBLE); - - if (permissionType == PermissionType.ENABLE_LOCATION_PERMISSON) { - - permissionRequestButton.setOnClickListener(new OnClickListener() { - @Override - public void onClick(View view) { - if (!((MainActivity)context).isFinishing()) { - ((MainActivity) context).locationManager.requestPermissions((MainActivity) context); - } - } - }); - - } else if (permissionType == PermissionType.ENABLE_GPS) { - - permissionRequestButton.setOnClickListener(new OnClickListener() { - @Override - public void onClick(View view) { - new AlertDialog.Builder(context) - .setMessage(R.string.gps_disabled) - .setCancelable(false) - .setPositiveButton(R.string.enable_gps, - (dialog, id) -> { - Intent callGPSSettingIntent = new Intent( - android.provider.Settings.ACTION_LOCATION_SOURCE_SETTINGS); - Timber.d("Loaded settings page"); - ((MainActivity) context).startActivityForResult(callGPSSettingIntent, 1); - }) - .setNegativeButton(R.string.menu_cancel_upload, (dialog, id) -> { - dialog.cancel(); - displayPermissionRequestButton(true); - }) - .create() - .show(); - } - }); - } - - - } else { - cardViewVisibilityState = CardViewVisibilityState.LOADING; - /*permissionRequestButton.setVisibility(GONE); - contentLayout.setVisibility(VISIBLE); - // Set visibility of elements in content layout once it become visible - progressBar.setVisibility(VISIBLE); - notificationTitle.setVisibility(GONE); - notificationDistance.setVisibility(GONE); - notificationIcon.setVisibility(GONE); - - permissionRequestButton.setVisibility(GONE);*/ - - this.setVisibility(GONE); - Handler nearbyNotificationHandler = new Handler(); - Runnable nearbyNotificationRunnable = new Runnable() { - @Override - public void run() { - if (cardViewVisibilityState != NearbyNoificationCardView.CardViewVisibilityState.READY - && cardViewVisibilityState != NearbyNoificationCardView.CardViewVisibilityState.ASK_PERMISSION - && cardViewVisibilityState != NearbyNoificationCardView.CardViewVisibilityState.INVISIBLE) { - // If after 30 seconds, card view is not ready - errorOcured(); - } else { - suceeded(); - } - } - }; - nearbyNotificationHandler.postDelayed(nearbyNotificationRunnable, 30000); - } - } - private void errorOcured() { this.setVisibility(GONE); } + /** + * Data for card view is ready, display card view + */ private void suceeded() { this.setVisibility(VISIBLE); } diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java index 74af35de5..903a93839 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java +++ b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java @@ -32,6 +32,10 @@ public class NearbyPlaces { private final String wikidataQuery; public double radius = INITIAL_RADIUS; + /** + * Reads Wikidata query to check nearby wikidata items which needs picture, with a circular + * search. As a point is center of a circle with a radius will be set later. + */ public NearbyPlaces() { try { wikidataQuery = FileUtils.readFromResource("/queries/nearby_query.rq"); @@ -41,6 +45,20 @@ public class NearbyPlaces { } } + /** + * Returns list of nearby places around curLatLng or closest result near curLatLng. This decision + * is made according to returnClosestResult variable. If returnClosestResult true, then our + * number of min results set to 1, and max radius to search is set to 5. If there is no place + * in 5 km radius, empty list will be returned. This method sets radius variable according to + * search type (returnClosestResult or not), but search operation will be handled in overloaded + * method below, which is called from this method. + * @param curLatLng Our reference location + * @param lang Language we will display results in + * @param returnClosestResult If true, return only first result found, else found satisfactory + * number of results + * @return List of nearby places around, or closest nearby place + * @throws IOException + */ List getFromWikidataQuery(LatLng curLatLng, String lang, boolean returnClosestResult) throws IOException { List places = Collections.emptyList(); @@ -81,6 +99,16 @@ public class NearbyPlaces { return places; } + /** + * Creates and sends query for nearby wikidata items needs picture. + * Reads results from query and turns them into meaningful place variables. + * Adds them to the list of places. + * @param cur Our reference location to check around + * @param lang Language we will display results in + * @param radius Our query area is a circle, where cur is center and radius is radius + * @return + * @throws IOException + */ private List getFromWikidataQuery(LatLng cur, String lang, double radius) diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NoPermissionsFragment.java b/app/src/main/java/fr/free/nrw/commons/nearby/NoPermissionsFragment.java deleted file mode 100644 index f08fa6acd..000000000 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NoPermissionsFragment.java +++ /dev/null @@ -1,38 +0,0 @@ -package fr.free.nrw.commons.nearby; - -import android.content.Context; -import android.os.Bundle; -import android.support.v4.app.Fragment; -import android.view.LayoutInflater; -import android.view.View; -import android.view.ViewGroup; - -import butterknife.ButterKnife; -import dagger.android.support.AndroidSupportInjection; -import fr.free.nrw.commons.R; -import timber.log.Timber; - -/** - * Tells user that Nearby Places cannot be displayed if location permissions are denied - */ -public class NoPermissionsFragment extends Fragment { - - public NoPermissionsFragment() { - } - - @Override - public void onAttach(Context context) { - AndroidSupportInjection.inject(this); - super.onAttach(context); - } - - @Override - public View onCreateView(LayoutInflater inflater, ViewGroup container, - Bundle savedInstanceState) { - Timber.d("NoPermissionsFragment created"); - View view = inflater.inflate(R.layout.fragment_no_permissions, container, false); - ButterKnife.bind(this, view); - return view; - } - -}