-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
Implement direct uploads for Nearby Map #1060
Implement direct uploads for Nearby Map #1060
Conversation
…loads' into nearby-uploads
…loads' into nearby-uploads
…loads' into nearby-uploads # Conflicts: # app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java
…c for Nearby Uploads
@psh @maskaravivek I've refactored the PR to use Dagger injections for saving the title/desc, please let me know if the injections are done correctly (I'm very new to Dagger, haha). I must say that using Dagger really cleans up all the shuffling of SharedPreferences between fragments and activities, which is very nice. I've tested this and it works for me on API 25 and 21 emulators. But please let me know if you're still getting the NPEs @neslihanturan . |
Thanks @misaochan :) |
Hope that fixes it! |
I couldn't reproduce the error. I think it is fixed and all uploads were successful:) Great job @misaochan . I will just wait @psh and @maskaravivek review the changes before merging. |
Any thoughts on the Dagger additions? Would be good if we could merge this soon, so that @neslihanturan can start working on real-time user positions in the Nearby map with minimal conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dagger usage looks perfectly fine to me. I would like to have it merged. :)
I decided to submit this PR after getting the Nearby Map direct uploads for #252 sorted, and will work on the Nearby List direct uploads in a separate PR. This is aimed at allowing @neslihanturan to start work on updating the user's real-time position in the Nearby Map once this PR has been merged, while minimizing conflicts. I have tested this on an emulator running API 25.
What this PR does:
What this PR does NOT cover (and will thus need to be handled in our future PRs):