-
Notifications
You must be signed in to change notification settings - Fork 101
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
ui: Dialog panel coloring #2588
Conversation
Nice, the additional contrast between elements is a welcome addition. |
@peterzen asked me to take a look,
|
5804e78
to
f374973
Compare
Agreed - this is out of scope for this PR though; #2483 contains the redesign of the settings panel as a UI dev subtask. |
I don't disagree with that. Note, however, there are different ways to draw attention - some are better than others (meaning, could be more subtle). Anyway, all of those observations I've listed are mostly just minor details (perhaps even subjective). |
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.
A lot of this is a big NO from me. The only reason we should be adding the background color is if it provides a obvious benefit and a needed visual indication of separation of sections, and even then, it should be used sparingly.
Starting with the initialization dialog sequence...
There's nothing to differentiate there. There is only one major element showing at a time.
For the wallets page...
If you apply it to all elements, than you might as well apply it to none. It also just doesn't look good or conform to modern standards. Maybe try just the asset selection column on the left.
Similar for the markets view...
Applying to all elements might as well not apply to any. Maybe try just the market selection column on the left.
For the settings view...
Here again, there's only one column. There are really no elements that need differentiation, and it doesn't look good.
Even for the modal dialogs, I'm not sure anymore. I'm leaning against this whole effort, TBH. Maybe if you scale it back about 90% and apply these colors extremely sparingly and only where there is an obvious benefit, there could be something here.
If you go back far enough in our repo history, you'll see that we used to have a similar scheme with a class called, I think, .card
. We got rid of it because it simply did not look good, and did not match modern styling standards. I don't plan to bring that back, but if we can apply it in a very targeted manner where there is an obvious benefit, I might be convinced.
The dialog boxes are modal, have a semi-transparent background color that covers the rest of the page, and have an animation associated with their display. There are enough indicators that the user needs to pay attention to that dialog already. |
In #2578, you applied some |
Maybe it's my screen but that transparency is not visible at all, don't see it in the CSS either. The semi transparent layer under the dialogs would go a long way in communicating that I'm still in the same context and differentiate the dialog. A slightly different dialog background would still help differentiating the modal from the rest of the page. TBH the fly-in animation is more disorienting than helpful. A short fade-in or such would work better. |
It's there just not very visible against the dark background - look at the light scheme. |
Oh. I guess it's not semi-transparent. The point is that the user can't not notice the new focus.
If you think the the
I don't completely disagree. I've often found the form change animations to be a little blunt. On the other hand, that has nothing to do with the intentions of this PR, and they're not that bad. |
Maybe. But if that's the case, we should apply the background color to the banner rather than all of the page elements, and I don't think that would be an improvement. Be subtle. |
Well, let's say you click Send on the Wallet page. Suddenly the screen goes blank and a bunch of elements fly in from the left - once they're in place you realize it's the form you're after. After a few times we get used to it and stop noticing, but most new users experience a WTF moment simply because this behavior is unusual, almost resembling a UI misbehaving. :)
It makes a difference.
The purpose of the animation is the same as those of the colors - help communicate visual hierarchy - in UI design they're usually developed together but it's indeed out of scope here. |
f374973
to
e71d648
Compare
3a0e812
to
5fd7519
Compare
I'm all about copying! At least in a general sense of styling standards. Here are the two highest volume exchanges. Coinbase uses no background colors to separate elements, just faint lines, like we have. Binance has a different background color for the header. Main page elements are all the same. |
I think the key to dark scheme is to layer it with a highly-blurred light glow. Something like https://jsfiddle.net/jfu8hrve/1/. |
I think a large part of what makes these unaesthetic is the inconsistent or simply nonsensical margins. For the "Create Account" form, the margins are just too big. For the bond asset selection form, the margins/padding were being set by an animation, and were asymmetric at the end. Also, dark mode looks better with a light border. I tried to resolve such issues in #2601. |
I really appreciate this effort, @peterzen. I will almost certainly merge #2601 before reconsideration of background colors. I will close this PR in favor of smaller, more-focused proposals. Some potential followup.
There are surely an infinite amount of other design changes that would improve the user experience, but either keep such PRs very limited in scope, or discuss your intentions in Element chat and get a go-ahead before moving forward. |
This proposes a set of UI changes to add depth to the UI elements by using panels instead of borders, and improve contrast.