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

Dark mode, flat UI, alignment fixes #4121

Closed
wants to merge 20 commits into from
Closed

Dark mode, flat UI, alignment fixes #4121

wants to merge 20 commits into from

Conversation

pedromvpg
Copy link
Member

@pedromvpg pedromvpg commented Apr 5, 2020

UI improvements and consistency pass:

  • a darker Dark Mode 🦇
  • reduce contrast in active button on main nav
  • add hover states in main nav
  • adjustments on UI to make the design more minimal and flat
    • remove shadows and gradients
    • remove bottom lines from fields
    • standardize colors of divider lines
  • alignment issue fix in settings page in national currencies table
  • alignment issue in slide toggles
  • fix color disparity on support chat (dark mode)

Bisq_UI_Changes

Screen Shot 2020-04-05 at 6 54 58 PM

chat_arrows

@boring-cyborg
Copy link

boring-cyborg bot commented Apr 5, 2020

Thanks for opening this pull request!

Please check out our contributor checklist and check if Travis or Codacy found any issues with your PR. Also make sure your commits are signed, and that you applied Bisq's code style and formatting.

A maintainer will add an is:priority label to your PR if it is up for compensation. Please see our Bisq Q1 2020 Update post for more details.

@wiz
Copy link
Contributor

wiz commented Apr 6, 2020

Pedro, excellent work on these changes. I know how hard it is to work with Bisq CSS and you did a great job. I just tested your PR and it looks great, but I think removing the lines from text input fields might not be a good idea because now I can't easily tell which text field is focused for text input, and also there are many lines still remaining in other text input fields in other places in the app. Was your intention to only remove them for pull-down combo boxes?

@wiz
Copy link
Contributor

wiz commented Apr 6, 2020

Also, here are the CSS errors from the log

Apr 06, 2020 4:27:53 PM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-bs-rd-nav-deselected' while resolving lookups for '-fx-fill' from rule '*.nav-button *.text' in stylesheet jar:file:/Applications/Bisq.app/Contents/Java/Bisq-1.2.9.jar!/bisq/desktop/bisq.css
Apr 06, 2020 4:27:53 PM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-bs-rd-nav-selected' while resolving lookups for '-fx-fill' from rule '*.nav-button:selected *.text' in stylesheet jar:file:/Applications/Bisq.app/Contents/Java/Bisq-1.2.9.jar!/bisq/desktop/bisq.css
Apr 06, 2020 4:27:54 PM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-bs-rd-nav-deselected' while resolving lookups for '-fx-fill' from rule '*.nav-button *.text' in stylesheet jar:file:/Applications/Bisq.app/Contents/Java/Bisq-1.2.9.jar!/bisq/desktop/bisq.css
Apr 06, 2020 4:27:54 PM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-bs-rd-nav-deselected' while resolving lookups for '-fx-fill' from rule '*.nav-button *.text' in stylesheet jar:file:/Applications/Bisq.app/Contents/Java/Bisq-1.2.9.jar!/bisq/desktop/bisq.css
Apr 06, 2020 4:27:54 PM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-bs-rd-nav-selected' while resolving lookups for '-fx-fill' from rule '*.nav-button:selected *.text' in stylesheet jar:file:/Applications/Bisq.app/Contents/Java/Bisq-1.2.9.jar!/bisq/desktop/bisq.css

@wiz
Copy link
Contributor

wiz commented Apr 6, 2020

Yeah, you can't hide those lines... Here's an example of what is breaking

Expected
Screen Shot 2020-04-06 at 19 55 39

Actual
Screen Shot 2020-04-06 at 19 53 03

the red text is now invisible

@ghost
Copy link

ghost commented Apr 6, 2020

#4019 is a css issue, if you have time to look into it.

@pedromvpg
Copy link
Member Author

pedromvpg commented Apr 6, 2020

@wiz Thanks for the comments and review. I'll resolve the issues on the error log! Should be easy.
Can you tell me how to access that log?

Regarding the fields. Let me see if I can create an alternative that maybe adds a border around the whole field when focused and when invalid. I think it's a more elegant design. Will address it ASAP.

@jmacxx I'll try to resolve this one too! thanks for the heads up!!

OPTION 1 - Outlines
Fields Option 1

OPTION 2 - Less lines, less red
Fields Option 2

@pedromvpg
Copy link
Member Author

Added a focus state, an error state, and put back the invalid helper text.

Screen Shot 2020-04-07 at 11 15 12 AM

@pedromvpg
Copy link
Member Author

#4019 is a css issue, if you have time to look into it.

There's something strange overriding the definition of the color on darkmode. Need a little longer to fix this.

@ghost
Copy link

ghost commented Apr 10, 2020

See also #3957 (comment) when you get the chance.

@ripcurlx ripcurlx added this to the v1.3.3 milestone Apr 13, 2020
Comment on lines +268 to +276









Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for that blank lines?

@@ -65,8 +65,8 @@
}

.headline-label {
-fx-font-weight: bold;
-fx-font-size: 1.692em;
/*-fx-font-weight: bold;*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Just delete if if not needed anymore

@@ -154,7 +155,7 @@
-fx-pref-height: 32;
-fx-min-height: -fx-pref-height;
-fx-padding: 0 40 0 40;
-fx-effect: dropshadow(gaussian, -bs-text-color-transparent, 2, 0, 0, 0, 1);
/*-fx-effect: dropshadow(gaussian, -bs-text-color-transparent, 2, 0, 0, 0, 1);*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here - just delete it if not required anymore

.jfx-combo-box {
-jfx-focus-color: -bs-color-primary;
-jfx-unfocus-color: -bs-color-gray-line;
-fx-background-color: -bs-background-color;
/*-jfx-disable-animation: true;*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to disable animations only for the combobox?

Comment on lines 226 to 228



Copy link
Contributor

Choose a reason for hiding this comment

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

Are those empy lines by intent?

Comment on lines 234 to 235


Copy link
Contributor

Choose a reason for hiding this comment

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

Are those empy lines by intent?

-fx-padding: 0.333333em 0.333333em 0.333333em 0.333333em;
/*-jfx-disable-animation: true;*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't it work doing it like that?

Comment on lines 348 to 352





Copy link
Contributor

Choose a reason for hiding this comment

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

Are those empy lines by intent?

Comment on lines 391 to 394




Copy link
Contributor

Choose a reason for hiding this comment

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

Are those empy lines by intent?

@@ -347,10 +420,15 @@ tree-table-view:focused {
-fx-font-size: 1em;
}


.field-label {
/*-fx-text-fill: red;*/
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that was still for testing, based on our last call?

@@ -430,6 +508,9 @@ tree-table-view:focused {
.jfx-toggle-button:focused:selected {
-jfx-toggle-color: -bs-color-primary-dark;
-jfx-size: 8;
-jfx-disable-visual-focus: true;
/*-jfx-disable-animation: true;*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't it work as expected?

@ripcurlx
Copy link
Contributor

Maybe lets have a chat where I can point you in the right direction for the last few issues and I'm happy to try out this new style adaption.

@stale
Copy link

stale bot commented May 30, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the was:dropped label May 30, 2020
@wiz
Copy link
Contributor

wiz commented Jun 2, 2020

hey @pedromvpg wanna talk about this

@stale stale bot removed the was:dropped label Jun 2, 2020
@ripcurlx ripcurlx modified the milestones: v1.3.4, v1.3.6 Jun 4, 2020
@boring-cyborg boring-cyborg bot added in:altcoins is:no-priority PR or issue marked with this label is not up for compensation right now labels Jun 20, 2020
@pedromvpg pedromvpg requested a review from cbeams as a code owner June 20, 2020 22:39
@cbeams
Copy link
Contributor

cbeams commented Jun 22, 2020

Create MempoolFeeRateProvider 2.java

Hm... looks like this was an accidental addition; correct, @pedromvpg?

@pedromvpg
Copy link
Member Author

I screwed up this pull request. Most likely will have to start over and break it down into smaller scopes.

@wiz
Copy link
Contributor

wiz commented Jun 24, 2020

I got you bro. See PR #4334

wiz added a commit to wiz/bisq that referenced this pull request Jun 29, 2020
* Increase overall darkness
* Clean up tables and increase background contrasts
* Add background color to disabled buttons
* Remove large padding on read only fields to fix Create Offer scroll
* Darken color of dark theme grey speech bubble
* Remove negative padding from toggle switches
* Add background to password field
* Closes bisq-network#4121

Author: pedromvpg <[email protected]>
Co-Authored-By: wiz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in:altcoins is:no-priority PR or issue marked with this label is not up for compensation right now
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants