-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use new tab based design for common chats #1502
Conversation
I think for the private chats we could use the same design as in trade private chats (list above the chat for selecting the peer). |
I think in events we can remove: No-KYC services, and Node projects as those are more static content anyway and events should have some live-feeling with updates. Also Discussions can be removed at about 4 public channels (markets and/or economy would be removal candidates IMO). |
@HenrikJannsen - thanks for the input.
I made this mock-up as a proposal design to unify private chats everywhere. To optimize space usage and considering the nature of chat interfaces, I propose using side-by-side columns (users list and chat box), which I believe is more efficient than a stacked arrangement like that of 'Bisq Easy Private Chats'. With a stacked layout, as the number of chats increases, the chat box could become very small. Or a fixed-size layout would require scrolling, which may not be the best user-experience approach IMO. A left side vertical column for the users seem to offer the best trade-off here. Also, upon looking of various chat applications, this is always the layout design of choice. What do you think?
I have to think about this. Not sure if it's necessary. I will explore some options.
I was planning on adding them, but space was an issue. I will revisit this now that we are removing tabs.
Agree. I will remove that. |
Agree to the left side nav arguments regarding private chat. A reason at the trade chat was as its very similar to the open trade layout. Here I think a left nav is no option as we need more info of the trade to be shown and the trade process UI requires more horizontal space. But I agree that the normal private chats should be the same everywhere and I dont see strong arguments to stick with the way how its done in bisq easy pm. |
85dc868
to
b896fa4
Compare
Looks great! The space above the tabs is a bit high. I think its better to have it the same as in offerbook. The meetup has the same icon as events. Maybe we can find a replacement there. Same for support/gen support. The icon used at off topic looks more like trade and the trade icon looks more like markets. Not sure if there is a good fit for off-topic of the current icons... Maybe "Chat peers" would be better than "Peers"? The selection at the left nav in private chats has a dark border left and right. I think without border it would be more in line with other design. But I understand that the grey tone with the background might be then too close. Maybe a very dark grey could work? The Bisq and BTC icons are a bit small. Also markets. For the BTC icon the typical tilted version with a circle might be also better. If we use a header, we could also consider to use colored icons in larger size, similar as the markets in offerbook. I guess the design could benefit from a few light color drops (as in trade protocols). The leave button gets truncated in small width if info sidebar is open at private chats. |
Some code remarks (I add it here not in the commits, as I looked over the whole code change and finding the commit would be extra effort): TabButton line 91: I think ChatSearchService should not be handled like a domain service. I will send you a patch with an alternative design suggestion. |
Thanks for the quick review!
Yes, I would prefer to work on a new PR. Some comments follow, though:
My idea here was to put the search inside the tab line as you mention, and put it where it is now when the space is too small. However, I really like your idea of doing it similar to offerbook. I also have some small improvements I would like to make there, so perhaps we could align them both in a new PR.
Yes, sound good. Let's do that together with the above.
I have changed this and also "Private chats" to "Chat window". I wasn't convinced with this header either.
I will review this selection in the next PR.
Yeah. I will look at that then.
I don't particularly like this button, I think it looks too big. I would like to look for a better solution for this.
I added this in order to wrap the words dynamically when changing the window size. Since we removed channels instead, this is not necessary anymore. I have therefore deleted it. When it comes to the icons, I have created a new issue (#1514) with all the things you pointed out. I will work on it. |
Here is my suggestion to replace the ChatSearchService. To use a small component for the toolbox items helps also to keep the main view more light weight. But the complicate hierarchy here smells as a UX issue. If we would move the toolbox to a header bar in the chat view those problems are gone as the toolbox functionality sits right at the view where its operating on. Here the patch:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NACK
There is missing some changes from the patch. See also https://github.com/HenrikJannsen/bisq2/tree/ChatSearchService-alternative
9281e04
to
3148006
Compare
@HenrikJannsen - there was an issue with the patch you sent me ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
Fix #1279
Public chats:
Private chats:
Worst case scenario - With domain
events
, which has the most channels, all of them with notifications, and the smallest possible screen:I will make follow-up PRs with some minor UI improvements.