-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Fix custom gr.HTML updates #12592
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
Fix custom gr.HTML updates #12592
Conversation
🪼 branch checks and previews
Install Gradio from this PR pip install https://gradio-pypi-previews.s3.amazonaws.com/9541b32c2c3b43a0ff600bc27d764018708e49d7/gradio-6.1.0-py3-none-any.whlInstall Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@9541b32c2c3b43a0ff600bc27d764018708e49d7#subdirectory=client/python"Install Gradio JS Client from this PR npm install https://gradio-npm-previews.s3.amazonaws.com/9541b32c2c3b43a0ff600bc27d764018708e49d7/gradio-client-2.0.0.tgz |
🦄 change detectedThis Pull Request includes changes to the following packages.
✅ Changeset approved by @freddyaboulton
|
| """, | ||
| container=container, | ||
| label=label, | ||
| ordered=ordered, |
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.
Do you still need to pass attributes to the parent HTML class?
| props = None | ||
| if hasattr(block, "props"): | ||
| props = {k: v for k, v in update_dict.items() if not hasattr(block, k)} | ||
| if isinstance(block, HTML): |
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.
Might be worth adding a comment here explaining that this is needed for custom html components since it might not be clear to someone reading the codebase why this is needed
abidlabs
left a comment
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.
LGTM nice fix!
freddyaboulton
left a comment
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.
LGTM nice @aliabid94 !
If you tried updating an extended gr.HTML component, if you set the props, it wouldn't work if you also set the prop to self.prop_name. E.g.:
The line
self.todos = todosstopped updates from changing todos, because previously we were relying on class props to determine if they were part of gr.HTML or if they should be part of additional props. The above is fixed, we just read the gr.HTML constructor directly.