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

Theme adds support for dicts #518

Merged
merged 15 commits into from
Mar 23, 2024
Merged

Conversation

LondonClass
Copy link
Contributor

fixes #400

I found that update_theming and load_theme perform similar functions... Do they need to be refactored?

My other goal is to enable UIManager to dynamically replace ui_theme during runtime. If this is necessary, I may try to do it in the future.

Copy link

codecov bot commented Feb 16, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 96.25%. Comparing base (8ef9c8d) to head (aeaf490).

Files Patch % Lines
pygame_gui/ui_manager.py 85.71% 3 Missing ⚠️
pygame_gui/core/ui_appearance_theme.py 88.88% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #518      +/-   ##
==========================================
- Coverage   96.26%   96.25%   -0.01%     
==========================================
  Files          88       88              
  Lines       12472    12498      +26     
==========================================
+ Hits        12006    12030      +24     
- Misses        466      468       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MyreMylar
Copy link
Owner

Cool, looks like this could just use a quick unit test of changing a theme by passing in a dictionary instead of a theme file.

My other goal is to enable UIManager to dynamically replace ui_theme during runtime. If this is necessary, I may try to do it in the future.

You should be able to do this fairly easily in the UIMananger with something like:

def load_and_update_theming(theme_file_path: str)
  # load theme data
  self.ui_theme.load_theme(theme_file_path)
  # update UIElements
  for sprite in self.ui_group.sprites():
      sprite.rebuild_from_changed_theme_data()

Or at least it used to work like that at one point because you could edit the theme files while the apps were running and see the changes live. Maybe it still does!

@LondonClass
Copy link
Contributor Author

LondonClass commented Feb 27, 2024

@MyreMylar update_theming and load_theme seem to be doing the same thing. I want to remove the update_theming method and merge its functionality into the load_theme. update_single_element_theming may also related to it.

@LondonClass
Copy link
Contributor Author

You should be able to do this fairly easily in the UIMananger with something like:

def load_and_update_theming(theme_file_path: str)
  # load theme data
  self.ui_theme.load_theme(theme_file_path)
  # update UIElements
  for sprite in self.ui_group.sprites():
      sprite.rebuild_from_changed_theme_data()

Or at least it used to work like that at one point because you could edit the theme files while the apps were running and see the changes live. Maybe it still does!

My idea is to save multiple UIAppearanceTheme instances and dynamically replace them at runtime, so there is no need to read files repeatedly.

@MyreMylar
Copy link
Owner

just need to add a test for update theming and this should pass.

@LondonClass
Copy link
Contributor Author

My idea is to save multiple UIAppearanceTheme instances and dynamically replace them at runtime, so there is no need to read files repeatedly.

Done this.

Copy link
Owner

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

OK, LGTM 👍

@MyreMylar MyreMylar merged commit 347dc4c into MyreMylar:main Mar 23, 2024
11 checks passed
@LondonClass LondonClass deleted the theme-modification branch March 23, 2024 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It would be nice to pass in the theme as a variable defined rather than a file
2 participants