search

fossasia / open-event-droidgen

Open Event Android App Generator https://github.com/fossasia/open-event-android/raw/apk/sample-apk-fossasia17-development.apk
GNU General Public License v3.0
2.06k stars 828 forks source link

fix #2363: Fix undo button not updating bookmark icon #2365

Closed pulakdp closed 6 years ago

pulakdp commented 6 years ago

Fixes #2363

Changes: Added method onBookmarksChanged() that gets called when a bookmark is added or removed. Refactored code that loads speaker details to avoid code repetition. Created method loadData() for that purpose.

dr0pdb commented 6 years ago

@pulakdp Please atttach a GIF showing that it indeed solves the issue.

pulakdp commented 6 years ago

Sure @srv-twry. Here is the gif after the fix

pulakdp commented 6 years ago

@iamareebjamal Added the TODO, have a look

pulakdp commented 6 years ago

Moved the TODO

pulakdp commented 6 years ago

Apologies! Moved TODO inside onResume where loadData is called @iamareebjamal

pulakdp commented 6 years ago

@iamareebjamal Kindly review and let me know if any further changes are needed

iamareebjamal commented 6 years ago

@pulakdp The solution still consists of eventbus pattern

pulakdp commented 6 years ago

We can accept these changes. But write a TODO, that this is a workaround and fix needs to be done in the implementation of RealmLiveData

Isn't this why we added a TODO @iamareebjamal? I understand this isn't what you want in future but your earlier instruction was to include a TODO so I did likewise. If a solution with RealmLiveData is expected at the moment then I'll have to learn about LiveData more before I come up with the solution.

iamareebjamal commented 6 years ago

The TODO is for the hook in onResume, event bus is deprecated, but I guess we can go ahead with it. Add another above the subscribe event

TODO: High Priority: Remove event bus pattern

pulakdp commented 6 years ago

@iamareebjamal done.