mikedilger / gossip

Gossip is a nostr client
Other
687 stars 77 forks source link

Uniform and restyle the repost/quote visualization #305

Closed dtonon closed 1 year ago

dtonon commented 1 year ago

I propose to uniform the repost visualization, so kind-6 (with or without event payload as content) and kind-1 with root e-tag and only a "#[0]" content should be rendered identical.

This could be the the plain repost layout, just a quick sketch reusing as much as possibile the actual UI:

image

If there is a text around the mention we can basically have 3 cases: 1) The text is around the mention, above and after, and we can have more mentions too. This should be rendered as a standard post, without showing the mentions inline. 2) The text is only above/below the mention and it is "short" so we consider it a comment. 3) The text is only above/below the mention and is "long" so we consider the mention a footnote.

For cases 2 and 3the visualization could be basically the same, like this:

image

Of course a better UI/UX for case 1 could be possible with some sort of preview for footnotes (ex. an note abstract in smaller font).

bu5hm4nn commented 1 year ago

We need to restructure the way content is layed out. Currently it's a wild mix of horizontal and vertical layouts. I think we should have a top level layout that is vertical

header
content
footer

That way we can extract rendering of those areas into their own functions. The avatar can overlap into content top a little in order to get the same look we have now, and standard content would start out with a margin left. I also think we can get rid of a few of the spacers and use frames with margin instead.

mikedilger commented 1 year ago

@bu5hm4nn yes this code has all grown organically and could use a refactor.

@dtonon In your first image, I'd shift the smaller gigi line left so it doesn't look like an indentation of the previous post. I haven't thought enough about the other factors to comment.

dtonon commented 1 year ago

@mikedilger yes this alignment can be a little confusing; we can elaborate a different and cleaner version of course. For example we can can centrally align the avatar and simplify the top content showing less data:

image

The same central alignment and user data simplification can be applied for the mockup about the mention with a comment.

As said, I just copied/pasted the current structure and modified/hid the smallest details to sketch the goal of a uniform display for the repost; is not a final design; we can go further and I can come up with a completely revised version, but I seemed to understand that now is not the right time to do this.

If we start a full redesign we have to think about a lot of more details, for example if make sense have a clickable name with a dropdown and an avatar clickable that bring to the account page (spoiler: I would keep only the avatar clickable, showing a dropdown: See Notes, Mute, Follow/Unfollow, Zap, etc).

bu5hm4nn commented 1 year ago

Looks like other platforms do use the outer note for reactions. See the the reaction counts on Jack's and Amanda's post in the screenshot.

note1qqqq0vshmlh9ma8d4805xj9lj9anzjjxpmerqmhkywr9px5x99lqky9vkl

So this means we need to think again about hiding which reply buttons. It seems hiding the inner note's buttons is what other implementations do.

bu5hm4nn commented 1 year ago

I have gone with showing only the outermost reply buttons. Please review.

mikedilger commented 1 year ago

IMHO we should show buttons whenever the quoting post adds some kind of content.

And the innermost post is the main event IMHO so it should have a row of buttons.

All those reactions to Jack's repost in which all Jack did was repost -- I don't think anybody should care about those sycophants.

dtonon commented 1 year ago

@bu5hm4nn

Looks like other platforms do use the outer note for reactions

Currently reactions are not managed in a visual form except for the icon counter; I suppose we can keep them out this review. They could be analyzed when/if we will talk about user notifications.

By the way I don't understand what is happening in the screenshot: Jack reacted to Joe reposting itself. And PurZZ? Can you share the note id?

And the innermost post is the main event IMHO so it should have a row of buttons.

Agree with @mikedilger, the main content should have all the buttons. As said in the group on chains of recursive repost I would only show the outermost and the innermost (the real content).

We could then visualize recursive repost and more reposts of the same content (useful to clean up the feed and raise the signal) like this:

image

But this isn't only a design review, it glitches the timeline aggregation/logic, so perhaps should be talked about in another issue.

dtonon commented 1 year ago

@bu5hm4nn a couple of glitches I found running your last commit 27aee36 :

Mention with comment: note1qqqqqjrzmylrr9hqz0xc7pq7ecrmxrt6r7ndqqufdwz7mldz7gcsytyuc6 The left margin is missing. The avatars size should be inverted: my one bigger and the quoted one smaller.

image

Mention with comment: note1qqqqqdj9f2nap6dzqtvtde9hn0ljkwa74rywpsxqr74vfy0syzlsjje6yf At the begin the mentioned post was showed as id on the bottom, I clicked/load it correctly, come back, and now doesn't render the full content neither the noteid.

image
dtonon commented 1 year ago

@bu5hm4nn I just hit the same bug we talked about: note1qqqqqwg6j7ufz3er8qkuzmayljmhac7asp8ee2rkqddq7w5w3nnqwqh77k

image

It seems to happen when the parent post already has an indentation (old repost style or reply).

dtonon commented 1 year ago

Just posted a mention with comment, it shows up without the mention: note1qqqqqjhpjlcdn47tx0lygaqafsssjx077ydv2l7wu3d9yheaxfaszzmksn

image

Probably related to https://github.com/mikedilger/gossip/issues/305#issuecomment-1467649623

dtonon commented 1 year ago

On this repost (kind 6 with embedded event) the bottom icons refer to the Calle repost itself:

note1t0m6qz89gpssjvq30e7tnphamjhdkqwy4ue0xmw3dzxr8h65ee9qy398u7

image

We said that on a plain repost without comment we should show only the main (innermost) content footer actions, so it would seem logical do it here as well.

bu5hm4nn commented 1 year ago

@dtonon Please take another look now

bu5hm4nn commented 1 year ago

Still need to find where that whitespace under the buttons is coming from

Screenshot 2023-03-14 at 14 30 16
dtonon commented 1 year ago

@bu5hm4nn

@dtonon Please take another look now

All good!

Still need to find where that whitespace under the buttons is coming from

Can you share this the note id? I would understand the nested structure.

bu5hm4nn commented 1 year ago

note1fqfg0t756a92cc4evlgyv4ap9xvnnc7vftwctr9lzkd38d82whvsqe9k5u

bu5hm4nn commented 1 year ago

@dtonon I think it's ready for more testing. Please patch in my branch of nostr-types for testing. Add this to the bottom of .cargo/config file:

[patch."https://github.com/mikedilger/nostr-types"]
nostr-types = { git = "https://github.com/bu5hm4nn/nostr-types", branch="process-all-mentions" }
bu5hm4nn commented 1 year ago

Indenting of comment above mention is one open issue

Screenshot 2023-03-14 at 18 35 26

note1nsvrp8pel97lry8hmfyadkfsz9y3mn5jlmx7cqmga0v9fg9e24zs6l392k

bu5hm4nn commented 1 year ago

Indenting comment + mention should work now. @dtonon Note that the vertical whitespace in your post here are actual newlines in your content:

Screenshot 2023-03-14 at 20 04 25

note1qqqqqjhpjlcdn47tx0lygaqafsssjx077ydv2l7wu3d9yheaxfaszzmksn

dtonon commented 1 year ago

@bu5hm4nn I'm going to test it!

Note that the vertical whitespace in your post here are actual newlines in your content:

I added a couple of newlines before the note id to space it. I suppose we can safely remove all the whitespaces (regex \s) at the begin/end of the content, both before and after the mention elaboration. I can't think of a legit/useful use of more whitespaces in this context.

/cc @mikedilger

dtonon commented 1 year ago

@bu5hm4nn

note1nsvrp8pel97lry8hmfyadkfsz9y3mn5jlmx7cqmga0v9fg9e24zs6l392k

Ok done, I was unable to find it on Gossip even if I'm connected to some relays where it is present (checked with nostr.band), then loading the parent note1qqqq0vshmlh9ma8d4805xj9lj9anzjjxpmerqmhkywr9px5x99lqky9vkl the thread showed up. It's an hell :))

image

Of course this a forged edge case where we have some sort of recursive posting, perhaps we can skip it because is a little extreme as benchmark for standard situations.

We agree that reposts of reposts should be flatted showing only the outer repost and the innermost content? I think that to achieve this we have internally to equal kind-6 and flat mention without comment. With this logic we can flat the showed conversations keeping only the events with a orange border. Perhaps we can keep also a repost before a mention with comment, the yellow bordered ones in the image. Make sense?

bu5hm4nn commented 1 year ago

Yes it does make sense, however it would require traversing the entire tree of notes before deciding what to render. With how egui works that would be done at every refresh (default 15FPS up to 60FPS). It would make sense to introduce this at the storage/caching layer so it only has to be done once for retrieved notes and potentially we could even store that information (e.g this note is part of a mention-chain). Then we could serve the entire mention-chain to the UI and it only has to render based on some metadata we calculate (e.g. is_empty_repost, is_comment_repost, etc). So all that to say, I don't think it should be in the scope of the current UI work.

bu5hm4nn commented 1 year ago

I will look at trimming whitespace between comment and mention. Hoping all this processing didn't create any significant performance drawbacks.

bu5hm4nn commented 1 year ago

@fiatjaf can we get your eyes on this?

dtonon commented 1 year ago

@bu5hm4nn I like the idea to have a db relation (es. event.parent_id) to chain events using a recursive-model strategy; I agree that is out of the scope of this issue. For now we can make an UI that easily scales to this approach, and I think the proposed one is already fine.

bu5hm4nn commented 1 year ago

And when we do that work, we could also think about how to identify and store all reposts of the same note, so that if many people repost the same note, we can visually group (e.g. "fiatjaf and 5 others reposted") and only show the reposted note once.

bu5hm4nn commented 1 year ago

Just had the idea to check classic theme 😬

Screenshot 2023-03-15 at 17 16 57

More work needed

dtonon commented 1 year ago

@bu5hm4nn as we discussed please try to update the mention as proposed before, but adding a light background to highlight the mentioned note:

image

If possible a gradient background should be better:

image

Small detail: the bottom icons should be aligned with the poster name and text content.

PS: I think this mention rendering could work well and safely be used every time a mention is placed on a new line (regex ^\s*#\[\d+\]\s*$, so not inline).

bu5hm4nn commented 1 year ago

How far do you think we are on this issue with the latest commit on my branch?

dtonon commented 1 year ago

@bu5hm4nn the issue seems fulfilled, great work. The UI details are not so relevant now, we can think about them in a more wide analysis, and different issue. The only point I will try to update is how more mentions are rendered:

note1qqqrmpd047dnu6r3hcss4j5veelg27m474pq5gnvumgueh0ltk0sjnm43a

image

As suggested in my last message could be nice to use the same format (grey background) in this context too. Is possible? Last detail: I would lighten the background of the mention, something like 250-250-250.

bu5hm4nn commented 1 year ago

I would suggest you check and rework the template for the color changes once this is merged. Since master is a work in progress I would think that would be acceptable.

mikedilger commented 1 year ago

Many people run off of master, so while you can indeed put half-complete features on master, please don't break master.

bu5hm4nn commented 1 year ago

Can we consider this issue complete?