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

The metro color $SystemBackground is defined twice, leading to it being non-functional #467

Closed
bonzibudd opened this issue Sep 24, 2020 · 9 comments

Comments

@bonzibudd
Copy link
Member

bonzibudd commented Sep 24, 2020

Describe the bug
I am not completely sure if this can be fixed by Open-Shell or not, but I will put this out anyway. I found this out while trying to make a skin which matches the Taskbar color. I found that I could use $SystemBackground, and it should respect the Taskbar/Windows Start menu/Action Center color. However, I found that this does not seem to work. I took a look back at the Classic Shell utility, and it appears that there are 2 colors called SystemBackground, and only one of them actually adapts to the color, while the other one stays black. I will call back to #262 for reference.

To Reproduce
Steps to reproduce the behavior:

  1. Define $SystemBackground for a prominent Start menu color, like the main bitmap
  2. Compile the skin and apply it
  3. No matter what system background color is set, it will stay black due to the color being faulty.

Expected behavior
The color should adapt to the background color. I tried this method with the AppBackground and it actually works. However, this makes it so that the color does not match if the System mode and App mode are set differently, like it is by default.

Screenshots
image
Here is the color being defined twice in the Classic Shell utility.

Version:

  • Open-Shell: 4.4.156, but likely on older versions as well.
  • OS: Windows 10 2004

Additional context
I don't really know how the Metro colors work, so I can't say if this is a problem with Windows or Open-Shell. I referenced #262 because the best way of accomplishing that is using this color, which seems to be broken. I will note that the text color which goes along with this, $SystemText, is completely functional.

@ge0rdi
Copy link
Member

ge0rdi commented Sep 24, 2020

I took a look back at the Classic Shell utility, and it appears that there are 2 colors called SystemBackground, and only one of them actually adapts to the color, while the other one stays black.

This is because Classic Shell Utility shows in fact two (merged) color lists - metro colors and system colors.
First SystemBackground color is metro one (should be called ImmersiveSystemBackground), the other one is system color (obtained via GetSysColor API).

Now, MenuSkin::GetMetroColor function looks whether color name begins with System and then uses GetSysColor to obtain actual color. Otherwise it prepends Immersive to the name and uses GetImmersiveColorTypeFromName API to obtain metro color.
This means that for colors named System* skin manager never gets metro color.

What I would try is to modify skin manager to allow full Immersive* color names, so they you could use ImmersiveSystemBackground to get actual metro color. I'm not sure it will work, but we can give it a shot if you want to experiment with it.

@bonzibudd
Copy link
Member Author

@ge0rdi

Well, that's an unfortunate coincidence. I knew about the standard "System" colors, but I didn't know that they had to be activated by using the term "system". The solution for "Immersive" naming sounds like a good idea, as long as it doesn't mess up any existing skins and their colors. I would consider that a pretty significant improvement in general if it was implemented.

@ge0rdi
Copy link
Member

ge0rdi commented Sep 24, 2020

The solution for "Immersive" naming sounds like a good idea, as long as it doesn't mess up any existing skins and their colors.

Well, this is what Open-Shell does already. It adds Immersive prefix to all non-system color names. So I guess there should be no problem to allowing such names in skins.

I will prepare build with the change, so you can experiment with it.

@bonzibudd
Copy link
Member Author

@ge0rdi Sounds good!

@ge0rdi
Copy link
Member

ge0rdi commented Sep 24, 2020

Please, try build from PR #468.

Basically you should just replace $SystemBackground with $ImmersiveSystemBackground and see what happens.

@bonzibudd
Copy link
Member Author

It works! It seems like a lot of people wanted an automatic light/dark theme, and this will allow for that.

@CTVCAM8
Copy link

CTVCAM8 commented Sep 24, 2020

If included in a skin, it would have to be made clear to a user that it is for Win10 only.
I think on the Win8 system the menu will always use $ImmersiveSystemBackground as a black reference unless a theme changed the registry [HKEY_CURRENT_USER\Control Panel\Colors] background color to something else?

@ge0rdi
Copy link
Member

ge0rdi commented Sep 25, 2020

I have merged the change. Now you can use either $SystemBackground or $ImmersiveSystemBackground color in skins.

@ge0rdi ge0rdi closed this as completed Sep 25, 2020
@bonzibudd
Copy link
Member Author

Sounds good.

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

No branches or pull requests

3 participants