-
Notifications
You must be signed in to change notification settings - Fork 3
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
1214: Add pois to favorites #1461
Conversation
a860d16
to
ef16ad2
Compare
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.
Looks good. Nice job 🚀
Tested on Android, works as expected 👍
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 work 👍
There is one scenario we should catch.
- User adds a store to favorites.
- Later the store will be deleted because it doesn't participate anymore. (i removed from database to test)
Current behavior
The store is still in favorites. That ok but when i click on the store, the user is lost in this error screenshot. Retry doesn't help
Currently there is no proper error handling for this case. We should show in this case probably another error message and provide a way to go back
3bb5959
to
a60cc40
Compare
@f1sh1918 I think I would need your help to reproduce that.
Currently I'm not quite sure how we can achieve this result, because in order to display a store item in the favorites list we need data about this store (name, description, category). But from where we could get it, if not from the database? UPD: I also wonder, if you reproduce this scenario, do you get the same error if you open the removed store details on the “Search” page? |
UPD:
|
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.
Short description
Add the ability to save POIs as favorites on device
Proposed changes
Not included:
Maybe we could separate this part?
It's taking a while, because I'm not quite sure how to do it yet, if we don't make “Favorites” as a category.
Or I might need some help with that.
UPD. As discussed in the channel, we will implement that separately.
Side effects
Couldn't find any
Not tested on iOS
Resolved issues
Fixes: #1214