-
Notifications
You must be signed in to change notification settings - Fork 931
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
System Theme vs Window Theme #3837
Labels
DS - macos
DS - web
DS - windows
S - api
Design and usability
S - platform parity
Unintended platform differences
Comments
daxpedda
added
DS - macos
DS - windows
S - api
Design and usability
DS - web
S - platform parity
Unintended platform differences
labels
Jul 29, 2024
This was referenced Jul 29, 2024
I like that proposal. Would you accept a PR that could be included in 0.30.x that follows up on #3736 and makes let window = self.window();
Some(appearance_to_theme(unsafe { &*window.appearance().unwrap_or_else(|| &*window.effectiveAppearance()) })) |
A PR (#3838) is already in place. |
Ah great, thanks :) |
7 tasks
This was referenced Jul 30, 2024
kchibisov
pushed a commit
that referenced
this issue
Aug 5, 2024
This also fixes 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. Fixes #3837.
kchibisov
pushed a commit
that referenced
this issue
Aug 7, 2024
This also fixes 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. Fixes #3837.
kchibisov
pushed a commit
that referenced
this issue
Aug 8, 2024
This also fixes 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. Fixes #3837.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
DS - macos
DS - web
DS - windows
S - api
Design and usability
S - platform parity
Unintended platform differences
In #3736 the meaning of
Window::theme()
was changed to not return the system theme, ergo returningNone
without a theme override throughWindow::set_theme()
.This is how currently MacOS works, but not how Web and Windows do.
I propose the following change:
Window::theme()
should be changed to return the current window theme, if no override is in place it returns the system theme. I think a use-case for only returning an override has yet to be observed.Window::theme()
if no theme override is in place.ActiveEventLoop::system_theme()
method should be introduced that returns the system theme.Window::system_theme()
convenience method should be added that returns the same asActiveEventLoop::system_theme()
.Cc @madsmtm.
The text was updated successfully, but these errors were encountered: