Skip to content
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

Jump to bottom

Nearby not working as intended #5461

Closed
RitikaPahwa4444 opened this issue Jan 21, 2024 · 14 comments
Closed

Nearby not working as intended #5461

RitikaPahwa4444 opened this issue Jan 21, 2024 · 14 comments
Assignees
@ShashwatKedia
Labels
bug nearby

Comments

@RitikaPahwa4444
Copy link
Collaborator

RitikaPahwa4444 commented Jan 21, 2024 •
edited

Summary

Nearby used to display pins on the map. They are no longer visible. So, I can't initiate any uploads from Nearby. Moreover, the following features are also not working:

  • Search this area button
  • The list button at the top right corner

Moreover, the app often crashes or stops responding while trying the above features.

Steps to reproduce

  1. Open the app and switch to Nearby tab
  2. Wait for the pins to load or navigate to other areas to search for them using "Search this area" button
  3. Tap the list icon at the top right corner

Expected behaviour

The nearby pins should load, "Search this area" should load pins on other areas on Earth and tapping the list icon should show a list of nearby places.

Actual behaviour

Nearby pins do not load, nor do the "Search this area" button or the list icon work.

Device name

Redmi 5A

Android version

8.1.0

Commons app version

main and prodDebug

Device logs

No response

Screen-shots

No response

Would you like to work on the issue?

None
@nicolas-raoul
Copy link
Member

Interesting, this is working fine on my Android 14 device.
Anyone who wants to work on this, please first make sure you can reproduce, using an emulator if needed.

@RitikaPahwa4444
Copy link
Collaborator Author

nearby.mp4

This has been partly fixed by PR#5418 , I'll just highlight the differences with the app available on Play Store.

@ShashwatKedia
Copy link
Contributor

@RitikaPahwa4444 the problem here is that the app is asking for location permission but does not ask to turn on the location services (which it should do directly after permission is given, if they are off). I reported this in another issue as well, #5255 (comment). Does this happen even when the location services are on?

@RitikaPahwa4444
Copy link
Collaborator Author

RitikaPahwa4444 commented Jan 21, 2024 •
edited

This issue is a bit different from the one highlighted in that comment. That issue used to exist, I agree. I'll test for that too and post my observations there. For this issue, I'll share for all the cases I can think of.

@ShashwatKedia
Copy link
Contributor

This issue is a bit different from the one highlighted in that comment.

I think you are right, it's not dependant on whether the location services are on/off, because for my device it's working irrespective of the state of location services. I'll try to reproduce this on some other device / emulator

@RitikaPahwa4444 RitikaPahwa4444 mentioned this issue Jan 21, 2024
Open
@RitikaPahwa4444
Copy link
Collaborator Author

Differences:

Case App on Play Store (Oct 2023 version) main branch PR#5418
App does not have location access + location (GPS) turned off Pins visible. Search this area button and list icon are working. No crash on target icon. Screencast Pins not visible. Search this area button and list icon are not working. No crash on target icon. Screencast Pins not visible. Search this area button and list icon are not working. No crash on target icon. Screencast
App has location access + location (GPS) turned off Pins visible. Search this area button and list icon are working. No crash on target icon. Screencast Pins not visible. Search this area button and list icon are not working. Crash on target icon. Screencast Pins not visible. Search this area button and list icon are not working. No crash on target icon. Pins appear after using the target icon. Screencast

There might be a few more, I'll share if I find them.

@RitikaPahwa4444 RitikaPahwa4444 mentioned this issue Jan 21, 2024
Closed
@rohit9625 rohit9625 mentioned this issue Jan 23, 2024
Closed
@ShashwatKedia ShashwatKedia mentioned this issue Jan 27, 2024
Closed
@ShashwatKedia
Copy link
Contributor

@RitikaPahwa4444 After a lot of digging, I found that in the cases you mentioned, the onclicklistener for SearchThisArea button was not being set, so no operations were being performed. I'll fix this and send a PR soon, to resolve this.

@sivaraam
Copy link
Member

@RitikaPahwa4444 After a lot of digging, I found that in the cases you mentioned, the onclicklistener for SearchThisArea button was not being set, so no operations were being performed. I'll fix this and send a PR soon, to resolve this.

Great. Thanks for taking the time to figure this out, @ShashwatKedia 🙂

@ShashwatKedia ShashwatKedia mentioned this issue Jan 28, 2024
Closed
@sivaraam sivaraam mentioned this issue Jan 28, 2024
Closed
@ShashwatKedia ShashwatKedia mentioned this issue Jan 29, 2024
Merged
@ShashwatKedia
Copy link
Contributor

@RitikaPahwa4444 could you confirm that the above PR works on your device as well, since you were the one who found this issue out ;)

@RitikaPahwa4444 RitikaPahwa4444 mentioned this issue Jan 30, 2024
Closed
@RitikaPahwa4444
Copy link
Collaborator Author

Hi @ShashwatKedia, thanks for the PR. I'm testing that branch, working great so far :)

@nicolas-raoul
Copy link
Member

@RitikaPahwa4444 As the original reporter, do you think the PR is worth merging? :-)

@RitikaPahwa4444
Copy link
Collaborator Author

I'd tested that branch over a month ago and approved too, since it fixed this issue back then (I can test again if required) and we agreed on opening a separate ticket for a minor popup related issue (#5549).

@sivaraam
Copy link
Member

@RitikaPahwa4444 As the original reporter, do you think the PR is worth merging? :-)

I could understand the PR has been long standing. If you're asking this since you plan on merging this, could you wait for the last two comments to be resolved? We could take the rest as fixes on top, as needed

sivaraam added a commit that referenced this issue Apr 17, 2024
@ShashwatKedia @sivaraam
Fixes Location related flow of the app #5256 , #5461, #5490 (#5494)
04f9ef4 
* Resolved merged conflicts

* Resolved merge conflicts and updated workflow

* Updated Location Flow and merged conflicts

* Update flow and merge conflicts

* Fixed LocationPicker's location flow

* Removed redundant code from  LocationPermissionsHelper

* Fixed Explore fragment crashing and incorrect behaviour

* Updated LocationPicker Flow

* Fixed Nearby not working as intended

* Final update to location flow of all maps

* Added the reqested changes and fixed bugs

* Resolved requested change in in-app camera location flow

* Fixed In-app camera location flow

* Resolved conflicts in ContributionsListFragment

* Updated java doc as requested

* Resolved nearby card dialog not being shown

* Optimised LocationPermissionsHelper javadoc

* Made requested changes for preference check

* Added javadoc and requested comment for later reference

* Implemented requested code changes

* Fixed failing test due to changes made during PR

* Added string resource for ExploreMapFragment

* Changed string resource for rationale dialog

* Added standard location flow information in LocationPermissionsHelper

* Added javadoc for doNotAskForLocationPermission

* Removed unused import

* Updated javadoc

* Removed values-yue-hant

* Fix some merge conflict errors

* Fix minor errors due to mergre conflicts

* Fix some refactor errors

* Fixed minor bug due to merging conflicts

* Delete app/src/main/res/values-yue-hant directory

* Final changes to NearbyParentFragment

* Fixes #5686 map coordinates set to image coords

* Removed some redundant code from recenterMap

* Removed one test whose method no longer exists

* Removed unused method from contract of nearby

* Removed redundant method from NearbyParentFragment

* nearby: add a FIXME about the possibly redudant code

---------

Co-authored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
vtalos pushed a commit to karyotakisg/apps-android-commons that referenced this issue Apr 21, 2024
@karyotakisg @vtalos
Add AlertDialog for categories and modularize receiveSharedItems
21d3816 
Improve nearby-place search function for a multi-upload

Enhance the depiction consistency of a multi-upload by ensuring that it corresponds to a single place

Add javadoc

Fixes Location related flow of the app commons-app#5256 , commons-app#5461, commons-app#5490 (commons-app#5494)

* Resolved merged conflicts

* Resolved merge conflicts and updated workflow

* Updated Location Flow and merged conflicts

* Update flow and merge conflicts

* Fixed LocationPicker's location flow

* Removed redundant code from  LocationPermissionsHelper

* Fixed Explore fragment crashing and incorrect behaviour

* Updated LocationPicker Flow

* Fixed Nearby not working as intended

* Final update to location flow of all maps

* Added the reqested changes and fixed bugs

* Resolved requested change in in-app camera location flow

* Fixed In-app camera location flow

* Resolved conflicts in ContributionsListFragment

* Updated java doc as requested

* Resolved nearby card dialog not being shown

* Optimised LocationPermissionsHelper javadoc

* Made requested changes for preference check

* Added javadoc and requested comment for later reference

* Implemented requested code changes

* Fixed failing test due to changes made during PR

* Added string resource for ExploreMapFragment

* Changed string resource for rationale dialog

* Added standard location flow information in LocationPermissionsHelper

* Added javadoc for doNotAskForLocationPermission

* Removed unused import

* Updated javadoc

* Removed values-yue-hant

* Fix some merge conflict errors

* Fix minor errors due to mergre conflicts

* Fix some refactor errors

* Fixed minor bug due to merging conflicts

* Delete app/src/main/res/values-yue-hant directory

* Final changes to NearbyParentFragment

* Fixes commons-app#5686 map coordinates set to image coords

* Removed some redundant code from recenterMap

* Removed one test whose method no longer exists

* Removed unused method from contract of nearby

* Removed redundant method from NearbyParentFragment

* nearby: add a FIXME about the possibly redudant code

---------

Co-authored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>

Localisation updates from https://translatewiki.net.

Remove the value-yue-hant file

The file is not properly recognized by Android and we've actually
codemapped it to yue. The translatewiki configuration has been
done in the incorrect file. So, it is still being created.

The following Gerrit change would correct it for updates after that
change is merged.

  https://gerrit.wikimedia.org/r/c/translatewiki/+/1022508

For the time being remove it for the sake of release.
@sivaraam
Copy link
Member

Closing this as #5494 has been merged.

Feel free to reopen if I missed something that's left to be done. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ShashwatKedia ShashwatKedia

Labels
bug nearby
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@nicolas-raoul @sivaraam @RitikaPahwa4444 @ShashwatKedia