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

Add dark mode support for Windows. #2196

Merged
merged 1 commit into from
Jun 6, 2022
Merged

Conversation

dristic
Copy link
Contributor

@dristic dristic commented Jun 4, 2022

Small edit to let Windows theme the title bar with dark mode support. I followed the instructions at the bottom of this page: https://docs.microsoft.com/en-us/windows/apps/desktop/modernize/apply-windows-themes

Here is how it looks:
DarkMode

First time contributing so let me know if I missed anything. Thanks!

@jneem
Copy link
Collaborator

jneem commented Jun 4, 2022

Thanks for the PR! Given that our theme is dark, this makes total sense. But can I also check my understanding of the situation with you? (I don't use windows much, so I might be way off.)

So the page you linked to describes a way of determining whether the theme is "dark" or "light," and the ideal would be to adapt druid's default colors according to that. This PR does not do anything about changing druid's theme, but it does tell windows that druid supports dark themes, so that windows can draw a dark titlebar when a dark theme is enabled. This is a little misleading but it's still an improvement over the status quo (and we'll want this code anyway, once we start actually supporting dark/light modes).

@dristic
Copy link
Contributor Author

dristic commented Jun 5, 2022

Yes you are correct. This allows Windows to restyle the title bar to the system-preferred mode. This is disabled by default for backwards compatibility reasons. It does not allow the application to respond to the system-preferred mode. This would probably take a bit more work to pipe the events up to the application layer through the shell and write the code to detect it. I could look into it in a future change when I have some more time!

@dristic
Copy link
Contributor Author

dristic commented Jun 5, 2022

It looks like the function to query theme color is in the windows-rs library so I would have to pull this in as a dependency to add the theme mode detection functions: https://github.com/microsoft/windows-rs/blob/cbc54970034445e0e50ae548ab8d93768548a4fb/crates/libs/windows/src/Windows/UI/ViewManagement/mod.rs#L3677

@jneem
Copy link
Collaborator

jneem commented Jun 5, 2022

Got it, thanks! I think there was some discussion in the past about moving to windows-rs, but I don't remember the conclusion. And anyway it's definitely out of scope for this PR!

Would you mind adding an entry to the changelog? Other than that, I think this is good to go.

@xStrom
Copy link
Member

xStrom commented Jun 5, 2022

Yes windows-rs is most likely a future dependency. However we've been postponing it because in its current state it causes slow compiles and also links the final binary to libraries that we don't actually need. Microsoft has said that they're working on addressing those issues.

A changelog entry would be nice indeed. You can read about it in our CONTRIBUTING.md. It should probably be clear about the limited nature of this work though. So something like Windows: Dark mode support for the titlebar under the Added section.

@xStrom xStrom added shell/win concerns the Windows backend S-waiting-on-author waits for changes from the submitter labels Jun 5, 2022
@dristic
Copy link
Contributor Author

dristic commented Jun 5, 2022

Add a changelog entry, thanks!

@jneem
Copy link
Collaborator

jneem commented Jun 5, 2022

Sorry, I forgot to tell you: the PR and author links in the changelog aren't magic: they're all defined at the bottom of the file...

@dristic
Copy link
Contributor Author

dristic commented Jun 5, 2022

Fixed, thanks!

@jneem jneem merged commit 5a7947b into linebender:master Jun 6, 2022
@xStrom xStrom removed the S-waiting-on-author waits for changes from the submitter label Jun 6, 2022
@xStrom
Copy link
Member

xStrom commented Jun 8, 2022

Finally got around to testing this and as far as I can tell this doesn't actually have any automatic theme switching by the OS. Now Druid apps always have a dark title bar.

@dristic have you seen the title bar be light after this change?

@dristic
Copy link
Contributor Author

dristic commented Jun 11, 2022

When testing locally I see the same thing. It looks like I have to add the color detection hook in to see if we need to set the title bar to dark or not. I will get this fixed and open another PR. Good catch!

xarvic pushed a commit to xarvic/druid that referenced this pull request Jul 29, 2022
xarvic pushed a commit to xarvic/druid that referenced this pull request Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
shell/win concerns the Windows backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants