Skip to content

Only set wallpaper/theme when true #364

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

Open
j0yu wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Only set wallpaper/theme when true #364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions yin_yang/plugins/_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ def set_mode(self, dark: bool) -> bool:
if not self.enabled:
return False

theme = self.theme_dark if dark else self.theme_light
self.set_theme(theme)
if (theme := self.theme_dark if dark else self.theme_light):

@l0drex l0drex Feb 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry, I don't really like that syntax. I prefer to have one statement per line, that's less confusing to me.

self.set_theme(theme)
return True

@abstractmethod
Expand Down
12 changes: 6 additions & 6 deletions yin_yang/plugins/wallpaper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,22 +95,22 @@ def __init__(self):
self._theme_dark = None

@property
def theme_light(self) -> str:
def theme_light(self) -> str | None:

@l0drex l0drex Feb 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should never be None, I'd rather return an empty string here

return self._theme_light

@theme_light.setter
def theme_light(self, value: str):
check_theme(value)
self._theme_light = value
if check_theme(value):
self._theme_light = value

@property
def theme_dark(self) -> str:
def theme_dark(self) -> str | None:
return self._theme_dark

@theme_dark.setter
def theme_dark(self, value: str):
check_theme(value)
self._theme_dark = value
if check_theme(value):
self._theme_dark = value

@property
def available(self) -> bool:
Expand Down
12 changes: 5 additions & 7 deletions yin_yang/ui/main_window_connector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,17 +239,15 @@ def update_location(self):
self.load_times()

def select_wallpaper(self, dark: bool):
message_light = self.tr('Open light wallpaper')
message_dark = self.tr('Open dark wallpaper')
file_name, _ = QFileDialog.getOpenFileName(
self, message_dark if dark else message_light,
self, self.tr(f'Open {"dark" if dark else "light"} wallpaper'),

@l0drex l0drex Feb 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

self.tr() gets translations for that message. Since different languages use different, grammar, we shouldn't do that

QStandardPaths.standardLocations(QStandardPaths.PicturesLocation)[0],
'Images (*.png *.jpg *.jpeg *.JPG *.JPEG)')

group_wallpaper = self.ui.plugins_scroll_content.findChild(QtWidgets.QGroupBox, 'groupWallpaper')
inputs_wallpaper = group_wallpaper.findChildren(QtWidgets.QLineEdit)
i = 1 if dark else 0
inputs_wallpaper[i].setText(file_name)
if file_name:

@l0drex l0drex Feb 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, we should definitely handle empty strings better. I would say we should disable the plugin if no valid input was provided.
For example:

  1. add a is_valid() -> bool method to the plugin base class
  2. Call that in _write_config()
  3. If false, don't write and disable the plugin.

Other plugins with text input will have the same issue, so implementing it there makes sense. If available_themes() returns a non-empty dict, we can assume True (we use drop-down menus in that case). If you don't want to implement that for every plugin, just return true in the base class and I will do the rest after merge.

@l0drex l0drex Feb 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I just pushed an implementation for that to master. If any plugin has any theme with an empty string and is enabled, the user will see an error when saving.

group_wallpaper = self.ui.plugins_scroll_content.findChild(QtWidgets.QGroupBox, 'groupWallpaper')
inputs_wallpaper = group_wallpaper.findChildren(QtWidgets.QLineEdit)
inputs_wallpaper[1 if dark else 0].setText(file_name)

def select_color(self, dark: bool, initial_color: QColor):
selected_color = QColorDialog.getColor(initial_color)
Expand Down