Skip to content

Fixes #3720: None of the filters work on Nearby list #3723

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
May 4, 2020
Merged

Fixes #3720: None of the filters work on Nearby list #3723

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
859037f
Fixes #3720: None of the filters work on Nearby list
VitalyVPinchuk Apr 30, 2020
631db50
Fixed: Place state filter not applied when ALL at place type filter s…
VitalyVPinchuk May 3, 2020
e8af8e7
Merge remote-tracking branch 'remotes/upstream/master' into fix-3720
VitalyVPinchuk May 4, 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
13 changes: 13 additions & 0 deletions app/src/main/java/fr/free/nrw/commons/nearby/NearbyAdapterFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,17 @@ public void updateAdapterData(List<Place> newPlaceList, RVRendererAdapter<Place>
rendererAdapter.notifyDataSetChanged();
rendererAdapter.diffUpdate(newPlaceList);
}

public void clear(RVRendererAdapter<Place> rendererAdapter){
rendererAdapter.clear();
}

public void add(Place place, RVRendererAdapter<Place> rendererAdapter){
rendererAdapter.add(place);
}

public void update(RVRendererAdapter<Place> rendererAdapter){
rendererAdapter.notifyDataSetChanged();
}

}
110 changes: 54 additions & 56 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 @@ -628,6 +628,18 @@ public void updateListFragment(List<Place> placeList) {
adapterFactory.updateAdapterData(placeList, (RVRendererAdapter<Place>) rvNearbyList.getAdapter());
}

public void clearNearbyList() {
adapterFactory.clear((RVRendererAdapter<Place>) rvNearbyList.getAdapter());
}

public void updateNearbyList() {
adapterFactory.update((RVRendererAdapter<Place>) rvNearbyList.getAdapter());
}

public void addPlaceToNearbyList(Place place) {
adapterFactory.add(place, (RVRendererAdapter<Place>) rvNearbyList.getAdapter());
}

@Override
public fr.free.nrw.commons.location.LatLng getLastLocation() {
return lastKnownLocation;
Expand Down Expand Up @@ -1082,7 +1094,8 @@ public void updateMapMarkers(List<NearbyBaseMarker> nearbyBaseMarkers, Marker se

@Override
public void filterOutAllMarkers() {
hideAllMArkers();
hideAllMarkers();
updateNearbyList();
}

/**
Expand All @@ -1093,6 +1106,7 @@ public void displayAllMarkers() {
for (MarkerPlaceGroup markerPlaceGroup : NearbyController.markerLabelList) {
updateMarker(markerPlaceGroup.getIsBookmarked(), markerPlaceGroup.getPlace(), NearbyController.currentLocation);
}
updateNearbyList();
}

/**
Expand All @@ -1104,65 +1118,46 @@ public void displayAllMarkers() {
* @param filterForAllNoneType true if we filter places with all none button
*/
@Override
public void filterMarkersByLabels(List<Label> selectedLabels, boolean displayExists,
boolean displayNeedsPhoto,
boolean filterForPlaceState,
boolean filterForAllNoneType) {
if (selectedLabels.size() == 0 && filterForPlaceState) { // If nothing is selected, display all
// remove the previous markers before updating them
hideAllMArkers();
for (MarkerPlaceGroup markerPlaceGroup : NearbyController.markerLabelList) {
if (displayExists && displayNeedsPhoto) {
// Exists and needs photo
if (markerPlaceGroup.getPlace().destroyed.trim().isEmpty() && markerPlaceGroup.getPlace().pic.trim().isEmpty()) {
updateMarker(markerPlaceGroup.getIsBookmarked(), markerPlaceGroup.getPlace(), NearbyController.currentLocation);
}
} else if (displayExists && !displayNeedsPhoto) {
// Exists and all included needs and doesn't needs photo
if (markerPlaceGroup.getPlace().destroyed.trim().isEmpty()) {
updateMarker(markerPlaceGroup.getIsBookmarked(), markerPlaceGroup.getPlace(), NearbyController.currentLocation);
}
} else if (!displayExists && displayNeedsPhoto) {
// All and only needs photo
if (markerPlaceGroup.getPlace().pic.trim().isEmpty()) {
updateMarker(markerPlaceGroup.getIsBookmarked(), markerPlaceGroup.getPlace(), NearbyController.currentLocation);
}
} else if (!displayExists && !displayNeedsPhoto) {
// all
updateMarker(markerPlaceGroup.getIsBookmarked(), markerPlaceGroup.getPlace(), NearbyController.currentLocation);
}
public void filterMarkersByLabels(List<Label> selectedLabels,
boolean displayExists,
boolean displayNeedsPhoto,
boolean filterForPlaceState,
boolean filterForAllNoneType) {

// Remove the previous markers before updating them
hideAllMarkers();

for (MarkerPlaceGroup markerPlaceGroup : NearbyController.markerLabelList) {
Place place = markerPlaceGroup.getPlace();

// When label filter is engaged
// then compare it against place's label
if (selectedLabels != null && (selectedLabels.size() != 0 || !filterForPlaceState)
&& !selectedLabels.contains(place.getLabel())) {
continue;
}
} else {
// First remove all the markers
hideAllMArkers();
for (MarkerPlaceGroup markerPlaceGroup : NearbyController.markerLabelList) {
for (Label label : selectedLabels) {
if (markerPlaceGroup.getPlace().getLabel().toString().equals(label.toString())) {

if (displayExists && displayNeedsPhoto) {
// Exists and needs photo
if (markerPlaceGroup.getPlace().destroyed.trim().isEmpty() && markerPlaceGroup.getPlace().pic.trim().isEmpty()) {
updateMarker(markerPlaceGroup.getIsBookmarked(), markerPlaceGroup.getPlace(), NearbyController.currentLocation);
}
} else if (displayExists && !displayNeedsPhoto) {
// Exists and all included needs and doesn't needs photo
if (markerPlaceGroup.getPlace().destroyed.trim().isEmpty()) {
updateMarker(markerPlaceGroup.getIsBookmarked(), markerPlaceGroup.getPlace(), NearbyController.currentLocation);
}
} else if (!displayExists && displayNeedsPhoto) {
// All and only needs photo
if (markerPlaceGroup.getPlace().pic.trim().isEmpty()) {
updateMarker(markerPlaceGroup.getIsBookmarked(), markerPlaceGroup.getPlace(), NearbyController.currentLocation);
}
} else if (!displayExists && !displayNeedsPhoto) {
// all
updateMarker(markerPlaceGroup.getIsBookmarked(), markerPlaceGroup.getPlace(), NearbyController.currentLocation);
}
}

if (displayExists && displayNeedsPhoto) {
// Exists and needs photo
if (place.destroyed.trim().isEmpty() && place.pic.trim().isEmpty()) {
updateMarker(markerPlaceGroup.getIsBookmarked(), place, NearbyController.currentLocation);
}
} else if (displayExists && !displayNeedsPhoto) {
// Exists and all included needs and doesn't needs photo
if (place.destroyed.trim().isEmpty()) {
updateMarker(markerPlaceGroup.getIsBookmarked(), place, NearbyController.currentLocation);
}
} else if (!displayExists && displayNeedsPhoto) {
// All and only needs photo
if (place.pic.trim().isEmpty()) {
updateMarker(markerPlaceGroup.getIsBookmarked(), place, NearbyController.currentLocation);
}
} else if (!displayExists && !displayNeedsPhoto) {
// all
updateMarker(markerPlaceGroup.getIsBookmarked(), place, NearbyController.currentLocation);
}
}
updateNearbyList();
}

@Override
Expand All @@ -1177,6 +1172,8 @@ public fr.free.nrw.commons.location.LatLng getCameraTarget() {
* @param curLatLng current location
*/
public void updateMarker(boolean isBookmarked, Place place, @Nullable fr.free.nrw.commons.location.LatLng curLatLng) {
addPlaceToNearbyList(place);

VectorDrawableCompat vectorDrawable;
if (isBookmarked) {
vectorDrawable = VectorDrawableCompat.create(
Expand Down Expand Up @@ -1223,7 +1220,7 @@ public void updateMarker(boolean isBookmarked, Place place, @Nullable fr.free.nr
* but it is transparent more than grey(as the name of the icon might suggest)
* since grey icon may lead the users to believe that it is disabled or prohibited contribution
*/
private void hideAllMArkers() {
private void hideAllMarkers() {
VectorDrawableCompat vectorDrawable;
vectorDrawable = VectorDrawableCompat.create(
getContext().getResources(), R.drawable.ic_custom_greyed_out_marker, getContext().getTheme());
Expand All @@ -1234,6 +1231,7 @@ private void hideAllMArkers() {
}
}
addCurrentLocationMarker(NearbyController.currentLocation);
clearNearbyList();
}

private void addNearbyMarkersToMapBoxMap(List<NearbyBaseMarker> nearbyBaseMarkers, Marker selectedMarker) {
Expand Down
6 changes: 5 additions & 1 deletion 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 @@ -284,7 +284,11 @@ public void filterByMarkerType(List<Label> selectedLabels, int state, boolean fi
nearbyParentFragmentView.setRecyclerViewAdapterItemsGreyedOut();
break;
case CHECKED:
nearbyParentFragmentView.displayAllMarkers();
// Despite showing all labels NearbyFilterState still should be applied
nearbyParentFragmentView.filterMarkersByLabels(selectedLabels,
NearbyFilterState.getInstance().isExistsSelected(),
NearbyFilterState.getInstance().isNeedPhotoSelected(),
filterForPlaceState, false);
nearbyParentFragmentView.setRecyclerViewAdapterAllSelected();
break;
}
Expand Down