feat: Ability to hide posts from feeds

[burakcan]

https://github.com/aeharding/wefwef/issues/45

Adds the ability to hide posts from feeds. You can either use the action sheet or long slide from right to hide/unhide posts. Hidden posts can be reached by the link in the profile.

In this implementation, the list of hidden posts is not separated by user. Not sure if this is the desired behavior?

Another consideration is; this is a possible performance bottleneck if a user has lots of hidden items. Since the creation of initialState is sync, reading from localstorage and parsing can add some delay to app startup. I tested with 10000 items, it's not noticeable with that number (iphone 12) so I don't know where the limit is. But my guess is that lemmy will support this natively before we hit that :)

[aeharding]

Hi there!

1. Nice work! 🎉
2. I haven't checked out the code for a proper review, but I will tonight!
3. A quick observation if you want to get started now: post IDs are not unique per instance. This means that post id = 1 on lemmy.ml probably refers to a different post than post id = 1 on lemmy.world. That's why I think we should separate hidden posts by user. You can key off of useAppSelector(handleSelector). In fact, you could do this in the redux actions (handleSelector(getState())). You could also create a selector (createSelector) to get hidden posts by user handle.
4. Again, nice job, we should be able to get this in very soon!

[burakcan]

oh didn't know the point 3. Will fix later today then 👍

Edit: now reread the part about handleSelector. While this would be useful for saving per user; we would still need to separate them per instance, too. I don't want to save a complex json shape with an unknown size to localstorage though. I'll think about this a bit.

Edit2: Ah ok so the handle+post id is a unique combination for this user after all.

[burakcan]

updated the pr. Now they are saved separately for each logged in user.

[aeharding]

Looks awesome! Just did a review and a couple tweaks and polish.

Things I changed:

• I just filter from the existing feed instead of forcing a refresh. Was there any reason you made a new request every hide? (I might be missing something.)
• On the hidden post page, I check if it exists in the store before fetching from the API. Mainly for an issue where you go to the hidden posts page, then go to feed, then every time you swipe to hide a new request would be made to fetch.
• Added collapse animation because why not! 😊

Just a couple outstanding things. If you can fix them, awesome, otherwise I will!

• Can we prepend hidden posts instead, so that most recently hidden are at the top of the hidden list?
• Let’s disable hiding posts for logged out users like Apollo does. Otherwise it gets confusing (and you can switch instances when logged out, which has the post ID problem)
• Hidden tab should be hidden when viewing other user profiles
• Lets use the get() and set() functions for localstorage access (see appearanceSlice)

We're really close on this!

[burakcan]

Hey, thanks for the feedback. Nice changes!

I just filter from the existing feed instead of forcing a refresh. Was there any reason you made a new request every hide? (I might be missing something.)

where does this happen? It's not on purpose, probably missed something somewhere. Still not 100% feeling comfortable with the codebase :)

Will fix the rest of the outstanding items later today.

[burakcan]

Did the changes @aeharding . Used the get/set from settings/storage but maybe we should those utils to somewhere shared?

[aeharding]

Resolved the conflicts, sorry about that.

The only bug I've found is if you swipe away more than a page content in a feed, then refresh the page, the feed will not show anything.

Probably we need to call to get new posts in a loop the first time until we get something that isn't hidden.

[burakcan]

@aeharding empty feed bug should be resolved with the latest commit.

Edit: actually this will also be buggy if everything in the 2nd page is filtered out. I'll push another fix.

[aeharding]

Hi @burakcan

Once again, great work. Paging is behaving well when hiding like mad on my device :)

If you look at 96cf10e you'll see some performance changes I made.

One of my concerns with the perf was the large performance impact of checking hiddenPosts.includes() ~200 times per feed update (given the many places that check for a hidden post)

So I decided to test it.

Let's say the average ex-redditor hides 200 posts a day. A ton, but hey these are ex-redditors. I wanted to target ~6months of hidden posts with this use case.

Again, this is probably overkill, but that would be ~36k posts.

So I threw 50k items into the array, and swiping was pretty laggy and glitchy on my iPhone 13.

I changed it to index by an object after fetching from localStorage. This has a very noticeable improvement in perf.

I threw a slice to cap max hidden posts to 50k.

What are your thoughts on this?

With your feedback, I think this is about ready to ship!

[aeharding]

The only other concern is localStorage storage limits at 50k items.

[aeharding]

(sorry for the wall of text)

The more I think about this, the more I think that we need to treat hidden posts the same way that we treat votes.

That is: When we start up the app, we don't get every vote that you've ever made and throw it in the store. We just get the votes that come down with the relevant posts/comments and throw them in the store

Similarly, we could do this with hiding posts. Except instead of retrieving votes from Lemmy, we'd be retrieving hidden state from Indexeddb.

Like this

=> Recieve posts from Lemmy with ids [1, 2, 3], then query indexeddb for ids = [1, 2, 3], handle = me, and put only those hidden post values in the store that are relevant to the current state of the app.

=> Hide post, then put in indexeddb.

=> Get all hidden posts for hidden tab, query handle = me and order by most recent

[burakcan]

Hey, yeah actually these points make lots of sense. As I wrote in my first comment, I expected this to be a performance bottleneck and kinda hoping for a native solution from the lemmy side.

But if we're ok with introducing indexeddb then it's definitely going to be easier to manage performance. Having something more powerful than localstorage would probably be useful for other things, too. Currently I can't think of a sane way of doing what you described with localstorage - without creating a separate entry for each post :)

Not sure if I'll be able to work more on this this weekend though.

[aeharding]

No problem. I'm currently putting out fires, so I don't expect to get much more than bugfixes in this weekend 😅

And yes, I'm totally fine introducing indexeddb. I think an abstraction library would be good too, because indexeddb API is terrible.

It'll be really nice to get this working well, because it means other less friction post hiding features, like hide on scroll, will remain performant even with tons of things hidden.

Again, thanks for your work and your time on this

[burakcan]

Soo. Here's an implementation with indexeddb :) This implementation should be quite performant compared to the localstorage solution.

It's been quite a while since I used indexeddb so my knowledge about the wrappers wasn't up to date. I checked the popular ones and it seems like dexie is good. It's maintained, well documented and reasonably lightweight. It's also used by big name apps.

I think we can also move settings to indexeddb but didn't want to do that in this pr.

[aeharding]

Did some testing, this is really fantastic. Works great with tons of posts in the store.

I did some tweaks, feel free to check out. I think we're ready to ship

[burakcan]

@aeharding nice, looking good. Just something about your changes; I think we shouldn't delete the items from the db when unhidden. I named the objects "PostMetadata" because I thought we could keep more about the posts here in the future. So we will probably forgot that we do it here and lose other metadata about posts. One solution could be to check if everything in this post is "default" and then delete it.

Also about "sorting", I was gonna do it but forgot. But I think we should have a "hidden_at" timestamp. Because again if we add other stuff in here, then the ordering won't be guaranteed to be correct.

Both changes should be quick so we're close to shipping :)

Wdyt?

[aeharding]

Hi @burakcan, the main reason I delete then re-add is so that most recently hidden appears at the top of the results when querying for hidden.

My first attempt was adding hidden_at and trying to sort by that index - but I was running into roadblocks with paging - see: https://dexie.org/docs/Table/Table.orderBy()#orderby-on-collections

Feel free to take another shot at this, I likely missed something :)

[burakcan]

I think this is ready now :D

[aeharding]

The only issue I have seen is that tapping "unhide" from the ellipse menu of hidden posts page doesn't remove the post from the list.

Not a huge deal. We can merge this and @burakcan if you want to look at that we can do a separate PR :)