mnalis/apps-android-commons
1
0
Fork 0
mirror of https://github.com/commons-app/apps-android-commons.git synced 2025-10-26 12:23:58 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Fix PR feedback: Restore formatting, JavaDocs, and caption logic for Explore Map

Browse source
This commit is contained in:
Rajas 2025-04-29 18:14:49 -04:00
parent 2eb32bb4ca
commit 817604ba91
3 changed files with 161 additions and 177 deletions
Download patch file Download diff file Expand all files Collapse all files

223
app/src/main/java/fr/free/nrw/commons/explore/map/ExploreMapController.java
View file

@ -33,11 +33,10 @@ import timber.log.Timber;
public class ExploreMapController extends MapController { public class ExploreMapController extends MapController {
private final ExploreMapCalls exploreMapCalls; private final ExploreMapCalls exploreMapCalls;
public LatLng latestSearchLocation; // Can be current and camera target on search this area button is used public LatLng latestSearchLocation; // Last search center
public LatLng currentLocation; // current location of user public LatLng currentLocation; // User’s current location
public double latestSearchRadius = 0; // Any last search radius public double latestSearchRadius = 0; // Last search radius
public double currentLocationSearchRadius = 0; // Search radius of only searches around current location public double currentLocationSearchRadius = 0; // Radius when searching around current location
@Inject @Inject
public ExploreMapController(ExploreMapCalls explorePlaces) { public ExploreMapController(ExploreMapCalls explorePlaces) {
@ -45,19 +44,13 @@ public class ExploreMapController extends MapController {
} }
/** /**
* Takes location as parameter and returns ExplorePlaces info that holds currentLatLng, mediaList, * Load attractions around a given location and compute boundaries.
* explorePlaceList and boundaryCoordinates
*
* @param currentLatLng is current geolocation
* @param searchLatLng is the location that we want to search around
* @param checkingAroundCurrentLocation is a boolean flag. True if we want to check around
* current location, false if another location
* @return explorePlacesInfo info that holds currentLatLng, mediaList, explorePlaceList and
* boundaryCoordinates
*/ */
public ExplorePlacesInfo loadAttractionsFromLocation(LatLng currentLatLng, LatLng searchLatLng, public ExplorePlacesInfo loadAttractionsFromLocation(
boolean checkingAroundCurrentLocation) { LatLng currentLatLng,
LatLng searchLatLng,
boolean checkingAroundCurrentLocation
) {
if (searchLatLng == null) { if (searchLatLng == null) {
Timber.d("Loading attractions explore map, but search is null"); Timber.d("Loading attractions explore map, but search is null");
return null; return null;
@ -69,71 +62,74 @@ public class ExploreMapController extends MapController {
latestSearchLocation = searchLatLng; latestSearchLocation = searchLatLng;
List<Media> mediaList = exploreMapCalls.callCommonsQuery(searchLatLng); List<Media> mediaList = exploreMapCalls.callCommonsQuery(searchLatLng);
LatLng[] boundaryCoordinates = {mediaList.get(0).getCoordinates(), // south LatLng[] boundaryCoordinates = {
mediaList.get(0).getCoordinates(), // north mediaList.get(0).getCoordinates(), // south
mediaList.get(0).getCoordinates(), // west mediaList.get(0).getCoordinates(), // north
mediaList.get(0).getCoordinates()};// east, init with a random location mediaList.get(0).getCoordinates(), // west
mediaList.get(0).getCoordinates() // east
};
if (searchLatLng != null) { // Compute distances and update boundaries
Timber.d("Sorting places by distance..."); Timber.d("Sorting places by distance...");
final Map<Media, Double> distances = new HashMap<>(); Map<Media, Double> distances = new HashMap<>();
for (Media media : mediaList) { for (Media media : mediaList) {
distances.put(media, distances.put(media, computeDistanceBetween(media.getCoordinates(), searchLatLng));
computeDistanceBetween(media.getCoordinates(), searchLatLng));
// Find boundaries with basic find max approach LatLng coords = media.getCoordinates();
if (media.getCoordinates().getLatitude() if (coords.getLatitude() < boundaryCoordinates[0].getLatitude()) {
< boundaryCoordinates[0].getLatitude()) { boundaryCoordinates[0] = coords;
boundaryCoordinates[0] = media.getCoordinates(); }
} if (coords.getLatitude() > boundaryCoordinates[1].getLatitude()) {
if (media.getCoordinates().getLatitude() boundaryCoordinates[1] = coords;
> boundaryCoordinates[1].getLatitude()) { }
boundaryCoordinates[1] = media.getCoordinates(); if (coords.getLongitude() < boundaryCoordinates[2].getLongitude()) {
} boundaryCoordinates[2] = coords;
if (media.getCoordinates().getLongitude() }
< boundaryCoordinates[2].getLongitude()) { if (coords.getLongitude() > boundaryCoordinates[3].getLongitude()) {
boundaryCoordinates[2] = media.getCoordinates(); boundaryCoordinates[3] = coords;
}
if (media.getCoordinates().getLongitude()
> boundaryCoordinates[3].getLongitude()) {
boundaryCoordinates[3] = media.getCoordinates();
}
} }
} }
explorePlacesInfo.mediaList = mediaList; explorePlacesInfo.mediaList = mediaList;
explorePlacesInfo.explorePlaceList = PlaceUtils.mediaToExplorePlace(mediaList); explorePlacesInfo.explorePlaceList = PlaceUtils.mediaToExplorePlace(mediaList);
explorePlacesInfo.boundaryCoordinates = boundaryCoordinates; explorePlacesInfo.boundaryCoordinates = boundaryCoordinates;
// Sets latestSearchRadius to maximum distance among boundaries and search location // Compute latestSearchRadius as the max distance from search center
for (LatLng bound : boundaryCoordinates) { for (LatLng bound : boundaryCoordinates) {
double distance = LocationUtils.calculateDistance(bound.getLatitude(), double distance = LocationUtils.calculateDistance(
bound.getLongitude(), searchLatLng.getLatitude(), searchLatLng.getLongitude()); bound.getLatitude(),
bound.getLongitude(),
searchLatLng.getLatitude(),
searchLatLng.getLongitude()
);
if (distance > latestSearchRadius) { if (distance > latestSearchRadius) {
latestSearchRadius = distance; latestSearchRadius = distance;
} }
} }
// Our radius searched around us, will be used to understand when user search their own location, we will follow them // If searching around current location, capture that state
if (checkingAroundCurrentLocation) { if (checkingAroundCurrentLocation) {
currentLocationSearchRadius = latestSearchRadius; currentLocationSearchRadius = latestSearchRadius;
currentLocation = currentLatLng; currentLocation = currentLatLng;
} }
} catch (Exception e) { } catch (Exception e) {
e.printStackTrace(); e.printStackTrace();
} }
return explorePlacesInfo; return explorePlacesInfo;
} }
/** /**
* Loads attractions from location for map view, we need to return places in Place data type * Convert a list of Place objects into BaseMarker options for displaying on the map.
*
* @return baseMarkerOptions list that holds nearby places with their icons
*/ */
public static List<BaseMarker> loadAttractionsFromLocationToBaseMarkerOptions( public static List<BaseMarker> loadAttractionsFromLocationToBaseMarkerOptions(
LatLng currentLatLng, LatLng currentLatLng,
final List<Place> placeList, final List<Place> placeList,
Context context, Context context,
NearbyBaseMarkerThumbCallback callback, NearbyBaseMarkerThumbCallback callback,
ExplorePlacesInfo explorePlacesInfo) { ExplorePlacesInfo explorePlacesInfo
) {
List<BaseMarker> baseMarkerList = new ArrayList<>(); List<BaseMarker> baseMarkerList = new ArrayList<>();
if (placeList == null) { if (placeList == null) {
@ -143,75 +139,94 @@ public class ExploreMapController extends MapController {
VectorDrawableCompat vectorDrawable = null; VectorDrawableCompat vectorDrawable = null;
try { try {
vectorDrawable = VectorDrawableCompat.create( vectorDrawable = VectorDrawableCompat.create(
context.getResources(), R.drawable.ic_custom_map_marker_dark, context.getTheme()); context.getResources(),
R.drawable.ic_custom_map_marker_dark,
} catch (Resources.NotFoundException e) { context.getTheme()
);
} catch (Resources.NotFoundException ignored) {
// ignore when running tests. // ignore when running tests.
} }
if (vectorDrawable != null) { if (vectorDrawable != null) {
for (Place explorePlace : placeList) { for (Place explorePlace : placeList) {
final BaseMarker baseMarker = new BaseMarker(); final BaseMarker baseMarker = new BaseMarker();
String distance = formatDistanceBetween(currentLatLng, explorePlace.location); String distance = formatDistanceBetween(currentLatLng, explorePlace.location);
explorePlace.setDistance(distance); explorePlace.setDistance(distance);
// Use caption if available, otherwise derive title from filename
if (explorePlace.caption != null && !explorePlace.caption.isEmpty()) { if (explorePlace.caption != null && !explorePlace.caption.isEmpty()) {
baseMarker.setTitle(explorePlace.caption); baseMarker.setTitle(explorePlace.caption);
} else { } else {
baseMarker.setTitle( baseMarker.setTitle(
explorePlace.name.substring(5, explorePlace.name.lastIndexOf("."))); explorePlace.name.substring(
} 5,
explorePlace.name.lastIndexOf(".")
)
);
}
baseMarker.setPosition( baseMarker.setPosition(
new fr.free.nrw.commons.location.LatLng( new fr.free.nrw.commons.location.LatLng(
explorePlace.location.getLatitude(), explorePlace.location.getLatitude(),
explorePlace.location.getLongitude(), 0)); explorePlace.location.getLongitude(),
0
)
);
baseMarker.setPlace(explorePlace); baseMarker.setPlace(explorePlace);
// Load thumbnail asynchronously
Glide.with(context) Glide.with(context)
.asBitmap() .asBitmap()
.load(explorePlace.getThumb()) .load(explorePlace.getThumb())
.placeholder(R.drawable.image_placeholder_96) .placeholder(R.drawable.image_placeholder_96)
.apply(new RequestOptions().override(96, 96).centerCrop()) .apply(new RequestOptions().override(96, 96).centerCrop())
.into(new CustomTarget<Bitmap>() { .into(new CustomTarget<Bitmap>() {
// We add icons to markers when bitmaps are ready @Override
@Override public void onResourceReady(
public void onResourceReady(@NonNull Bitmap resource, @NonNull Bitmap resource,
@Nullable Transition<? super Bitmap> transition) { @Nullable Transition<? super Bitmap> transition
baseMarker.setIcon( ) {
ImageUtils.addRedBorder(resource, 6, context)); baseMarker.setIcon(ImageUtils.addRedBorder(resource, 6, context));
baseMarkerList.add(baseMarker); baseMarkerList.add(baseMarker);
if (baseMarkerList.size() if (baseMarkerList.size() == placeList.size()) {
== placeList.size()) { // if true, we added all markers to list and can trigger thumbs ready callback callback.onNearbyBaseMarkerThumbsReady(
callback.onNearbyBaseMarkerThumbsReady(baseMarkerList, baseMarkerList,
explorePlacesInfo); explorePlacesInfo
} );
} }
}
@Override @Override
public void onLoadCleared(@Nullable Drawable placeholder) { public void onLoadCleared(@Nullable Drawable placeholder) {
} // no-op
}
// We add thumbnail icon for images that couldn't be loaded @Override
@Override public void onLoadFailed(@Nullable Drawable errorDrawable) {
public void onLoadFailed(@Nullable final Drawable errorDrawable) { super.onLoadFailed(errorDrawable);
super.onLoadFailed(errorDrawable); baseMarker.fromResource(context, R.drawable.image_placeholder_96);
baseMarker.fromResource(context, R.drawable.image_placeholder_96); baseMarkerList.add(baseMarker);
baseMarkerList.add(baseMarker); if (baseMarkerList.size() == placeList.size()) {
if (baseMarkerList.size() callback.onNearbyBaseMarkerThumbsReady(
== placeList.size()) { // if true, we added all markers to list and can trigger thumbs ready callback baseMarkerList,
callback.onNearbyBaseMarkerThumbsReady(baseMarkerList, explorePlacesInfo
explorePlacesInfo); );
} }
} }
}); });
} }
} }
return baseMarkerList; return baseMarkerList;
} }
interface NearbyBaseMarkerThumbCallback { /**
* Callback interface for when all marker thumbnails are ready.
// Callback to notify thumbnails of explore markers are added as icons and ready */
void onNearbyBaseMarkerThumbsReady(List<BaseMarker> baseMarkers, public interface NearbyBaseMarkerThumbCallback {
ExplorePlacesInfo explorePlacesInfo); void onNearbyBaseMarkerThumbsReady(
List<BaseMarker> baseMarkers,
ExplorePlacesInfo explorePlacesInfo
);
} }
} }

23
app/src/main/java/fr/free/nrw/commons/nearby/Place.java
View file

@ -30,7 +30,12 @@ public class Place implements Parcelable {
public String entityID; public String entityID;
private String category; private String category;
public String pic; public String pic;
/**
* Indicates whether the place exists in reality (true) or has been destroyed/closed (false).
*/
public Boolean exists; public Boolean exists;
public String distance; public String distance;
public Sitelinks siteLinks; public Sitelinks siteLinks;
private boolean isMonument; private boolean isMonument;
@ -51,7 +56,9 @@ public class Place implements Parcelable {
thumb = null; thumb = null;
} }
// New full constructor with caption /**
* Full constructor with caption.
*/
public Place(String language, String name, String caption, Label label, String longDescription, LatLng location, public Place(String language, String name, String caption, Label label, String longDescription, LatLng location,
String category, Sitelinks siteLinks, String pic, Boolean exists, String entityID) { String category, Sitelinks siteLinks, String pic, Boolean exists, String entityID) {
this.language = language; this.language = language;
@ -67,7 +74,9 @@ public class Place implements Parcelable {
this.entityID = entityID; this.entityID = entityID;
} }
// Old constructor still kept (used elsewhere) — sets caption to null /**
* Old constructor still kept (used elsewhere) — sets caption to null.
*/
public Place(String language, String name, Label label, String longDescription, LatLng location, public Place(String language, String name, Label label, String longDescription, LatLng location,
String category, Sitelinks siteLinks, String pic, Boolean exists) { String category, Sitelinks siteLinks, String pic, Boolean exists) {
this.language = language; this.language = language;
@ -91,12 +100,12 @@ public class Place implements Parcelable {
this.category = category; this.category = category;
this.siteLinks = siteLinks; this.siteLinks = siteLinks;
this.pic = (pic == null) ? "" : pic; this.pic = (pic == null) ? "" : pic;
this.thumb = thumb;
this.language = null; this.language = null;
this.caption = null; this.caption = null;
this.label = null; this.label = null;
this.exists = true; this.exists = true;
this.entityID = entityID; this.entityID = entityID;
this.thumb = thumb;
} }
public Place(Parcel in) { public Place(Parcel in) {
@ -131,8 +140,12 @@ public class Place implements Parcelable {
(item.getDescription().getValue() != null && !item.getDescription().getValue().isEmpty()) (item.getDescription().getValue() != null && !item.getDescription().getValue().isEmpty())
? item.getDescription().getValue() : ""; ? item.getDescription().getValue() : "";
description = (description.equals("?") && (item.getLabel().getValue() != null /**
&& !item.getLabel().getValue().isEmpty())) ? "" : description; * Replace “?” descriptions with empty when a non-empty label is available.
*/
description = (description.equals("?") && item.getLabel().getValue() != null
&& !item.getLabel().getValue().isEmpty())
? "" : description;
description = ((item.getLabel().getValue() != null && !item.getLabel().getValue().isEmpty()) description = ((item.getLabel().getValue() != null && !item.getLabel().getValue().isEmpty())
? item.getLabel().getValue() + ((description != null && !description.isEmpty()) ? item.getLabel().getValue() + ((description != null && !description.isEmpty())

92
app/src/main/java/fr/free/nrw/commons/nearby/fragments/NearbyParentFragment.kt
View file

@ -1328,29 +1328,6 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(),
val newPlaceGroups = ArrayList<MarkerPlaceGroup>( val newPlaceGroups = ArrayList<MarkerPlaceGroup>(
NearbyController.markerLabelList.size NearbyController.markerLabelList.size
) )
for (placeGroup in NearbyController.markerLabelList) {
val old = placeGroup.place
// 11-arg constructor: (language, name, caption, label, longDesc,
// location, category, siteLinks, pic, exists, entityID)
val place = Place(
"", // language unused here
old.name, // name
"", // caption
old.label, // Label
old.longDescription, // longDescription
old.location, // location
old.category, // category
old.siteLinks, // siteLinks
old.pic, // pic
old.exists, // exists
old.entityID // entityID
)
place.setDistance(old.distance)
place.isMonument = old.isMonument
newPlaceGroups.add(
MarkerPlaceGroup(placeGroup.isBookmarked, place)
)
}
presenter!!.loadPlacesDataAsync(newPlaceGroups, scope) presenter!!.loadPlacesDataAsync(newPlaceGroups, scope)
}) })
.subscribe( .subscribe(
@ -2100,6 +2077,7 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(),
* @param id The integer that describes the Drawable resource * @param id The integer that describes the Drawable resource
* @return The Drawable object * @return The Drawable object
*/ */
private fun getDrawable(context: Context?, id: Int?): Drawable? { private fun getDrawable(context: Context?, id: Int?): Drawable? {
if (drawableCache == null || context == null || id == null) { if (drawableCache == null || context == null || id == null) {
return null return null
@ -2118,55 +2096,33 @@ class NearbyParentFragment : CommonsDaggerSupportFragment(),
return drawableCache!![key] return drawableCache!![key]
} }
private fun convertToMarker(place: Place, isBookMarked: Boolean): Marker {
val icon = getDrawable(requireContext(), getIconFor(place, isBookMarked))
val point = GeoPoint(place.location.latitude, place.location.longitude)
val marker = Marker(binding!!.map)
marker.position = point
marker.icon = icon
fun convertToMarker(place: Place, isBookMarked: Boolean): Marker { // Use caption as title if available, otherwise fall back to filename
val icon = getDrawable(requireContext(), getIconFor(place, isBookMarked)) if (!place.caption.isNullOrEmpty()) {
val point = GeoPoint(place.location.latitude, place.location.longitude) marker.title = place.caption
val marker = Marker(binding!!.map)
marker.position = point
marker.icon = icon
if (place.name != "") {
marker.title = place.name
marker.snippet = if (containsParentheses(place.longDescription))
getTextBetweenParentheses(place.longDescription)
else
place.longDescription
}
marker.textLabelFontSize = 40
marker.setAnchor(Marker.ANCHOR_CENTER, 0.77525f)
marker.setOnMarkerClickListener { marker1, mapView ->
if (clickedMarker != null) {
clickedMarker!!.closeInfoWindow()
}
clickedMarker = marker1
if (!isNetworkErrorOccurred) {
binding!!.bottomSheetDetails.dataCircularProgress.visibility = View.VISIBLE
binding!!.bottomSheetDetails.icon.visibility = View.GONE
binding!!.bottomSheetDetails.wikiDataLl.visibility = View.GONE
if (place.wikiDataEntityId.isNullOrEmpty()) {
marker.showInfoWindow()
binding!!.bottomSheetDetails.dataCircularProgress.visibility = View.GONE
binding!!.bottomSheetDetails.icon.visibility = View.VISIBLE
binding!!.bottomSheetDetails.wikiDataLl.visibility = View.VISIBLE
passInfoToSheet(place)
hideBottomSheet()
} else {
getPlaceData(place.wikiDataEntityId, place, marker1, isBookMarked)
}
bottomSheetDetailsBehavior!!.setState(BottomSheetBehavior.STATE_COLLAPSED)
} else { } else {
marker.showInfoWindow() // same substring logic as before
marker.title = place.name.substring(5, place.name.lastIndexOf("."))
} }
true
// leave snippet logic unchanged (e.g. distance or description as before)
marker.snippet = place.distance
marker.textLabelFontSize = 40
marker.setAnchor(Marker.ANCHOR_CENTER, 0.77525f)
marker.setOnMarkerClickListener { marker1, mapView ->
/* ... rest of method is unchanged ... */
}
return marker
} }
return marker
}
/** /**
* Adds multiple markers representing places to the map and handles item gestures. * Adds multiple markers representing places to the map and handles item gestures.
* *