Add EventHub, add fav, reblog events, improve timelines #562

charlag · 2018-04-07T08:36:03Z

This is an experiment on better handling of events like fav, boost, reply.
Basically we need something like EventBus but

it is limited
I'm tired to see this verbose error handling across the project (onResponse ... if response.body ... onFailure ...)
I also consider LocalBroadcastManager a huge hack added for the sake of convenience

So my solution is RxJava.
How is this better than broadcasts? Well, imagine we want to update a thread after posting a reply (#552). We would have to serialize/deserialize status somehow or make extra network request. But we can pass anything through RxJava so we don't have such a problem/overhead.

I didn't touch any other parts (like replacing all Calls with Singles or getting rid of broadcasts) just yet but that's what I would like to do in some point in the future if everyone agrees.

I've also converted TimelineFragment to Kotlin because at one point it just became unreadable. Sorry about that.

This is not complete in any sense (doesn't handle boosted things e.g.), just wanted to hear your opinion on fixing some things with RxJava.

(to make it clear: in TimelineFragment I just added a couple of methods on the bottom, not much else)

connyduck · 2018-04-16T15:00:32Z

I like your initative on this <3 We definitely need something like this and might as well do it with RxJava.

Regarding your current code: when liking/boosting, notifyItemChanged(position) makes the item "flimmer" (you can see its updated), I really don't want this. (maybe its possible to tell RecyclerView to not play animation, but only updating the button is definitely preferable)

Please use Kotlin Android Extensions instead of findViewById

And can you merge master back in please?

I would really like it if you continue to work on this!

charlag · 2018-04-16T15:44:33Z

Thanks for review! 🙇
Yes, I agree, we should be able to disable animation in this case!
Indeed
I will probably work on this in May only, but we'll see! I will definitely rebase it.
Thanks a lot!

charlag · 2018-04-30T21:36:59Z

app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java

+                    } else if (event instanceof ReblogEvent) {
+                        handleReblogEvent((ReblogEvent) event);
+                    }
+                    if (event instanceof BlockEvent) {


oops, missing else here

charlag · 2018-04-30T22:03:26Z

Something is still wrong with boosted toots, needs more love, sigh

charlag · 2018-04-30T22:27:01Z

Updating threads after reply is buggy too.

charlag · 2018-05-01T20:05:22Z

I didn't find the bug with boosts bug I've fixed things with inserting new toots in threads and only in specific timelines. I'm not sure if we should insert "load more" in profiles because it doesn't seem to work there. Otherwise it should be good.

I've spent half a day trying to make Robolectric work but I've stumbled upon couple of bugs so I give up. It doesn't worth trying to make it work until we refactor logic into separate entity.
I am not sure if it's worth making things public and test them this way at least because everything is stateful.

charlag · 2018-05-02T20:58:04Z

It crashed for me when the app did unload and then i tooted

charlag · 2018-05-05T14:47:30Z

Will fix #471 #280

connyduck

This is awesome and works really well! Will continue testing.

When I post a status, it always shows a "load more" below the new status, but usually does not load anything when clicking there? It might be better to send a fetch timeline request instead of inserting the placeholder?

connyduck · 2018-05-06T12:37:27Z

app/src/main/java/com/keylesspalace/tusky/ComposeActivity.java

@@ -169,6 +170,8 @@
    public MastodonApi mastodonApi;
    @Inject
    public AccountManager accountManager;
+    @Inject


yes, more injecting 😎

connyduck · 2018-05-06T12:40:44Z

app/src/main/java/com/keylesspalace/tusky/AccountActivity.java

@@ -91,6 +94,8 @@
    public AccountManager accountManager;
    @Inject
    public DispatchingAndroidInjector<Fragment> dispatchingAndroidInjector;
+    @Inject
+    public AppStore appstore;


Is there a better name for this? It just reminds me so much of fdroid/google play 🙈

It's a Redux/Flux terminology to name such a thing Store. Can rename to GlobalStore or something

charlag · 2018-05-06T12:54:07Z

About load more: the way we do it now will make timeline move on fetch result and it's very annoying to lose your postiion in timeline when you reply to someone. Do you have any idea how to fix this?

connyduck · 2018-05-06T13:01:08Z

I think in TimelineAdapter.update we should not call notifyDataSetChanged but notifyItemRangeInserted, that should make the jumping go away?

charlag · 2018-05-06T13:05:29Z

But we have to calculate what we inserted then because we may not have loaded anything?
I propose usage of hasStableIds until we migrate to DiffUtils. Need to give placesholders IDs tho.

charlag · 2018-05-13T20:06:28Z

Anyway, @connyduck , if you insist we should actually refresh timeline on every post, I can try to make it work. It's kinda wasteful IMO but I see UX value in this.

connyduck · 2018-05-16T17:04:28Z

I do not think its wasteful, the UX improvement is huge.

charlag · 2018-05-20T19:11:50Z

I've made some progress on it.
I ended up re-doing part of timelines stuff with DiffUtil. I also think that when I'll finish with "load more" progress it will be better than now.

charlag · 2018-05-24T20:43:34Z

I've stumbled upon some super weird bug, adding spinner as the last item crashes app with inconsistency exception. Will look at it tomorrow

charlag · 2018-05-25T22:08:49Z

I've figured out the crash, it was a race condition because I used AsyncListDiffer incorrectly.
Now timelines work mostly correctly (I remove "load more" spinner in the wrong place). The real problem I have is that it gets into infinite loop on short lists because I changed the way InfiniteScrollListener (or whatever it's called) was only firing callbacks when amount of items increased. It caused unpleasant things (like if one timeline fetch failed we wouldn't load the rest). Now I fire them every time but I have to track when we stumbled upon bottom. Here is some problem, it doesn't work.

Fix infinite loading. Remove spinner correctly. Don't refresh timeline without need.

charlag · 2018-05-26T10:27:31Z

I think that's it. It's a much bigger PR than I wanted but IMO it feels much better.
I'm mostly concerned about criteria for stopping loading things on the bottom but I didn't find a better one. It seems to work just fine.
Please check @connyduck and everyone else ❤️

connyduck · 2018-05-27T07:52:04Z

Works like a charm and code is way cleaner - really awesome work

One thing that personally bugs me with timelines is, when pulling down to refresh, it always jumps to the top instead of just scrolling down a little bit, enough to see that something new has loaded. The Twitter app does this really good. But we should probably handle this in a new pull request?

charlag · 2018-05-27T07:57:48Z

Cool, thanks for the review
As discussed in a dedicated issue it is tricky to pick one option. I would love to handle this without any preferences and knobs. Probably keeping position would be the best but yeah, I think it's worth another PR, I tried to match current behavior. Keeping position is trivial, we just need to delete a couple of lines.

connyduck · 2018-05-27T08:17:03Z

ok you can merge this PR :)

charlag · 2018-05-27T08:21:42Z

Okay, let's hope you tested it properly 😉

* Add AppStore, add fav, reblog events * Add events, add handling to Timeline * Add event handling to Notifications * Mostly finish events * Fix unsubscribing * Cleanup timeline * Fix newStatusEvent in thread, fix deleteEvent * Insert new toots only in specific timelines * Add missing else * Rename AppStore to EventHub * Fix tests * Use DiffUtils for timeline * Fix empty timeline bug. Improve loading placeholder * Fix AsyncListDiff, loading indicator, "load more" * Timeline fixes & improvements. Fix infinite loading. Remove spinner correctly. Don't refresh timeline without need.

Add AppStore, add fav, reblog events

5871d2d

charlag added enhancement help wanted work in progress labels Apr 7, 2018

charlag requested a review from connyduck April 7, 2018 08:36

charlag added 5 commits April 29, 2018 00:31

Merge branch 'master' into events

9779376

Add events, add handling to Timeline

1bd78b2

Add event handling to Notifications

5702d56

Mostly finish events

e172b37

Fix unsubscribing

4b8ca76

charlag changed the title ~~[WIP, Experimental] Add AppStore, add fav, reblog events~~ [WIPl] Add AppStore, add fav, reblog events Apr 30, 2018

charlag commented Apr 30, 2018

View reviewed changes

charlag added 4 commits May 1, 2018 12:59

Cleanup timeline

02cb5d3

Fix newStatusEvent in thread, fix deleteEvent

497ccf2

Insert new toots only in specific timelines

83b853e

Add missing else

1972d0a

charlag mentioned this pull request May 5, 2018

Thread/conversation not updating after replying #552

Closed

1 task

connyduck added this to the Tusky 1.9 milestone May 6, 2018

connyduck reviewed May 6, 2018

View reviewed changes

charlag added 3 commits May 13, 2018 23:22

Merge branch 'master' into events

c909591

Rename AppStore to EventHub

86d7137

Fix tests

a77e8d8

Use DiffUtils for timeline

7ca9621

Fix empty timeline bug. Improve loading placeholder

29e5d67

Fix AsyncListDiff, loading indicator, "load more"

107c024

Timeline fixes & improvements.

9836599

Fix infinite loading. Remove spinner correctly. Don't refresh timeline without need.

charlag changed the title ~~[WIPl] Add AppStore, add fav, reblog events~~ Add AppStore, add fav, reblog events May 26, 2018

charlag removed help wanted work in progress labels May 26, 2018

charlag changed the title ~~Add AppStore, add fav, reblog events~~ Add EventHub, add fav, reblog events, improve timelines May 26, 2018

This was referenced May 26, 2018

Add the ability to pin/unpin statuses, show pinned statuses in profiles #443

Closed

Timeline cache #663

Closed

Remember position #89

Open

charlag merged commit 3756a1f into master May 27, 2018

connyduck deleted the events branch July 10, 2018 17:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add EventHub, add fav, reblog events, improve timelines #562

Add EventHub, add fav, reblog events, improve timelines #562

charlag commented Apr 7, 2018

connyduck commented Apr 16, 2018

charlag commented Apr 16, 2018

charlag Apr 30, 2018

charlag commented Apr 30, 2018

charlag commented Apr 30, 2018

charlag commented May 1, 2018

charlag commented May 2, 2018

charlag commented May 5, 2018

connyduck left a comment

connyduck May 6, 2018

connyduck May 6, 2018

charlag May 6, 2018

charlag commented May 6, 2018

connyduck commented May 6, 2018

charlag commented May 6, 2018

charlag commented May 13, 2018

connyduck commented May 16, 2018

charlag commented May 20, 2018

charlag commented May 24, 2018

charlag commented May 25, 2018

charlag commented May 26, 2018

connyduck commented May 27, 2018

charlag commented May 27, 2018

connyduck commented May 27, 2018

charlag commented May 27, 2018

Add EventHub, add fav, reblog events, improve timelines #562

Add EventHub, add fav, reblog events, improve timelines #562

Conversation

charlag commented Apr 7, 2018

connyduck commented Apr 16, 2018

charlag commented Apr 16, 2018

charlag Apr 30, 2018

Choose a reason for hiding this comment

charlag commented Apr 30, 2018

charlag commented Apr 30, 2018

charlag commented May 1, 2018

charlag commented May 2, 2018

charlag commented May 5, 2018

connyduck left a comment

Choose a reason for hiding this comment

connyduck May 6, 2018

Choose a reason for hiding this comment

connyduck May 6, 2018

Choose a reason for hiding this comment

charlag May 6, 2018

Choose a reason for hiding this comment

charlag commented May 6, 2018

connyduck commented May 6, 2018

charlag commented May 6, 2018

charlag commented May 13, 2018

connyduck commented May 16, 2018

charlag commented May 20, 2018

charlag commented May 24, 2018

charlag commented May 25, 2018

charlag commented May 26, 2018

connyduck commented May 27, 2018

charlag commented May 27, 2018

connyduck commented May 27, 2018

charlag commented May 27, 2018