arj03 / ssb-browser-demo

A secure scuttlebutt client interface running in the browser
Other
58 stars 11 forks source link

Include replies in Notifications #143

Closed KyleMaas closed 3 years ago

KyleMaas commented 3 years ago

Attempted to do this using pull-stream. I think it seems to work okay, although I'm also not an active enough poster to really give this a good test.

KyleMaas commented 3 years ago

One thing I can think of that we might want to change about this is to "clear" notifications after you react, just like how it excludes posts in threads from before when the user posted to them. But I wanted to check this over first and see if I'm going in a valid direction, and besides, improvements could be made in other pull requests.

KyleMaas commented 3 years ago

Hm...do we need to also exclude blocked users at the UI layer here, or would that be already filtered out underneath somewhere?

arj03 commented 3 years ago

I tried testing this but didn't get any results. I just replied to one of my browsers old threads and that message should show up as a notification. The good thing about that code is that its easy to change SSB.net.id to some other id :-)

arj03 commented 3 years ago

Regarding blocked, then yes and no. If you block and delete then they are gone. But if you just blocked, then they need to be filtered out at the UI level. This is why I'm sort of in favor of the block & delete.

KyleMaas commented 3 years ago

Feel free to change this if you can make it work. I'm still not very familiar with either db2 or pull-stream, and this uses both. It worked for me, but like I said, I don't post enough for much to actually show up, so it's hard for me to test this myself.

arj03 commented 3 years ago

Not sure I understood the clear notifications question. It is true that once you reply to a thread again that you "clear" old notifications. This might be a bit counter intuitive. Is that what you mean? I can see the implementation would be even more tricky because of this. I'm actually quite impressed with the pull thing you pulled together, it should work from reading the code at least :)

KyleMaas commented 3 years ago

The question about "clearing" notifications is that I would think that reacting to a post would indicate that you've read something just as much as replying to a thread would. Do we want to filter out notifications for messages prior to a reaction (what I was asking about) rather than just messages prior to a reply (how it works now)?

And thanks! It was tough, trying to learn the pull-stream API at the same time, but I figured if I could implement it that way, then we could use something similar to display a "new notifications" indicator and also do event-driven autorefresh.

arj03 commented 3 years ago

I think it's fine how it works in this PR regarding clearing notifications

arj03 commented 3 years ago

Found the problem. When you open a thread, root is not set. So you need something like this instead in getSameRoot:

if (data) {
   const root = data.value.content.root ? data.value.content.root : data.key

With that I got my reply :)

KyleMaas commented 3 years ago

How's that look?

arj03 commented 3 years ago

Thanks :)