"Edit location" is hard to use (and confusing) #5713
Assignees
Labels
Comments
|
Can I be assigned this issue? I already have a pull request which hopefully resolves issue #5704 and I am familiar with the code that is probably causing the rest of the issues. |
|
@mnalis I'm having trouble reproducing the third problem. When I follow the steps you provided, the map will center near the EXIF location, not the center of the map. I believe this is also happening in the video you uploaded. I recommend trying to move the map a large distance (a kilometer) away from the pin before tapping the blue "Edit Location" button. Does the problem still occur? Thank you. |
You are correct, when current GPS location is several kilometers away from EXIF location, clicking "Edit Location" does seem to move the map towards (or at least very near) EXIF location. I'm eagerly awaiting next release to test out your fixes @Jason-Whitmore, so I'll do more check then to confirm if everything seems fine! |
nicolas-raoul
added a commit
that referenced
this issue
Sep 18, 2024
) * LocationPickerActivity.java: fix "Show in Map App" bug Once the "Show in Map App" button is pressed, the Map app will center on the current photo's EXIF location, if that data is available. If not, the map app will center on where the location picker's map is centered. Javadoc updated to reflect this small change. * LocationPickerActivity.java: add methods to easily move the map center Before this change, any time the map center had to move, several lines of code had to be written. This change creates several methods which move the map center to a specified location. By using these new methods, moving the map center only takes one simple method call. * LocationPickerActivity.java: add null check to method The original method did not include a null check for the input GeoPoint. This change includes a null check. If the input Geopoint is null, the method will simply return. * LocationPickerActivity.java: remove redundant method, renaming method Before this change, there were two methods with the same behavior. This change removes the newer method and renames the old method to the descriptive name the newer one had. Additionally, code which calls this method has been changed to reflect the new name. * LocationPickerActivity.java: rearrange method calls. Before this change, a method call to move the map to the device's GPS location was called at the very end of the map setup method. This would move the map away from the media's EXIF location data (if it was available). This change places the method call to move the map to the device's GPS location before the method call to move the map to the media's EXIF location (if the data is available). In short, if no exif data is available, the map attempts to move to the device's GPS location. If the exif location data does exist, the map will move to that location. * LocationPickerActivity.java: rewrite method scope, name, and documentation Before this commit, the method name was unclear and the documentation did not fully explain the method's behavior. Additionally, it was a public method. This commit renames the method (also changing calls to it), adds more documentation, and changes the method scope from public to private. * LocationPickerActivity.java: create method to move location picker map to device GPS location Before this method was created, several lines of code were required to move the location picker map to the device's GPS location. After this method was created, the location picker map can be moved to the GPS location in a single method call. * LocationPickerActivity.java: add code to move map to either EXIF or device GPS location Before this change, there was no explicit if check on whether there was EXIF data available before moving the location picker map. After this change, an explicit if check (which checks the activity name string) will move the location picker map to either the EXIF location (if that data is available) or to the device's GPS location (if EXIF data is not available). * LocationPickerActivity.java: refactor showInMapApp method Before this change, the showInMapApp method had correct behavior, but could be rewritten to become more clear. After this change, the showInMapApp code has been rewritten. Specifically, common code in an if statement has been factored out. * LocationPickerActivity.java: add null checks to showInMapApp Before this change, there was no null checks when accessing data from an object, which could create null pointer exceptions at runtime. After this change, null checks are introduced to make sure null pointer exceptions will not occur. * LocationPickerActivity.java: rewrite comments to clarify if statement Before this change, a comment in showInMapApp did not properly describe an if statement's intended behavior. After this change, the old comment was removed and 2 new comments were added in each part of the if statement to properly describe the intended behavior. * LocationPickerActivity.java: replace code with a method call Before this change, there was several lines of code which moved the map center to the EXIF location. After this change, the several lines of code have been replaced with a simpler method call which produces the same result. * LocationPickerActivity.java: remove redundant code Before this change, there was two sections of code which moved the map center to the same location. Both of these code sections occur very close to each other (one in onCreate(), the other in setupMapView()) After this change, the code section in onCreate() has been removed. With the map centering code only in the setupMapView() method. This change eliminates redundancy, reduces the amount of code in onCreate(), and makes this java file easier to understand. * content_location_picker.xml: adjust picker pin and shadow location Before this commit, the location picker pin and shadow graphics were incorrectly located on the screen. Specifically, the bottom of the pin was not located at the center of the view. Since pressing the checkmark button saves the location at the center of the view, users would most likely save the wrong location. After this commit, the location picker pin and shadow graphics have been moved upward. The shadow graphic is now located at the exact center of the view, and the bottom of the location picker pin is directly over the center as well. Users may now use the bottom of the pin as an accurate location picker. * LocationPickerActivity.java: fix bug with permissions menu moving map Prior to this change, if the menu asking for permissions to access the device's GPS was accepted, the map would automatically move to the most recent or current GPS location, rather than staying at the uploaded image's available EXIF location. After this change, the map will stay centered at the image's available EXIF location, as intended. The relevant method name and javadoc have been changed to more accurately describe the method's behavior. * LocationPickerActivity.java: fix map centering bug when editing location on already uploaded media Before this commit, when the user pressed the edit location icon on media which was already uploaded, the map would center on the device's current GPS location. After this commit, pressing the same edit location icon will now center the map on the location metadata. This is done by checking the activity string to see if the media is already uploaded. If so, a method is called to center the map on the media's location metadata. * LocationPickerActivity.java: replace references to EXIF in documentation Before this commit, there were several mentions of EXIF location data in the javadocs. It is not clear if this is correct, since many images have location data that is chosen by the user rather than derived from EXIF. After this commit, the more generic term "location metadata" is used in place of EXIF location data. Hopefully this change will help avoid confusion in the future. --------- Co-authored-by: Nicolas Raoul <nicolas.raoul@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Feedback
When geotagged picture is being uploaded, and "Edit location" button is clicked, there are several usability problems, see this video:
small_Screen_Recording_20240504_214622_Commons.mp4
00:00:02on opening; the map is positioned on user's current position, and not to the location of GPS EXIF Tags in the picture which would make sense (that one is already being tracked in Upload Wizard: "Edit location" shows my location instead of picture's location #5704)00:00:10even when user manually navigates to picture location and presses and pressesShow in map app, the location selected in the other app (OsmAnd in this case) is not the location of the GPS EXIF tags from the picture, but currently viewed map center (which is going to be off, even if user tried to carefully position it in the middle). It should instead always position on exact location contained in the GPS EXIF tags.00:00:34when users pressesEdit location, the map jump from EXIF GPS location to current center of the map, thus losing the original picture position. It should instead start from the EXIF GPS position and only move at explicit user click to move it somewhere else; as it is likely that the user only wants to move it slightly from location recorded in EXIF GPS tags (as the location of the subject of the picture is usually not the exactly same as the location of the phone, as you usually take picture of object at several (or several dozen) meters away.Wiki username
Mnalis
Device name
Samsung Galaxy S23+
Android version
Android 14 (OneUI 6.1)
Commons app version
5.0.1~af028cbdd (latest F-droid)
The text was updated successfully, but these errors were encountered: