mikedilger / gossip

Gossip is a nostr client
Other
708 stars 81 forks source link

Review Relays section #834

Open dtonon opened 2 months ago

dtonon commented 2 months ago

Here are some fixes to clean up and improve (imo) the Relay section:

General interface

Lists

Relay details

bu5hm4nn commented 1 month ago

I skipped moving the "Advertise Relays" button for now, because it is also used as a status indicator for the advertisement (it displays "n relays to go"). I think we need a general solution for displaying status of "background tasks".

dtonon commented 1 month ago

Is the realtime update really important for the user? I don't think so. I would keep trace only of the final status, and for this we need to enable the general log view. But it's an optional step.

mikedilger commented 1 month ago

I think we need a general solution for displaying status of "background tasks".

Yes I agree. People don't want to look at terminal logs, but they do want to know if their posts succeeded to the relays they are going to, and when advertise is done, etc.

dtonon commented 1 month ago

@bu5hm4nn there is a scrolling issue with long relay names:

image
dtonon commented 1 month ago

Yes I agree. People don't want to look at terminal logs, but they do want to know if their posts succeeded to the relays they are going to, and when advertise is done, etc.

I also agree, and we actually have a free slot on the bottom of the sidebar for this. We can show both informative logs (maybe the level can be set by a preference) and the in-progress tasks, putting them on the top with a different style (e.g. a light animation).

But for this specific case, I think that, for now, we can safely avoid showing the user a counter that goes from 250+ to zero.

bu5hm4nn commented 1 month ago

Should be all done on branch

mikedilger commented 1 month ago

Looks good to me, except that the relay path is missing. If relays have a path (which is the exception) it must be shown. I'm fine with wss:// being elided, as well as trailing slashes. But we need the path. I insist.

Gossip picks up a lot of bad relay records. I will try to work on pruning those out. For the moment, you can't identify the bad ones without the path.

dtonon commented 1 month ago

@mikedilger

But we need the path.

I agree. To have a cleaner default view, we can show it (together with the port) when the relay is expanded, like this:

image

And in the details mode:

image

What do you think?

dtonon commented 1 month ago

@bu5hm4nn almost perfect, just this notes:

image

I would further reduce the search width, 8-9 characters seems sufficient to filter the relay list. This way we have more space to show the title when the window is compact.

image

The tooltips are still present.

image

The outbox preview box is not always clickable, it have blind spots, you can verify it from the fact that the mouse cursor changes when hovering.

image

Do we have a neutral (gray) version of the dropdown button? I would prefer it.

I added an item:

mikedilger commented 1 month ago

What do you think?

That's ok. As long as it is there.

dtonon commented 1 month ago

That's ok. As long as it is there.

@bu5hm4nn can you that the path/port as proposed?

bu5hm4nn commented 1 month ago

Please re-test the branch. I am not confident enough to remove the tooltip with the full url string, I might have missed some cases (maybe some with arguments).