Skip to content
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

Unify chat window headers for public channels #1539

Merged
merged 8 commits into from
Dec 28, 2023

Conversation

axpoems
Copy link
Contributor

@axpoems axpoems commented Dec 19, 2023

Introduce improvements in new design for public channels (BisqEasy offerbook, and domains Discussions, Events and Support).

Offerbook:
image

Events domain:
image

I will follow-up with another PR regarding improvements in private chats and other further design reviews.

@HenrikJannsen
Copy link
Contributor

HenrikJannsen commented Dec 19, 2023

Looks good to me.

Was the change of the green color in the main path buttons at dashboard and bisq easy welcome screen intended?
I find it better with the green and in usability test it worked very well that ppl felt they should follow that suggestion.

For the common chats header I think it would be better to move the icon from the tab to the header and make maybe the header text the same style as in offerbook.

For the create offer button a bigger text would give it more weight and we could consider to add an icon.

Maybe to move the 2 icons left to the search box would make the search box more aligned with the right side. Alternatively maybe we could find other icons which would fit better to the search box style.

@axpoems
Copy link
Contributor Author

axpoems commented Dec 19, 2023

Was the change of the green color in the main path buttons at dashboard and bisq easy welcome screen intended?

No, this is a regression. Thanks for pointing it out. I will fix this now.

For the common chats header I think it would be better to move the icon from the tab to the header and make maybe the header text the same style as in offerbook.

I agree. I will add the icons with the PR where I make the improvements in the icons.
As for the header title, I agree as well with aligning both, but I think it should be the offerbook taking after the public chats.

For the create offer button a bigger text would give it more weight and we could consider to add an icon.

I will review all the text sizes in general so for the moment, some things will appear a bit off.
In this case in particular, I think the surrounding texts have the text size too big. In my PR #1530 I am decreasing the size of the input text, and this will adjust the weights. I will update that PR soon.

Maybe to move the 2 icons left to the search box would make the search box more aligned with the right side. Alternatively maybe we could find other icons which would fit better to the search box style.

I am working on this. This is part of follow-up PRs that I am intending to make soon.
I will create an issue with the mock-ups that I have made already for this.

@axpoems
Copy link
Contributor Author

axpoems commented Dec 19, 2023

@HenrikJannsen

image

I have aligned both titles now. For the sake of design, I had to disable the channel icon until we have a better one.

Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@axpoems axpoems force-pushed the unify-chatwindow-headers branch from 8018101 to 126a479 Compare December 19, 2023 18:51
Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@HenrikJannsen
Copy link
Contributor

HenrikJannsen commented Dec 20, 2023

Small thing: The arrow for the markets looks a bit too bold compared to the headline and the padding and y position might be adjusted a bit. But no need to do that in this PR...

@axpoems
Copy link
Contributor Author

axpoems commented Dec 21, 2023

The arrow for the markets looks a bit too bold compared to the headline

That's true. I've noticed, and will make a new image for this.

the padding and y position might be adjusted a bit

Can you be more specific? I see it centered. "BTC/EUR" is in the middle of the y axis, and then the description continues on the same line. This is by design.

@HenrikJannsen
Copy link
Contributor

Can you be more specific? I see it centered. "BTC/EUR" is in the middle of the y axis, and then the description continues on the same line. This is by design.

I meant only the arrow down icon for the drop down. It feels to me 1 px too low and maybe 1-2 px too much left.
The same base line for headline and description looks good to me.

@axpoems
Copy link
Contributor Author

axpoems commented Dec 22, 2023

I meant only the arrow down icon for the drop down. It feels to me 1 px too low and maybe 1-2 px too much left.

Yeah, I did not go pixel perfect there since the arrow (as discussed before) is incorrect. Will fix paddings when I introduce the new arrow in another PR.

Copy link
Contributor

@alvasw alvasw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@alvasw
Copy link
Contributor

alvasw commented Dec 27, 2023

Hi @axpoems, I can't merge this PR due to a merge conflict in apps/desktop/desktop/src/main/java/bisq/desktop/main/content/chat/ChatView.java.

@axpoems axpoems force-pushed the unify-chatwindow-headers branch from 126a479 to 8ddc8e9 Compare December 27, 2023 10:06
@axpoems
Copy link
Contributor Author

axpoems commented Dec 27, 2023

Hi @axpoems, I can't merge this PR due to a merge conflict in apps/desktop/desktop/src/main/java/bisq/desktop/main/content/chat/ChatView.java.

@alvasw - Fixed now. Thanks for the heads up.

Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Copy link
Contributor

@alvasw alvasw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@alvasw alvasw merged commit b046038 into bisq-network:main Dec 28, 2023
15 checks passed
@axpoems axpoems deleted the unify-chatwindow-headers branch December 30, 2023 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants