-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Search this area #2051
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
Search this area #2051
Conversation
… to search nearby areas.
…modular method and decrease code repetition
… on operation is done
… to previous position
…t follow users location with camera, if search this area mode is on
Codecov Report
@@ Coverage Diff @@
## master #2051 +/- ##
=========================================
- Coverage 4.05% 4.02% -0.04%
=========================================
Files 225 226 +1
Lines 11357 11467 +110
Branches 1048 1064 +16
=========================================
+ Hits 460 461 +1
- Misses 10863 10972 +109
Partials 34 34
Continue to review full report at Codecov.
|
@@ -312,6 +319,33 @@ private void refreshView(LocationServiceManager.LocationChangeType locationChang | |||
} | |||
} | |||
|
|||
public void refreshViewForCustomLocation(LatLng customLatLng) { |
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.
Could we get Javadocs for these new methods, please?
* on the gap between list sheet and tab layout. | ||
* @param isBottomListSheetExpanded | ||
*/ | ||
private void updateMapCameraAccordingToBottomSheet(boolean isBottomListSheetExpanded) { |
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.
Codacy reports that this method is unused, could you double check? "Avoid unused private methods such as 'updateMapCameraAccordingToBottomSheet(boolean)'."
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.
Yes actually this method got obsolete after fix for #2018 .Although it was a doing a good job adjusting the job based on some calculations for adjusting map zoom level based on bottom sheet's state, but yes can be removed now.
@neslihanturan I just tested this on an API 26 Samsung Galaxy s7. "Search this area" works very well for me in general, great job! I like that it maintains the zoom level, too. A couple of issues:
Solution: aside from the initial load, I do not think that the user's position should be treated as special. Meaning that when I load Nearby, then yes, automatically search around my location and display a few pins (not 600!). if I search 100km away, then tap the center button, do not initiate a search around my location automatically. Rather, display the "Search this area" button as normal. |
Actually loading points for your real location when you come back wasn't a bug, it was a feature:D I thought both recenter button and manual recentering should load real nearby pins automatically. But we can switch to manual version as you suggested. When it comes to pin numbers and distances, You are right sorry I will take care |
Are fixed, but since I changed a lot this should be tested carefully. Possible test cases:
|
…h other to continue other operations
…for no reason while around of current location is already loaded
…rent location of custom location
…ton when we are out of 3/4 of total searched area
Ready to be tested @misaochan :) |
This works well for me, thanks! :) The only issue now is that I found that rotating the screen makes the map auto-refresh, and then it NEVER loads. Not even when rotating it back to portrait, or after going to Contributions and back to Nearby. I will merge this PR first, please fix this in your next PR. Logs below:
|
Description (required)
Fixes #591 "Search this area" to launch a nearby search around that location
What changes did you make and why?
Tests performed (required)
Tested betaDebug, on API19 and 26
Screenshots showing what changed (optional - for UI changes)