-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fix errors pop ups in map page #354
Conversation
…n there is no location
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.
Good job, I see that there are less problems while manually testing on my phone. I think you should add regression tests however.
The nearby posts posts page still throws an error after toggling the location, and it never reloads correctly. I have this problem on the emulator and my real phone.
location_toggle.webm
I think we should not show the current position marker in the my posts screen if we have location disabled, so as not to confuse the user. We could change the icon on the center button to show that we don't have location as well.
Thanks for comment/suggestions! I pushed a fix that should resolve the last issue. Concerning the UI part, I think that the custom user feedback on the map when location is turned off is out of scope for this PR. The goal of this PR is to have stable error system. The features that you propose are not bug fixes in my opinion. Don't get me wrong, I think that those suggested features are great and should be backlogged! It's just that I don't see them fit in this particular PR. |
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.
Good job fixing that last problem and adding tests.
I now found a new bug where the center button no longer works after toggling location. It works again after refresh of the map page so it's not too bad. I'll add it to the backlog with my suggestions for the my posts tab. This PR indeed already accomplishes its criteria and improves the app.
This PR fixes multiple issues linked to error pop ups in the map page. Here are the main fixs brought with this PR:
Mutiple pop ups : Error pop ups are now filtered in order to only display relevant one once (no more multiple "no location" errors).
Errors for user posts : Now, the map does't show an error when user checks the user posts tab with no location.
Errors when location is re-enabled: no more error pop up occur when the location is re-enabled by the user.
The last point DOES NOT WORK on the emulator. Please use a real phone to test this bug fix.
First relevant commit: 0280f65
Acceptance criteria: