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

Change colors of qdarkstyle according to new ux palette #262

Merged
merged 15 commits into from
Mar 24, 2021

Conversation

juanis2112
Copy link
Contributor

@juanis2112 juanis2112 commented Feb 18, 2021

This PR makes the changes to the colors of QDarkstyle as discussed in spyder-ide/ux-improvements#13

Summary of what we did:

  1. Changed the values of some of the colors to match the scale proposed initially

  2. Changed the scale of each group of colors to numerical instead of having DARK, LIGHT and NORMAL, so that we could be able to add more colors if necessary. Color 1 is the darkest for all the scales and it gets lighter as it goes up.

  3. Changed "foreground color" group to "text color" group and "selection" to "accent" so that it is more clear what the colors are used for

  4. Matched all the existing colors to some of the new ones like this:

    COLOR_BACKGROUND_DARK -> COLOR_BACKGROUND_1
    COLOR_BACKGROUND_NORMAL -> COLOR_BACKGROUND_4
    COLOR_BACKGROUND_LIGHT -> COLOR_BACKGROUND_6

    COLOR_FOREGOUND_DARK -> COLOR_TEXT_1
    COLOR_FOREGOUND_NORMAL -> COLOR_TEXT_3
    COLOR_FOREGOUND_LIGHT -> COLOR_TEXT_4

    COLOR_SELECTION_DARK -> COLOR_ACCENT_1
    COLOR_SELECTION_NORMAL -> COLOR_ACCENT_2
    COLOR_SELECTION_LIGHT -> COLOR_ACCENT_3

    BORDER_DARK -> BORDER_1
    BORDER_NORMAL -> BORDER_2
    BORDER_LIGHT -> BORDER_3

    BORDER_SELECTION_DARK -> BORDER_SELECTION_1
    BORDER_SELECTION_NORMAL -> BORDER_SELECTION_2
    BORDER_SELECTION_LIGHT -> BORDER_SELECTION_3

  5. Added new colors (specially some background ones to handle hover, deactivate, pressed and normal state of buttons for different toolbars)

Missing:

  • Update main.scss to make the proper descriptions of the new colors (Will be done once the changes are approved)
  • Figure out what to do about the icon colors in the toolbar since the color selected for hover only works for white icons. (This is probably not part of this PR but it has to be addressed at some point)
  • Probably remove the border color for a lot of the items. After our discussions we decided to change how we handle the hover state of buttons: it changes the background of the button, not the border (it was blue originally) so I think most of the borders are not needed for hover/active states of buttons but I haven't removed all of the code for these borders, just assigned the same background color to the border so it is not noticeable.

@juanis2112 juanis2112 changed the title Change colors of qdarkstyle according to new ux palette (WIP) Change colors of qdarkstyle according to new ux palette Feb 18, 2021
@juanis2112
Copy link
Contributor Author

I'll try to post some screenshots and describe the main changes that can be seen in all of them so hopefully I can cover the majority of the changes

  1. General view of main window

Relevant changes:

  • Color of main toolbar and status bar
  • Color of tabs in focus and not-focus states
  • Color of scrollbars

Screenshot 2021-02-18 at 5 36 26 PM

  1. Main Toolbar

Relevant changes:

  • Background color for deactivated buttons
  • Hover state for buttons in toolbar. (Changed from blue border to grey background)
  • Focus state for widgets is the brighter blue now and the hover is the more opaque now. (We inverted those two states because it made more sense that the focus state is more noticeable than the hover one)

Screenshot 2021-02-18 at 5 58 17 PM

  1. Tooltips

Relevant changes:

Screenshot 2021-02-18 at 5 51 19 PM

  1. Pane toolbars

Relevant changes:

  • Hover state for buttons in toolbar. (Changed from blue border to grey background)
  • Pressed state for buttons in toolbar. (Changed from blue border to grey background lighter than the hover one)

NOTE: @isabela-pf will update this grey scale in the color system to make these two states more different.

Screenshot 2021-02-18 at 5 53 13 PM

  1. Dialogs/Buttons

Relevant changes

-Background color for buttons
-Hover state for buttons. (Changed from blue border to lighter background)
-Pressed state for buttons. (Changed from blue border to lighter background, it is lighter than the hover)

button

  1. Menus

Relevant changes

-Background color of the menu
-Color of hover in menus
-Color of text for deactivated options

Screenshot 2021-02-18 at 6 09 09 PM

  1. List widgets

Relevant changes

-Background color of focus state of items (Less brighter blue)
-Background color of hover state of items (It was too similar to the color of the header bar so now it is lighter)
-The text color of the hover state of pressed items was changing to dark but we think it shouldn't change since there is no need for hover state in pressed items.

Note: Haven't figure out how to change the colors of the left side bar with the triangles but we still need to decide what to do about it.

Screenshot 2021-02-18 at 6 12 46 PM

@juanis2112
Copy link
Contributor Author

juanis2112 commented Feb 18, 2021

@ccordoba12 and @dpizetta let me know if you have any questions or comments about this! Hope the screenshots make the changes more clear.
Thanks @isabela-pf for all your work!! I think this is looking amazing already

@isabela-pf
Copy link
Contributor

NOTE: @isabela-pf will update this grey scale in the color system to make these two states more different.

Right now, I think the best option is to change Gray B20 to #293544. It's still a little dark, but it's not so light it can be confused with the pressed state by accident.

Do you want me to make this change as a separate small PR, or add it in to this one?

@ccordoba12
Copy link
Collaborator

Thanks @juanis2112 for your work on this! It looks really amazing, as you said, and I think it's almost ready.

I have some small suggestions (these are the ones I remember from our meeting):

  • Find a more contrasting shade of blue for the line above/below tabs. In case that's not possible, we should remove it.
  • Text for disabled buttons doesn't have enough contrast. Changing this could have unexpected ripple effects for other colors in the palette, but @isabela-pf will explore the possibility.
  • We should disable hover state for selected entries in the Files pane because it seems unnecessary and it doesn't look good either.

@ccordoba12
Copy link
Collaborator

About your initial comments:

Figure out what to do about the icon colors in the toolbar

This needs to be fixed in our side by finding more contrasting colors for our icons.

Probably remove the border color for a lot of the items

I totally agree with this. I hope @dpizetta agrees too.

@ccordoba12
Copy link
Collaborator

Finally, could you also post screenshots with QDarkstyle's example file? That was described by @dpizetta in this comment: #260 (comment).

@juanis2112
Copy link
Contributor Author

juanis2112 commented Feb 25, 2021

I have these two options for the color above/below tabs.

  1. This is the lightest blue in our accent colors in the palette (Currently COLOR_ACCENT_4 = Blue.B70). The one between the current one and this one is already used for hover.

Screenshot 2021-02-25 at 1 12 06 PM

  1. If this is too bright, we can adjust COLOR_ACCENT_4 to be Blue.B60 instead which would look like this

Screenshot 2021-02-25 at 1 18 11 PM

I honestly prefer option 1 because I think option 2 makes color for the bar in selected and hover tabs too similar.

@isabela-pf and @ccordoba12 let me know what you think.

@juanis2112
Copy link
Contributor Author

The last commit fixes the following issues discussed in our last meeting:

Color for disabled text:

Screenshot 2021-02-25 at 2 22 02 PM

Color for x’s in tabs:

Screenshot 2021-02-25 at 2 23 29 PM

Color for arrows in files pane:

Screenshot 2021-02-25 at 2 24 01 PM

Bar in the top/bottom of tabs: (left it with brighter blue but let me know if it needs change)

Screenshot 2021-02-25 at 2 24 22 PM

General screenshot to show the changes in context:
Screenshot 2021-02-25 at 2 25 06 PM

@ccordoba12
Copy link
Collaborator

ccordoba12 commented Feb 26, 2021

I have these two options for the color above/below tabs

I like option 1. more than 2. @isabela-pf, what do you think?

The last commit fixes the following issues discussed in our last meeting

Thanks @juanis2112. I think all changes look pretty good, except for the Color for disabled text, but that's something @isabela-pf will take a look at.

General screenshot to show the changes in context

They look great, but you haven't still uploaded the screenshots with QDarkStyle example file, as I mentioned in #262 (comment). That will allow us to check how these changes affect other Qt widgets not part of Spyder.

@juanis2112
Copy link
Contributor Author

except for the Color for disabled text, but that's something @isabela-pf will take a look at.

I think this is the color we agreed on during the meeting. I changed COLOR_TEXT_4 from B60 to B70 (Lighter and hence it has more contrast).
However, here is the next lighter one which is B80:

Screenshot 2021-02-25 at 11 53 16 PM

This might look too similar to "normal text", but let me know if you prefer this option.

@isabela-pf
Copy link
Contributor

For the tab colors, I also like the first option using COLOR_ACCENT_4 = Blue.B70.

For the disabled button text, I made versions of them side by side for easier comparison. The whole bottom row is the current active text as a baseline.
Screen Shot 2021-02-25 at 10 09 52 PM

Personally, I like B80's contrast, but I could see it getting a little close to the active button states. I think B70 is probably a good choice here.

Thanks for providing all these screenshots, @juanis2112, they are really helpful!

@isabela-pf
Copy link
Contributor

Just so you know, @juanis2112 and I also talked about the hover state for images (in images.py) because since the icons are on so many different backgrounds, it's a challenge to find a hover state that works well for all of them. We ended up choosing COLOR_ACCENT_4 because the blue matches the hover state pattern that many other elements have (besides buttons) and this color has the best contrast background grays. I mocked up an example of this, though much larger than any example in the interface.

Screen Shot 2021-02-26 at 7 59 28 AM

From left to right this shows the standard white icon onCOLOR_BACKGROUND_1, hover on COLOR_BACKGROUND_1, hover on COLOR_BACKGROUND_3, and hover on COLOR_BACKGROUND_6.

@juanis2112
Copy link
Contributor Author

Thanks @isabela-pf!

@juanis2112
Copy link
Contributor Author

The last commit fixes some issues that I found while running the example file plus other pending ones from spyder:

  1. Remove hover from selected items in Widgets like files pane (as we discussed in the meeting)
Screenshot 2021-02-26 at 6 54 33 PM
2. Fix background color of horizontal
Screenshot 2021-02-26 at 6 55 10 PM scrollbars
3. Fix Checked, pressed and hover states of checkable buttons (not present in spyder)
Screenshot 2021-02-26 at 6 55 41 PM
4. Remove blue border from PushButton to match style of the rest of buttons 5. Fix hover state for QToolbuttons 6. Fix Slider style to match Qscrollbar style (note present in spyder)
Screenshot 2021-02-26 at 6 56 17 PM
7. Fix focus color of images (x's in tabs, arrows in files pane, checkboxes in preferences, radio buttons) Note: We tried several colors with Isabela and arrived to the conclusion that this is the best option. Even though the contrast is not desirable for the x in the tabs, any other color makes it not work for the checkboxes. The other (more complicated) option would be to handle this hover state for images separately for these two items since this is done in line 50 of `images.py`. We think that a possible solution would be to increase the size of “x”s in the tab bars. Though this change should be done in Spyder’s repo.
Screenshot 2021-02-26 at 6 51 21 PM Screenshot 2021-02-26 at 6 51 33 PM Screenshot 2021-02-26 at 6 51 47 PM
8. Fix color for disabled scrollbars and other widgets (not present in spyder)
Screenshot 2021-02-26 at 6 53 11 PM
9. Match border of tabs to top bar in hover state (It was hard to tell the difference but they were different blues before and they should be the same since it should look like one border.
Screenshot 2021-02-26 at 6 53 45 PM

@juanis2112
Copy link
Contributor Author

Here are some screenshots of the QDarkStyle example file. There's lot's of tabs and items so I tried to catch them in this four screenshots but let me know if I'm missing something.

Screenshot 2021-02-26 at 7 16 50 PM

Screenshot 2021-02-26 at 7 17 06 PM

Screenshot 2021-02-26 at 7 17 53 PM

Screenshot 2021-02-26 at 7 18 33 PM

@juanis2112 juanis2112 changed the title (WIP) Change colors of qdarkstyle according to new ux palette Change colors of qdarkstyle according to new ux palette Feb 27, 2021
@juanis2112
Copy link
Contributor Author

Also a gif might illustrate better some things:

Qdark

@ccordoba12
Copy link
Collaborator

ccordoba12 commented Feb 27, 2021

My thoughts on @isabela-pf comments:

Personally, I like B80's contrast, but I could see it getting a little close to the active button states. I think B70 is probably a good choice here.

I also agree text is quite readable and looks pretty good with B80, so let's go with it.

We ended up choosing COLOR_ACCENT_4 because the blue matches the hover state pattern that many other elements have (besides buttons) and this color has the best contrast background grays

Very good thinking. I also agree with this change.

@ccordoba12
Copy link
Collaborator

And about @juanis2112's last big #262 (comment) above, all changes look great to me! I only have these comments to say about point 7.

Fix focus color of images (x's in tabs, arrows in files pane, checkboxes in preferences, radio buttons) Note: We tried several colors with Isabela and arrived to the conclusion that this is the best option. Even though the contrast is not desirable for the x in the tabs, any other color makes it not work for the checkboxes.

I also think it's not ideal but I understand it messes checkboxes (which look pretty good).

The other (more complicated) option would be to handle this hover state for images separately for these two items since this is done in line 50 of images.py.

What do you mean exactly by this? If there's a code modification we could do for this and it's not too disruptive, I think we should implement it.

We think that a possible solution would be to increase the size of “x”s in the tab bars. Though this change should be done in Spyder’s repo.

I guess this is another solution, right? However, I'd like to explore the previous one first.

@juanis2112
Copy link
Contributor Author

juanis2112 commented Mar 2, 2021

After our meeting we decided to add a new accent color corresponding to Blue.B90 (lighter than the rest) for the hover state of images like x in tabs and checkboxes in preferences.

Screenshot for latest changes:

Deactivated text color:
Screenshot 2021-03-02 at 3 54 22 PM

Deactivated text color in context:
Screenshot 2021-03-02 at 3 54 08 PM

Hover state for x in tabs:
Screenshot 2021-03-02 at 3 55 25 PM

Hover state for x in tabs in context:
Screenshot 2021-03-02 at 3 55 35 PM

Hover state for checkboxes in preferences:
Screenshot 2021-03-02 at 3 55 52 PM

Hover state for checkboxes in preferences in context:
Screenshot 2021-03-02 at 3 56 08 PM

@juanis2112
Copy link
Contributor Author

We noticed the hover/pressed states of the undock and close buttons in the dock widget is different than the other buttons so my last commit fixes that to match the style of the rest of the buttons.

However @dpizetta let me know if there is a reason why the previous behaviour one is the desired one.

Screenshot 2021-03-02 at 4 17 15 PM

@dpizetta
Copy link
Collaborator

dpizetta commented Mar 8, 2021

Hi folks, many thanks for those improvements and the whole work you have been done. I agree with all the changes and decisions you have proposed, I read all of them!

However @dpizetta let me know if there is a reason why the previous behaviour one is the desired one.

They should be all the same. Also, you can implement the changes you have mentioned about X in tabs to be in Spyder inside the qdarkstyle.

Some points to improve, we can discuss in the meeting:

  • Decrease a little bit the size of close/undock buttons - about 20%, add spacing left/right. It could bigger than X in tabs.

image

  • Decrease height of dock's title bar, it could be the same height of toolbars:

image

  • Increase/check the font size for the headers (table/list/tree), title bars (docks), and toolbar buttons as they appear smaller than standard texts.

image

  • The header height could be the same as of items, it seems bigger and it costs to much space.

image

  • Add background to tool buttons?

image

Thanks!

@dpizetta
Copy link
Collaborator

dpizetta commented Mar 9, 2021

Sugestion for branch lines reduction:
image
image

@dpizetta
Copy link
Collaborator

Hi guys, just pushed a new branch with the changes in svg. The current changes represent the second option (my vote) of the picture below, so to use it you need to run the process_qrc to update the png files.

In the last one, there is no style applied (Windows) for reference. The orange arrow indicates the bad 'sort indicator' without style (at this point I kept as mentioned in the meeting). Tks

trees

@juanis2112
Copy link
Contributor Author

As discussed in the meeting the last commit implements the following changes:

  • Increase the size for button for closing tabs

Screenshot 2021-03-14 at 9 54 14 PM

  • Apply brighter blue for image hover

Screenshot 2021-03-14 at 9 55 15 PM

Screenshot 2021-03-14 at 9 55 38 PM

  • Decrease size for x in toolbars (also decreased the size of the float button to match the style)

Screenshot 2021-03-14 at 10 14 59 PM

  • Match heights of toolbars

Screenshot 2021-03-14 at 10 15 11 PM

  • Increase font size of QHeaderView

Screenshot 2021-03-14 at 10 18 45 PM

I couldn't implement the following changes:

  • Change hover color for menus: I realized that this color is associated to the items in the file pane which is associated to the selection color in the editor. Not sure if there is a way of affecting one of them without affecting the others but while discussing with @ccordoba12 and @isabela-pf we realised that the selection color in the editor would be too bright if we change these to the brighter shade of blue in the palette. We suggest leaving it as it is for spyder 5 and then maybe try changing one of the blues to work for all of these items if that's okay with you @dpizetta
    Here a screenshot for reference of what I'm trying to say:

Screenshot 2021-03-11 at 8 21 43 PM

  • Decrease size of the header in the files pane: Couldn't actually decrease the size of the header to math the other tabors/toolbars. I realised most of these changes are done by padding, margins and borders but couldn't figure out the right setting for decreasing this height. Any ideas @dpizetta? This is also not critical for us, so if it is too hard we are okay with leaving it as it is for now.
  • Decrease size of blue "x"s and float buttons in the in dock widget. (Also couldn't decrease the height of this one): Couldn't figure our how to affect the size of these images. Again, in the other ones I did it by padding and margins but this doesn't work here. I tried a couple of other stuff but couldn't get it working. Maybe if you know how @dpizetta, I could use some help here.

@juanis2112
Copy link
Contributor Author

juanis2112 commented Mar 15, 2021

@ccordoba12 noticed there was a weird space in the comboboxes between the "matches" field and the border:
Screenshot 2021-03-15 at 3 33 43 PM

The last commit removes this gap. Here is how it looks now:
Screenshot 2021-03-15 at 3 37 19 PM

Screenshot 2021-03-15 at 3 30 05 PM

Let me know if you agree with this change.

@dpizetta
Copy link
Collaborator

Change hover color for menus: I realized that this color is associated to the items in the file pane which is associated to the selection color in the editor. Not sure if there is a way of affecting one of them without affecting the others but while discussing with @ccordoba12 and @isabela-pf we realised that the selection color in the editor would be too bright if we change these to the brighter shade of blue in the palette. We suggest leaving it as it is for spyder 5 and then maybe try changing one of the blues to work for all of these items if that's okay with you @dpizetta

Decrease size of the header in the files pane: Couldn't actually decrease the size of the header to math the other tabors/toolbars. I realised most of these changes are done by padding, margins and borders but couldn't figure out the right setting for decreasing this height. Any ideas @dpizetta? This is also not critical for us, so if it is too hard we are okay with leaving it as it is for now.

Decrease size of blue "x"s and float buttons in the in dock widget. (Also couldn't decrease the height of this one): Couldn't figure our how to affect the size of these images. Again, in the other ones I did it by padding and margins but this doesn't work here. I tried a couple of other stuff but couldn't get it working. Maybe if you know how @dpizetta, I could use some help here.

Thanks, guys. Ok, I think we can skip them for now but opening an issue for each one to deal with it in the future. I'm also not sure how to change those items but I'll try to figure it out.

Abou the combobox padding fix, I have already pushed in the weekend a fix for it using 4px padding as default ones, it was reported in #239. You can rebase your branch to get it synchronized with develop branch, also to see if there is something that breaks for Spyder (when I checked it was ok in the first seem).

Please, check if this is ok on your side in the current version of develop: #263

Tks for all your time on this.

@juanis2112
Copy link
Contributor Author

Thanks @dpizetta! We tested your changes and we think they all seem fine to us. With your changes and the last commit I did, I think the only pending things to fix are the color for hover in menus and the sizes of x and float buttons in the dock widget. I opened issues for both of them so we can work on this after our release. Please let me know if there is something I'm missing.

And thanks for your help again!

@dpizetta
Copy link
Collaborator

@juanis2112 I'm checking your branch, it seems you have changed the border in the toolbars, maybe you can add this config as an internal one in Spyder. For me, it seems better the old format.

New:
image

Old:
image

Also, note the title bar of dock widgets, it looks different from yours. Tks

@juanis2112
Copy link
Contributor Author

juanis2112 commented Mar 22, 2021

You are right @dpizetta, sorry about that. I added the border back in the last commit. It is now looking like this:

Screenshot 2021-03-22 at 2 31 02 PM

Let me know if you want me to restore the 2px padding or if 1px padding is enough. I removed this padding in order to make this toolbar's height the same as the others but some padding is needed to make the border work.

note the title bar of dock widgets, it looks different from yours.

Also I'm not quite sure what you mean by this. Is there anything else I need to fix regarding the dock widget?

@juanis2112
Copy link
Contributor Author

I also removed the borders for tooltips because it looks bad in light theme. Let me know if you agree with this change @dpizetta

@juanis2112
Copy link
Contributor Author

@ccordoba12 and I noticed you reduced one of the numbers for the padding in the Menus from 24 to 8px (line 401 of _styles.scss). This introduced a bug in Linux and Windows because it seems to overlap checkboxes with text in menus:
imagen (1).
I was wondering if you could take a look at this, since I'm not quite sure what's the ideal padding in this case or why did you remove this padding.

@juanis2112
Copy link
Contributor Author

We found a possible solution for the hover in menus, we changed the background color for them to be a little darker so it had more contrast with the blue color for hover.

Screenshot 2021-03-23 at 3 49 18 PM

@juanis2112
Copy link
Contributor Author

The last commit also updates the lightest shades of grey and blue of the color system to fix the light mode but it doesn't impact the dark mode.

@dpizetta
Copy link
Collaborator

@juanis2112 please add screenshots with all screens of qdarkstyle example, so I can compare 'ios' version with linux/windows to properly apply the fixes for X in dock and tabs, and undock button in docks. Tks

Writing the main points of our meeting/issues that I'll implement:

  • Disconnect tree/list selection/hover color with editor selection color, also check the colors;
  • Match/connect pressed menu color with hover/pressed color in the menu (submenus), also check the colors;
  • Remove arrows from special tool buttons (menu like);
  • Match Undock/Close buttons in docks and tabs (OS-dependent);
  • Fix padding for menus with indicators/icons (Qt version dependent, base it on 5.12 LTS);
  • Hover color for indicators (checkbox/radio) in menus (check on Spyder hamburger menu) may be related to the previous item;

@juanis2112
Copy link
Contributor Author

juanis2112 commented Mar 24, 2021

I realized that this hover color for indicators (checkbox/radio) in menus is related to the color_disabled in the images.py. For some reason these "images" seem to be disabled in the Menus. So the way to fix it is to change color_disabled to palette.COLOR_TEXT_1 to match the rest of the icons. However this would make the indicators in disabled items not look disable, for example the "x" in disabled tabs.

Not sure if there is any other solution to this problem, ideas @dpizetta ?

@dpizetta
Copy link
Collaborator

@juanis2112 nice catch. Actually, the images are wrong, look at this line it must be "checkbox_unchecked_selected.png". I remember that we have fixed something similar, I need to check.

I'll merge your PR to develop so I can work on this current version.

@juanis2112
Copy link
Contributor Author

Sounds good! Thanks!

@dpizetta dpizetta merged commit cf0146e into ColinDuquesnoy:develop Mar 24, 2021
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.

4 participants