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.

Sign up for GitHub

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

fix(linux): Fix Window::theme may return incorrect theme #800

Merged
merged 3 commits into from
Oct 16, 2023
Merged

fix(linux): Fix Window::theme may return incorrect theme #800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
87ae886
fix(linux): Fix `Window::theme` may return incorrect theme
rhysd Oct 13, 2023
1236933
Return preferred theme on `Window::theme` if available
rhysd Oct 16, 2023
94bf325
Remove some redundant clones
rhysd Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/get-window-theme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"tao": "patch"
---

Fix `Window::theme` may return a theme different from the actual window's theme on Linux.
32 changes: 19 additions & 13 deletions src/platform_impl/linux/window.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ pub struct Window {
inner_size_constraints: RefCell<WindowSizeConstraints>,
/// Draw event Sender
draw_tx: crossbeam_channel::Sender<WindowId>,
preferred_theme: Option<Theme>,
}

impl Window {
Expand Down Expand Up @@ -183,15 +184,13 @@ impl Window {
window.set_icon(Some(&icon.inner.into()));
}

let settings = Settings::default();

if let Some(settings) = settings {
let preferred_theme = if let Some(settings) = Settings::default() {
if let Some(preferred_theme) = attributes.preferred_theme {
match preferred_theme {
Theme::Dark => settings.set_gtk_application_prefer_dark_theme(true),
Theme::Light => {
let theme_name = settings.gtk_theme_name().map(|t| t.as_str().to_owned());
if let Some(theme) = theme_name {
if let Some(theme) = settings.gtk_theme_name() {
let theme = theme.as_str();
// Remove dark variant.
if let Some(theme) = GTK_THEME_SUFFIX_LIST
.iter()
Expand All @@ -204,7 +203,10 @@ impl Window {
}
}
}
}
attributes.preferred_theme
} else {
None
};

if attributes.visible {
window.show_all();
Expand Down Expand Up @@ -303,6 +305,7 @@ impl Window {
minimized,
fullscreen: RefCell::new(attributes.fullscreen),
inner_size_constraints: RefCell::new(attributes.inner_size_constraints),
preferred_theme,
};

win.set_skip_taskbar(pl_attribs.skip_taskbar);
Expand Down Expand Up @@ -773,15 +776,18 @@ impl Window {
}

pub fn theme(&self) -> Theme {
if let Some(settings) = Settings::default() {
let theme_name = settings.gtk_theme_name().map(|s| s.as_str().to_owned());
if let Some(theme) = theme_name {
if GTK_THEME_SUFFIX_LIST.iter().any(|t| theme.ends_with(t)) {
Copy link
Member

Choose a reason for hiding this comment

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

If we choose to keep a theme state, we probably don't need this constant.
Can you remove GTK_THEME_SUFFIX_LIST's definition as well?

Copy link
Member

Choose a reason for hiding this comment

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

We still need to check using GTK_THEME_SUFFIX_LIST because a developer may not specify an explicit theme to be set so it could be either light or dark but this PR implementation assumes that it will always be light theme.

return Theme::Dark;
}
if let Some(theme) = self.preferred_theme {
return theme;
}

if let Some(theme) = Settings::default().and_then(|s| s.gtk_theme_name()) {
let theme = theme.as_str();
if GTK_THEME_SUFFIX_LIST.iter().any(|t| theme.ends_with(t)) {
return Theme::Dark;
}
}
return Theme::Light;

Theme::Light
}
}

Expand Down
Loading