(I rebased onto master to fix the merge conflict)

For the most part, this seems good. Could you add a function to WindowExtWindows to allow users to retrieve whether dark mode is active or not? We should let users change their theme based on the OS option if they can.

EDIT: Ideally, we'd expose some sort of event that would notify users when the theme change has occurred, but we don't currently expose any mechanism for platform-specific messages and I don't know if this can be abstracted across multiple platforms. Neither of those are unsolvable problems, but I don't know if we want to solve them here.

Thanks for the review! I've added a field to WindowState to track the current state of dark mode.

Regarding the event, I was also wondering about such. I know MacOS also has a dark mode though know nothing about its integration in APIs. So the message might be relevant on at least two platforms.

I know MacOS also has a dark mode though know nothing about its integration in APIs

Dark mode on macOS is enabled by setting a property in the info.plist file. I'm not sure how you query for it as an application, but to support it there's no support necessary from winit (Alacritty already sets this property).

A quick google for macOS suggests we might be able to recieve a system broadcast for it on that platform too: https://stackoverflow.com/questions/39048894/how-to-detect-switch-between-macos-default-dark-mode-using-swift-3

So it might be appropriate to add a WindowEvent called SystemThemeChanged or DarkModeChanged - which might never get sent on some platforms (for now) but could be sent on (at least) Windows and macOS.

I don't have a mac so can't do the implementation for that platform. If you agree we should add a new WindowEvent I can do the Windows implementation as part of this PR and then open an issue for someone else to follow up with mac support.

@davidhewitt I'm fine with taking that approach.

Great - have now added an event DarkModeChanged and confirmed it fires during the window example.

Just a heads up; I recently updated my system to the latest Windows Insiders and it appears that this definition: const DWMWA_USE_IMMERSIVE_DARK_MODE: DWORD = 19; now needs to be 20 for the window example to run successfully on my system. I've no idea if this is an intended change or a bug or what.

The windows terminal source doesn't appear to have changed from 19 to 20, but that still works fine, which is puzzling. Might be worth holding off on merging this until I figure out what's going on.

I've updated the to use a ThemeChanged event, and created a workaround for my problem. The best solution I could come up with is checking the Windows 10 build version and subsequently choosing the appropriate value for DWMA_USE_IMMERSIVE_DARK_MODE.

@Osspial This is, afaik, good for merge?

@davidhewitt Sorry it took so long to get back to you on this. I have no idea what's going on with dark mode on your system, since 8ba2055 completely broke this for me. There are two primary issues:

• WIN10_BUILD_VERSION is defined as 4026549603 on my system, far outside the range defined in the source file.
• Setting DWMWA_USE_IMMERSIVE_DARK_MODE to 20 causes the DwmSetWindowAttribute to fail. 19 works fine.

I'm inclined to remove the windows version check and publish this with DWMWA_USE_IMMERSIVE_DARK_MODE = 19.

I'm inclined to remove the windows version check and publish this with DWMWA_USE_IMMERSIVE_DARK_MODE = 19.

I would happily undo the version check! Strange about my system then. Are you on a stable windows 10 release or an insiders build?

Rebased on master.

@Osspial I've changed the method used for querying the Windows 10 build version. Does this new method now return a build version within the expected range on your machine?

(I'm currently on Windows 10 insiders build 19018, and WIN10_BUILD_VERSION is coming out as 19018)

If this new method doesn't work on your box, then I'll remove the version checking and hard-code 19 again. Though it might be great if someone else on Windows 10 insiders can report their build number and also whether 19 or 20 is the correct definition of DWMWA_USE_IMMERSIVE_DARK_MODE for them.

I will also try downgrading my system to the Windows 10 November 2019 update (build number 18363, I believe), and check what the behavior is on there. My suspcision is that in that version 19 is still the correct ordinal. That'll give me an opportunity to then reinstall insiders and confirm that it's not just my system being bugged somehow.

So I think there's still a little bit of research to do before we can merge this :(

On my machine WIN10_BUILD_VERSION is reported as 18362. This matches what is displayed by winver. I can confirm that the dark theme works as intended, including changing the theme while a window is open.

I had access to a machine with windows build 17763 on it, tried this out and it works as intended, WIN10_BUILD_VERSION was also reported correctly.

A tiny thing that I noticed: it doesn't change theme when the window isn't focused, but it will switch to the correct one as soon as it's focused again.

Thanks guys. Anyone else also on Windows 10 insiders builds greater than 18363? Looks like if there was an API change (if there is indeed one and it's not my system) it ocurred on a build later than that.

A tiny thing that I noticed: it doesn't change theme when the window isn't focused, but it will switch to the correct one as soon as it's focused again.

Interesting. Did you happen to notice at what time the ThemeChanged event is being fired in the window example? Is it when it switches into focus, or when you change the dark theme setting? (On my system the ThemeChanged event fires when I change the system setting, regardless of focus.)

Did you happen to notice at what time the ThemeChanged event is being fired in the window example?

It fires as soon as I change the system setting, even though it will only actually switch theme when I focus the window again.

Same behaviour as on your system.

It fires as soon as I change the system setting, even though it will only actually switch theme when I focus the window again.

Cool. So we can probably make it switch theme sooner if we force Windows to redraw the title bar.

Past experiments I've had into trying to do that have never been very successful... anyone know a nice API to force the title bar to redraw? 😄

Past experiments I've had into trying to do that have never been very successful... anyone know a nice API to force the title bar to redraw? 😄

I was playing around with this for a bit and as far as I could find, using SetWindowPos with SWP_FRAMECHANGED should fix this, but it doesn't. The only thing I could find is resizing the window to a new size, any size does it, just not the original one.

So the only workaround I could come up with, is if the window isn't focused, we want to resize the window by one pixel and than back again.

So the only workaround I could come up with, is if the window isn't focused, we want to resize the window by one pixel and than back again.

Yeah that's similar to the only thing I could find too. Feels rather hacky to me. Given that this focus issue only applies to an out-of-date Windows 10 build, my vote would probably be to not introduce such a workaround. :(

Given that this focus issue only applies to an out-of-date Windows 10 build

I didn't know this issue is not present in newer builds? I will try on a newer build now and report back.

I didn't know this issue is not present in newer builds? I will try on a newer build now and report back.

I'm now doubting myself. Pretty sure on my system (most recent Windows Insiders build) the frame changes whether focussed or not. I'll check later.

Just tried it on the newest build, works as intended.

You were right, this only affects older versions of windows. I would consider this a bug on windows, not our problem.

Out of curiosity, how are you testing so many Win10 versions so quickly? VMs?

Hahaha, that would be epic! 😄
My main windows machine is running LTSC, which is currently stuck at 17763, but my tablet is running 18362, so we just happen to be fortunate 😁.

Thanks; I switched to use get_function! and LoadLibraryA.

Thanks again, have addressed all those suggestions.

Looking really good now. Great work @davidhewitt

Sorry about the delay reviewing this. Once my nit is addressed this should be good to go. Thank you for researching and implementing this!

Nit fixed, and rebased - expecting the CI to be green and fingers crossed this is good to merge!