arj03 / ssb-browser-demo

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

Add new Threads tab which displays a forum view of public posts #252

Closed KyleMaas closed 3 years ago

KyleMaas commented 3 years ago

Adds a forum view of Public posts.

Fixes #228.

KyleMaas commented 3 years ago

I'll note that for the moment, this just displays public posts in a forum-ish view. But it could easily be extended to show the latest messages in favorited channels or messages from user groups. But I wanted to start with this and make sure I was going in the right direction first. I actually find this far more useful than the "drinking from a firehose" Public tab.

arj03 commented 3 years ago

Got a bunch of:

this.created is not a function
    at a.feedId (bundle-ui.js:149536)
    at fn.run (vue.min.js:6)
    at un (vue.min.js:6)
    at Array.<anonymous> (vue.min.js:6)
arj03 commented 3 years ago

And yes this is indeed a quite nice condensed view. A suggestion would be to add a miniature picture of the author to the right so it spans both the name and the time under last post. Also there seems to be something with Last post. It's always the same as started by. Havn't looked at the code yet.

And yes I like this view as well :-)

arj03 commented 3 years ago

Actually not sure about the image now. One could also add one for the original author to the left, but there is a certain asthatic to just the raw text format. Just bouncing ideas here.

KyleMaas commented 3 years ago

The name on the right always being the same as the thread starter would be related to the this.created error. It worked here, but looking at it, I could see how that could crash.

And, yeah, I thought about doing the profile picture and then figured it might be better with just the text. It's a little more condensed and forum-y this way, IMHO.

arj03 commented 3 years ago

That fixed the issue

KyleMaas commented 3 years ago

That should fix the error.

I also added tiny little profile images next to the names. That doesn't expand it much. How's that look?

arj03 commented 3 years ago

Yeah the tiny images are great I think. Still has the old school forum vibe but a lot easier to scan.

KyleMaas commented 3 years ago

Agreed. It's a lot easier to identify users that way. Good call.

KyleMaas commented 3 years ago

Enhancements I could see adding to this:

But, by the same token, I don't want this to turn in a behemoth of a monolith that creates conflicts every time we want to change something, so I'm thinking these would be better as individual pull requests.

KyleMaas commented 3 years ago

I think I'd like to try and get this stuff merged before I work on the concurrency stuff, since I'm fairly certain it'll require the same modifications as everything else and I'd rather have that be a nice clean pull request. What else needs to be done on this for it to be merged?

arj03 commented 3 years ago

The comment was just a minor thing I found. This seems to be something like 12 seconds on my laptop to display. There must be something we can do to speed this up. In a way this view seems very similar to what ssb-threads is doing. So maybe it makes sense to see if we can adapt that and use here?

arj03 commented 3 years ago

I can see why you'd want this merged first. This is also why I took a look and tried it out, but as it is now I would rather try and get some fixes in than putting in more features. I'm thinking of doing a release soon again.

KyleMaas commented 3 years ago

The comment was just a minor thing I found. This seems to be something like 12 seconds on my laptop to display. There must be something we can do to speed this up. In a way this view seems very similar to what ssb-threads is doing. So maybe it makes sense to see if we can adapt that and use here?

This is largely due to the second query. For some reason, asking for a bunch of messages by or'd keys seems to be really slow. It was much faster prior to commit a917df1.

ssb-threads was on my to-do list. I've been watching @staltz's progress on porting to db2 for a while now. But since that involves a change to ssb-browser-core and rebuilding the Thread page, I figured it'd be better for after we got this behaving the way we wanted.

KyleMaas commented 3 years ago

I can see why you'd want this merged first. This is also why I took a look and tried it out, but as it is now I would rather try and get some fixes in than putting in more features. I'm thinking of doing a release soon again.

Makes sense. But I'd be of the opinion that that release should happen before any concurrency fixes anyway, since there will be a lot of changes that will need to be tested. Really, I don't see any reason why a release couldn't happen now (or at least pretty soon). It's been pretty stable for me lately, as evidenced by the lack of "bug" issues I've been reporting. The two biggest and most serious bugs I'd love to see fixed are #234 and #186, both of which have proved to be outside of my competency level to debug.

KyleMaas commented 3 years ago

Well, turns out Thread probably shouldn't be ported to ssb-threads because it uses partial-replication to try to fetch missing messages. But it works pretty well for the forum-like view.

KyleMaas commented 3 years ago

Oh, I should note that this relies on this now: https://github.com/arj03/ssb-browser-core/pull/33

KyleMaas commented 3 years ago

Is this working any better for you?

KyleMaas commented 3 years ago

There we go. Merge conflicts fixed, and working again.

arj03 commented 3 years ago

Great, will have a look soon.

arj03 commented 3 years ago

Hey Kyle. I havn't forgotten about this. Have just been SUUUUPER busy. Hope you understand that there is a human in the other end :-) I will try to get to this soon.

KyleMaas commented 3 years ago

Heheh...thanks for the update. I was worried for a bit that this was just going to languish away. I've at least been using it in the meantime. :)

arj03 commented 3 years ago

Testing this now. A lot faster, nice to see ssb-threads used here.

I ran into a strange issue, the first thread showed andre's phone for a bit and then changed into this invalid date and no image:

image

arj03 commented 3 years ago

This is looking good code wise, we can always fix minor issues after merge.

KyleMaas commented 3 years ago

Hm. Not sure what would cause that. It's never done anything like that for me before, but I'll see if I can figure it out.

KyleMaas commented 3 years ago

Oh, and thanks for merging this! I've actually been using the Threads tab a lot lately, and there were several things I was kind of holding off on working on until we got this done because I didn't want to have to deal with the merge conflicts it would cause.

arj03 commented 3 years ago

Yeah it does seem to work pretty well. A whole different overview of what is going on. I especially like the replies count.