Skip to content

Fix auto zoom issue #3391 Zoom level gets reset at every second. #3564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 26, 2020
Merged

Fix auto zoom issue #3391 Zoom level gets reset at every second. #3564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5eba2b3
Add method to check if curr location marker is vsible or not
neslihanturan Mar 21, 2020
8c18255
Recenter map if users see their current location marker
neslihanturan Mar 21, 2020
f6b9b0d
Add new methods to Contract
neslihanturan Mar 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/src/main/java/fr/free/nrw/commons/nearby/contract/NearbyParentFragmentContract.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ interface View {
LatLng getLastLocation();

com.mapbox.mapboxsdk.geometry.LatLng getLastFocusLocation();

boolean isCurrentLocationMarkerVisible();
void setProjectorLatLngBounds();
}

interface NearbyListView {
Expand Down
31 changes: 20 additions & 11 deletions app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import com.mapbox.mapboxsdk.camera.CameraPosition;
import com.mapbox.mapboxsdk.camera.CameraUpdateFactory;
import com.mapbox.mapboxsdk.geometry.LatLng;
import com.mapbox.mapboxsdk.geometry.LatLngBounds;
import com.mapbox.mapboxsdk.maps.MapView;
import com.mapbox.mapboxsdk.maps.MapboxMap;
import com.mapbox.mapboxsdk.maps.Style;
Expand Down Expand Up @@ -201,6 +202,7 @@ public class NearbyParentFragment extends CommonsDaggerSupportFragment
private boolean isVisibleToUser;
private MapboxMap.OnCameraMoveListener cameraMoveListener;
private fr.free.nrw.commons.location.LatLng lastFocusLocation;
private LatLngBounds latLngBounds;


@Override
Expand Down Expand Up @@ -632,6 +634,22 @@ public LatLng getLastFocusLocation() {
return lastFocusLocation==null?null:LocationUtils.commonsLatLngToMapBoxLatLng(lastFocusLocation);
}

@Override
public boolean isCurrentLocationMarkerVisible() {
if (latLngBounds == null) {
Timber.d("Map projection bounds are null");
return false;
} else {
Timber.d("Current location marker %s" , latLngBounds.contains(currentLocationMarker.getPosition()) ? "visible" : "invisible");
return latLngBounds.contains(currentLocationMarker.getPosition());
}
}

@Override
public void setProjectorLatLngBounds() {
latLngBounds = mapBox.getProjection().getVisibleRegion().latLngBounds;
}

@Override
public boolean isNetworkConnectionEstablished() {
return NetworkUtils.isInternetConnectionEstablished(getActivity());
Expand Down Expand Up @@ -906,9 +924,8 @@ public void displayLoginSkippedWarning() {
}

private void handleLocationUpdate(fr.free.nrw.commons.location.LatLng latLng, LocationServiceManager.LocationChangeType locationChangeType){
setMapBoxPosition(latLng);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we removed this intentionally @neslihanturan ?

Copy link
Collaborator Author

@neslihanturan neslihanturan Mar 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ashishkumar468 , yes it was intentional. As recenter method already does a similar effect and I can't recognize any other function of having it since there is no JavaDocs either. Even if we will need it, we should have it inside presenter.updateNearbyMap method, nowhere else.

Can you think of any issues that will occur with lack of this method @ashishkumar468 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ashishkumar468 , can you re-view your review:)

this.lastKnownLocation=latLng;
NearbyController.currentLocation=lastKnownLocation;
this.lastKnownLocation = latLng;
NearbyController.currentLocation = lastKnownLocation;
presenter.updateMapAndList(locationChangeType);
}

Expand Down Expand Up @@ -941,14 +958,6 @@ public void onLocationChangedMedium(fr.free.nrw.commons.location.LatLng latLng)
}
}

void setMapBoxPosition(fr.free.nrw.commons.location.LatLng latLng){
CameraPosition position = new CameraPosition.Builder()
.target(LocationUtils.commonsLatLngToMapBoxLatLng(latLng)) // Sets the new camera position
.zoom(ZOOM_LEVEL) // Same zoom level
.build();
mapBox.moveCamera(CameraUpdateFactory.newCameraPosition(position));
}

public void backButtonClicked() {
presenter.backButtonClicked();
}
Expand Down
7 changes: 2 additions & 5 deletions app/src/main/java/fr/free/nrw/commons/nearby/presenter/NearbyParentFragmentPresenter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
import java.util.HashMap;
import java.util.List;

import javax.inject.Inject;

import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao;
import fr.free.nrw.commons.kvstore.JsonKvStore;
import fr.free.nrw.commons.location.LatLng;
Expand All @@ -21,9 +19,7 @@
import fr.free.nrw.commons.nearby.NearbyBaseMarker;
import fr.free.nrw.commons.nearby.NearbyController;
import fr.free.nrw.commons.nearby.NearbyFilterState;
import fr.free.nrw.commons.nearby.Place;
import fr.free.nrw.commons.nearby.contract.NearbyParentFragmentContract;
import fr.free.nrw.commons.upload.UploadContract;
import fr.free.nrw.commons.utils.LocationUtils;
import fr.free.nrw.commons.wikidata.WikidataEditListener;
import timber.log.Timber;
Expand Down Expand Up @@ -194,7 +190,7 @@ public void updateMapAndList(LocationServiceManager.LocationChangeType locationC
nearbyParentFragmentView.populatePlaces(nearbyParentFragmentView.getCameraTarget());
} else { // Means location changed slightly, ie user is walking or driving.
Timber.d("Means location changed slightly");
if (!nearbyParentFragmentView.isSearchThisAreaButtonVisible()) { // Do not track users position if the user is checking around
if (nearbyParentFragmentView.isCurrentLocationMarkerVisible()){ // Means user wants to see their live location
nearbyParentFragmentView.recenterMap(curLatLng);
}
}
Expand Down Expand Up @@ -259,6 +255,7 @@ public void onLocationChangedMedium(LatLng latLng) {

@Override
public void onCameraMove(com.mapbox.mapboxsdk.geometry.LatLng latLng) {
nearbyParentFragmentView.setProjectorLatLngBounds();
// If our nearby markers are calculated at least once
if (NearbyController.latestSearchLocation != null) {
double distance =latLng.distanceTo
Expand Down