Skip to content

Add ActiveEventLoop::system_theme()#3838

Merged
kchibisov merged 2 commits intorust-windowing:masterfrom
daxpedda:system-theme
Aug 5, 2024
Merged

Add ActiveEventLoop::system_theme()#3838
kchibisov merged 2 commits intorust-windowing:masterfrom
daxpedda:system-theme

Conversation

@daxpedda
Copy link
Copy Markdown
Member

In #3736 the definition of Window::theme() was changed to only return theme overrides and never the system theme. Which was only how Wayland worked. MacOS was changed to follow this.

However Web and Windows, and MacOS before #3736, currently do return the system theme in Window::theme() when no theme override is set. The changed definition and behavior currently does not make it possible to get the system theme, which leaves a gap in the API that was previously covered.

This PR follows the proposal set in #3837 that reintroduces the old behavior and adds a new ActiveEventLoop::system_theme() to retrieve the system theme without any window overrides.

Follow-up to #3736.
Fixes #3837.

@daxpedda daxpedda added DS - appkit Affects the AppKit/macOS backend DS - win32 Affects the Win32/Windows backend DS - wayland Affects the Wayland backend, or generally free Unix platforms S - api Design and usability DS - web Affects the Web backend (WebAssembly/WASM) S - platform parity Unintended platform differences labels Jul 29, 2024
@daxpedda
Copy link
Copy Markdown
Member Author

Considering that WindowEvent::ThemeChanged only reports system theme changes, which was clarified in #3736 as well (unless you can change the window theme externally?), I would argue that the event should be moved to a global Event and not be Window-specific anymore.

Fix MacOS returning `None` in `Window::theme()` if no theme override is set, instead it now returns the system theme.
MacOS and Wayland were the only ones working correctly according to the documentation, which was an oversight.
The documentation was "fixed" now.
@kchibisov kchibisov merged commit 15b79b1 into rust-windowing:master Aug 5, 2024
Copy link
Copy Markdown
Member

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Posthumously approving 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DS - appkit Affects the AppKit/macOS backend DS - wayland Affects the Wayland backend, or generally free Unix platforms DS - web Affects the Web backend (WebAssembly/WASM) DS - win32 Affects the Win32/Windows backend S - api Design and usability S - platform parity Unintended platform differences

Development

Successfully merging this pull request may close these issues.

System Theme vs Window Theme

3 participants