"Search this area" to launch a nearby search around that location

[nicolas-raoul]

I often plan my itinerary and decide where to go, before actually being there.

On the Nearby map, it would be great if I could long-press a location to launch a nearby search around there.

[oe-bayram]

We would be glad if we can take this issue.

[neslihanturan]

I consider this enhancement as a good one. However, its scope needs to be defined (and discussed) better before implementation.

1. Are we planning to update nearby list accordingly?
   • My opinion: Yes, we should.
2. How does user reset her location back?
   • My opinion: There should be a button to reset map view according to original location.

After adding #252 (This can be discussed under #252 )

1. Should we let user upload for mock locations?
   • My opinion: Actually this could give opportunity to upload nearby photos when location off, and would be great.

[misaochan]

I agree, the scope of this is very vague at the moment.

[misaochan]

To answer your questions:

1. Yes
2. Agreed
3. I'm not so sure about this.

In fact even implementing this enhancement in and of itself could potentially increase the risk of vandalism via direct uploads IMO, since someone would not even need to be in a location (or to spoof his location) in order to upload a picture for that Nearby item. When P18 edits are implemented, the consequences of this could be even more severe, as the Wikidata items will actually be edited and removed from Nearby.

That being said, maybe I'm being a bit too catastrophic, lol.

[neslihanturan]

In fact even implementing this enhancement in and of itself could potentially increase the risk of vandalism via direct uploads IMO, since someone would not even need to be in a location (or to spoof his location) in order to upload a picture for that Nearby item.

I was thinking for my DSLR camera uploads. Since I never took photos with my android device, I am never able to upload a nearby place while I am there. But I share your vandalism concern.

[misaochan]

Edit: Yeah, I suppose it's possible that some users may want to take all their photos and then upload them to the various Nearby items later. So I can see the benefits in allowing that. Just not sure if it's worth the tradeoff?

Edit 2: A possible solution: We check the geotagged location of the image and allow the direct upload to go through if the location is roughly close to the location of the Nearby item? Thoughts on this?

[nicolas-raoul]

I am never able to upload a nearby place while I am there

I want to second that. I upload most of my pictures on the train back, or when I am waiting for a friend for instance.
Checking the picture's coordinates is a great idea indeed, to put a strong warning or even forbid the upload.
In any case it would be great if the UI (itself or via a web link to the wiki) could educate the users about:

• how to setup their camera to write EXIF info
• what constitute a good Wikidata P18 image

[thealexguy]

We'd like to focus on 1. and maybe 2. because we're limited in scope and time due to the fact we're implementing this feature in the context of a course at our university. So these two subissues and the correspondent tests totally fullfill the limited scope and time of our course
We also think that the nearby list should be in sync with the map and we're following the discussion about mocking the location.

@oe-bayram and @thealexguy

[thealexguy]

Do you think it's possible to implement 1. and 2. isolated and irrespective of 3. ?

[nicolas-raoul]

@thealexguy Yes feel free to implement only 1 and 2 :-)

[misaochan]

@thealexguy Sure, please feel free. :)

[thealexguy]

Just one more question:
which log in do i have to use when debugging the app and developing the new feature?
Is there any default developing log in ? My normal user data doesn't work...
Thanks alot!
[I've already asked it in the mailing list but i haven't got any reply yet.]

[misaochan]

There is no default developer login unfortunately.

1. What build variant are you running?
2. What login did you try initially (is it your Commons username and password?)

[thealexguy]

I've pulled the latest version and used my normal login username and password. I registered again and now it works :)

[misaochan]

@thealexguy Happy to hear that. :) You can always check what build variant you are using (if you are using Android Studio, it should be on the left side of the screen). The build variants that include the term "beta" (e.g. betaDebug) will upload images to the beta server (instead of the real Commons server) and AFAIK that requires a separate login/registration from the actual Commons one.

[thealexguy]

Everything of the following is open for discussion :)

We discussed and have some questions so far:

• What is the difference between LatLng from mapbox and your own implementation of LatLng (fr.free.nrw.commons.location.LatLng)? Which one should we use ?

• Should we use the same radius as now right now ? (we would use the same, we just wanted to get that confirmed)

We also like to (and have to due to the course rules) propose a solution approach:

Function "add custom location via long press on map":

1. Add Marker to MapView and show NearbyPlaces as markers (in the NearbyMapFragment.java Class)

• 1.1 Handle Longpress Event and add new red marker

  • a) remove all markers on map

  • b) Add marker

  • c) Store custom-marked Location globally

• 1.2 Query to WIKI service with custom marker as parameter (use nearbyAsyncTask)

• 1.3 Use Return of Query to add markers to Map

  • a) Use current way to add the markers (loadAttractionsFromLocationToBaseMarkerOptions)

2. Update ListView accordingly

• 2.1 Use return of query to show listView

Function "button to set back to users current postion"

There already is an icon with the “reload the nearby places around the current location”- functionality. So we propose to change the icon which clarifies the current location of the user, e.g. this one (https://commons.wikimedia.org/wiki/File:Simple_icon_location.svg).
Of course the icon can be discussed.

We are also working on tests. Even it seems like there is no test yet for the current-loction-marker functionality in the NearbyMapFragment.class

PS: i hope we don't "overdo" things ;)

[oe-bayram]

In addition to the tasks above we want (and have) to write one (or more) test cases that captures the intended behavior of the feature.

Well as you can read it here, Android doesn’t contain any actual code when running unit tests. “This is to make sure your unit tests only test your code and do not depend on any particular behaviour of the Android platform”.

A possible way would be to mock some classes and methods but this wouldn’t make much sense because the feature is per se an interaction of Android classes (View) and the Mapbox library (MapView and MapboxMap).

So, do you have any suggestions how we could write a test case for this feature?
Did you have similar problems when trying to test something and did you find any solution approaches (that were costly for you to try) we could try?
Or did you just do the tests manually?

[misaochan]

Hi @oe-bayram and @thealexguy - apologies for the slow response, we have been pretty busy recently, 2 of our devs were travelling last weekend and we are gearing up for a release this week. Do you two have any strict deadlines for your project? We will try to answer you within the week if possible.

(Edited to fix my wonky temporal perception)

[thealexguy]

Hi @misaochan - the goal of our project is more learning the way how to contribute to an opensource software than perfectly implementing a feature. So we have a deadline but it does not have to be strictly adhered to. Of Course a fast reply would be nice.
Currently we have a working implementation following the solution approach above. If the approach is not well enough, we will rewrite our implementation. As soon as you confirm our solution approach, we will create a pull request.

Our project leader focuses more on tests, so we would be glad if you reply to this topic as well. So we could implement them as well.

[misaochan]

Hi @thealexguy , trying to go through your questions now, let me know if I miss anything.

What is the difference between LatLng from mapbox and your own implementation of LatLng (fr.free.nrw.commons.location.LatLng)? Which one should we use ?

I haven't had time to take a close look at the Mapbox documentation yet, but unless there is a good reason to do otherwise, I would use our own implementation, as there is a possibility that we may not continue to use Mapbox in the future.

Should we use the same radius as now right now ? (we would use the same, we just wanted to get that confirmed)

Yes, that should be OK.

We also like to (and have to due to the course rules) propose a solution approach:

The proposed approach sounds good to me. What do you think @nicolas-raoul @neslihanturan @maskaravivek ?

There already is an icon with the “reload the nearby places around the current location”- functionality. So we propose to change the icon which clarifies the current location of the user, e.g. this one

I believe this would make sense for the map, but not the list?

So, do you have any suggestions how we could write a test case for this feature?
Did you have similar problems when trying to test something and did you find any solution approaches (that were costly for you to try) we could try?
Or did you just do the tests manually?

Does looking at NearbyAdapterFactoryTest.java help? Unfortunately, I am not entirely sure how to answer this question, hopefully someone else can. :)

[nicolas-raoul]

Sounds good! Manual tests are probably enough, I would say, as it is primarily a UI feature. Good luck :-)

[oe-bayram]

Feature Description

Id: #591
Name: Long-press on map to launch a nearby search around that location

Functionality:

This Feature is an addition to the current feature of showing nearby Places around the current location of the user. Now it’s possible to add a custom location anywhere on the map via a long press. After a few seconds of loading the nearby places around the custom location are shown on the map.

API:

Basically we catch a long press event on the MapFragment and call the new static method refreshSelectedLocationView(LatLng custLatLang) in NearbyActivity with the pressed location as input. In this new method we do the same as it is right now with the current location but we use the custom Location instead.

Refactoring and new methods:

We refactored curLatLng variable in NearbyActivity and named it more generic selectedLocationLatLng. We also added a method called refreshSelectedLocationView(LatLng custLatLang).
We changed the name of the method refreshView to refreshLatestLocationView() due to the new generic functionality. So the name still shows what the function does even with the new feature.
Furthermore we replaced the refresh icon to ic_location_searching_white_24dp and changed the icon id accordingly.

[oe-bayram]

Dear Team,
we want to create a new branch from the master branch and transfer the changes to this new branch but it seems like that we don't have the permission to create a new branch in this project. Could you please give us the permission to do this (or maybe you could create the new branch yourself), so we can create a pull request?

@thealexguy and @oe-bayram

[maskaravivek]

@oe-bayram You could fork the repo and push changes to your branch, and submit a PR then. :)

[oe-bayram]

Due to the last version update our implementation for this feature isn't compatible anymore. Well the current version has some location issues in the nearby activity (#983). And this is being fixed with this merge.

Anyway I have tried to implement the feature with this merge and after a bit change in the nearby activity it works again. But now after a long click, the last position is shown just for a half second and then the long clicked position is shown with its nearby places (with a short white screen inbetween).

Our questions: When will this be merged and what could be the reason for this short display of the last shown position?

The implementation is on this forked repository.

[misaochan]

Hi @oe-bayram , sorry, we have been quite busy with solving the issues blocking our current release. Testing and merging PRs should hopefully resume next week after we clear the blockers, and hopefully we can try to look into the problems you are experiencing then as well, if still needed.

[nicolas-raoul]

When I go nearby-hunting, I only take pictures and walk/cycle.
I upload later when I have time.
Being able to upload from the nearby map is great, but when I am back home the points are out of the map.
A workaround is to use a fake GPS app, but it is very cumbersome.
This feature makes make the operation very natural.

[neslihanturan]

I am even more old-fashion @nicolas-raoul , writing down places to a paper since I took photos with DSLR, not phone. So I also think this feature is quite needed.

[misaochan]

I agree. Sorry we didn't follow up on this, @oe-bayram . :/ Are you guys still interested in working on this?

[misaochan]

I think we probably want to bump this up a bit, possibly include it in our next grant (WMCZ or PG). After using our direct uploads quite a bit, I agree completely with @nicolas-raoul that this feature is needed.

Some thoughts:

• How about instead of long-press, when the user moves to a different section of the map, we have a "Search this area" button that appears? That would be more intuitive to users I think, and they also get to control the radius. However, we do need to think about implementation and how we would define "different section of the map".

• IMO, when the user uploads to a point that isn't near his location, we definitely want to check the coordinates of the image being uploaded and at least pop up a warning if it is empty or does not match the coordinates of the point (within reasonable tolerance of course). Given that the upload will modify the P18 property, perhaps we might even forbid direct uploads entirely in that case, with a message that the coordinates must match, or that the image has no coordinates (and how to geotag images with their camera).

• Alternatively, if the Feedback GSoC task has been completed by then, perhaps we could even restrict direct uploading in a remote location to users with certain number of uploads and % of reverts?

[nicolas-raoul]

"Search this area" is better indeed, and it is found in apps like AirBNB

Yes, totally, making "adding a no-GPS picture via the nearby map at a remote location" a feature that can be "unlocked" (see #85 (comment)) is a great idea. Until "reputation" is implemented, though, I think we should not worry too much about vandals, who have much more accessible targets.

[misaochan]

Good point. Yeah, I think we can just use a warning until we implement that. :)

[misaochan]

Had a conversation with User:Jim Henderson at https://commons.wikimedia.org/wiki/Commons_talk:Mobile_app#Wish_list , and a concern of his (that I agree with) is that the map frequently auto-zooms out to display an unusable dense cluster of pins, which requires the user to keep zooming back in, especially if they are switching between apps.

When we implement this feature, we should ensure that we save the state of the map (the area that the user chose to search) when onPause or onStop. We should only zoom out to the default view onDestroy, IMO.

[nicolas-raoul]

"the map frequently auto-zooms out to display an unusable dense cluster of pins" <- very true.
How about creating a new issue about this?

[misaochan]

I figured I would include it in this issue because the zoom conditions will inherently have to change in the development of this feature - and this feature is due to start being developed within the next 1-2 weeks. Do you think it still warrants a separate issue?

[nicolas-raoul]

This problem happens as soon as the Nearby screen appears, before even zooming or moving the map, right?
The present issue is only applicable after that.
I believe the implementation of one does not interfere with the other.

[misaochan]

@nicolas-raoul Good point. I will create an issue.