-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
PWA icon customization #10341
PWA icon customization #10341
Conversation
🪼 branch checks and previews
Install Gradio from this PR pip install https://gradio-pypi-previews.s3.amazonaws.com/01322940db1b16c8e56716dbca563a634eef7018/gradio-5.12.0-py3-none-any.whl Install Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@01322940db1b16c8e56716dbca563a634eef7018#subdirectory=client/python" Install Gradio JS Client from this PR npm install https://gradio-npm-previews.s3.amazonaws.com/01322940db1b16c8e56716dbca563a634eef7018/gradio-client-1.10.0.tgz Use Lite from this PR <script type="module" src="https://gradio-lite-previews.s3.amazonaws.com/01322940db1b16c8e56716dbca563a634eef7018/dist/lite.js""></script> |
🦄 change detectedThis Pull Request includes changes to the following packages.
With the following changelog entry.
Maintainers or the PR author can modify the PR title to modify this entry.
|
gradio/blocks.py
Outdated
@@ -2450,6 +2452,7 @@ def reverse(text): | |||
block.key = f"__{block._id}__" | |||
|
|||
self.pwa = utils.get_space() is not None if pwa is None else pwa | |||
self.pwa_icon = pwa_icon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the original comment, the user specifically asks for the pwa icon to reflect the value of favicon_path
. It would be cumbersome to expect users to put the same path twice for two separate parameters. I think we should just set the pwa_icon to be the favicon and remove the pwa_icon parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, if we get request for further customization, then I suggest we let developers specify a complete manifest.json
file via a parameter. Anything else seems too specific of a parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, that's correct.
I updated the code.
I tested this here, and I'm no longer seeing the PWA installation. Checking the application debugging console: |
Side note, we don't have much documentation about pwa. Might be good to add a section here: https://www.gradio.app/guides/sharing-your-app |
Thanks,
|
|
But I thought we enable it automatically on Spaces? |
Sorry that's true. |
We need to configure the SSR server to proxy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing @whitphx, works great!
Just a nit: in order to reduce the repo size, can you put the gif and image in this Hugging Face Dataset instead? https://huggingface.co/datasets/huggingface/documentation-images/tree/main/gradio-guides
And load the images from there. That's what we do throughout our guides, e.g.
![](https://huggingface.co/datasets/huggingface/documentation-images/resolve/main/df-highlight.png) |
Thanks, done! |
Description
Customizable PWA icon.
Background: https://discord.com/channels/879548962464493619/1326796863944265810/1326796863944265810
Closes #10329