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

Switch all images to be loaded via base64 #70

Merged
merged 3 commits into from
Aug 22, 2024

Conversation

rohankapoorcom
Copy link
Contributor

@rohankapoorcom rohankapoorcom commented Jun 3, 2024

HACS 2.0 (currently known as "experimental") dropped support for the /hacfiles/themes path for loading images which this theme used for background images. Switching to base64 loaded images is likely the easiest path forward (eliminating the need to manually copy files around and adjust the paths in the theme as many people have been doing in #47). Once the new theme file is loaded by Home Assistant, it just works again 😄!

I've adjusted the create-themes.py script to base64 url encode the images and then inject that directly into the ios-themes.yaml file. I also updated the requirements.txt file with the current set of requirements needed to run the script.

Fixes #62, #47, #50

@basnijholt
Copy link
Owner

This is great!

I wonder if it is possible to use YAML anchors such that we don't have to duplicate the backgrounds in the file.

@rohankapoorcom
Copy link
Contributor Author

I haven't had a chance to test it with themes but I can't think of any reason it wouldn't inherently work. It might make the templating a little complicated though since we'll want to have it iterate over all of the backgrounds first to dump them out at the top and then reference each one.

Not sure how much of a practical problem it actually is though. The theme file is 2.54MB but I don't believe Home Assistant actually loads all of the themes into the browser at once, so it shouldn't actually slow things down for the user.

I'm happy to give it a try though.

@rohankapoorcom
Copy link
Contributor Author

I gave this a shot with the second commit on the branch, but either I got the resultant YAML wrong or Home Assistant doesn't parse the anchors properly. If you are able to get it working, feel free to push up the fix as a new commit; otherwise we can remove the second commit before merging.

@rohankapoorcom
Copy link
Contributor Author

Hey @basnijholt - Just wanted to send a friendly ping to see if you had a chance to review my previous comment.

Thanks!

@Garywoo
Copy link

Garywoo commented Aug 22, 2024

Now that HACS 2.0 has been officially released, I'm hoping this PR can be merged soon 🤞
The breaking change that it introduced prevents the background images of this theme from loading for all of us that eagerly updated the integration. I'd like to make my dashboards look pretty again 😉

@rohankapoorcom
Copy link
Contributor Author

I've reverted the previous nonfunctional commit with the YAML anchor attempt. This should be good to merge as is with no further changes.

@basnijholt
Copy link
Owner

Thanks a lot!

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.

No background? Help Fix
3 participants