-
Notifications
You must be signed in to change notification settings - Fork 729
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
Conversation
c190f50
to
5ec5d06
Compare
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
Relevant changes:
Relevant changes:
Relevant changes:
Relevant changes:
NOTE: @isabela-pf will update this grey scale in the color system to make these two states more different.
Relevant changes -Background color for buttons
Relevant changes -Background color of the menu
Relevant changes -Background color of focus state of items (Less brighter blue) 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. |
@ccordoba12 and @dpizetta let me know if you have any questions or comments about this! Hope the screenshots make the changes more clear. |
Right now, I think the best option is to change Gray B20 to Do you want me to make this change as a separate small PR, or add it in to this one? |
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):
|
About your initial comments:
This needs to be fixed in our side by finding more contrasting colors for our icons.
I totally agree with this. I hope @dpizetta agrees too. |
Finally, could you also post screenshots with QDarkstyle's example file? That was described by @dpizetta in this comment: #260 (comment). |
I have these two options for the color above/below tabs.
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. |
I like option 1. more than 2. @isabela-pf, what do you think?
Thanks @juanis2112. I think all changes look pretty good, except for the
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. |
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). This might look too similar to "normal text", but let me know if you prefer this option. |
For the tab colors, I also like the first option using 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. 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! |
Just so you know, @juanis2112 and I also talked about the hover state for images (in From left to right this shows the standard white icon on |
Thanks @isabela-pf! |
The last commit fixes some issues that I found while running the example file plus other pending ones from spyder:
|
My thoughts on @isabela-pf comments:
I also agree text is quite readable and looks pretty good with B80, so let's go with it.
Very good thinking. I also agree with this change. |
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.
I also think it's not ideal but I understand it messes checkboxes (which look pretty good).
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.
I guess this is another solution, right? However, I'd like to explore the previous one first. |
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. |
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!
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:
Thanks! |
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 |
As discussed in the meeting the last commit implements the following changes:
I couldn't implement the following changes:
|
@ccordoba12 noticed there was a weird space in the comboboxes between the "matches" field and the border: The last commit removes this gap. Here is how it looks now: Let me know if you agree with this change. |
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. |
895b131
to
534e17a
Compare
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! |
@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. Also, note the title bar of dock widgets, it looks different from yours. Tks |
You are right @dpizetta, sorry about that. I added the border back in the last commit. It is now looking like this: 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.
Also I'm not quite sure what you mean by this. Is there anything else I need to fix regarding the dock widget? |
I also removed the borders for tooltips because it looks bad in light theme. Let me know if you agree with this change @dpizetta |
@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: |
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. |
@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:
|
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 ? |
@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. |
Sounds good! Thanks! |
This PR makes the changes to the colors of QDarkstyle as discussed in spyder-ide/ux-improvements#13
Summary of what we did:
Changed the values of some of the colors to match the scale proposed initially
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.
Changed "foreground color" group to "text color" group and "selection" to "accent" so that it is more clear what the colors are used for
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
Added new colors (specially some background ones to handle hover, deactivate, pressed and normal state of buttons for different toolbars)
Missing: