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

Add icon and link to DuplicateButton #5111

Merged
merged 8 commits into from
Aug 7, 2023
Merged

Conversation

aliabd
Copy link
Collaborator

@aliabd aliabd commented Aug 7, 2023

PR #5080 added icon and link params to gr.Button and gr.ClearButton (which inherits from Button). But it didn't add them to gr.DuplicateButton and gr.UploadButton which also inherit from Button. That's fine for UploadButton because it defines its own docstrings, but DuplicateButton doesn't and inherits its docstrings from Button. This broke website docs generation because it was looking for the icon/link params that exist in the docstrings and not in the __init__ .

Added icon and link as params to DuplicateButton and to its docstrings.

@vercel
Copy link

vercel bot commented Aug 7, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
gradio ✅ Ready (Inspect) Visit Preview Aug 7, 2023 4:01pm

@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Aug 7, 2023

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
gradio patch
website patch
  • Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Add icon and link to DuplicateButton

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

  • Maintainers can change the version label to modify the version bump.
  • If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

@aliabd aliabd added v: patch A change that requires a patch release t: fix A change that implements a fix labels Aug 7, 2023
@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Aug 7, 2023

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-5111-all-demos


You can install the changes in this PR by running:

pip install https://gradio-builds.s3.amazonaws.com/8a61efade2ac88733e08ca86291906f6f58c72aa/gradio-3.39.0-py3-none-any.whl

@aliabd aliabd requested a review from hannahblair August 7, 2023 00:26
@aliabd aliabd self-assigned this Aug 7, 2023
@abidlabs
Copy link
Member

abidlabs commented Aug 7, 2023

Thanks for catching this @aliabd. I agree with having DuplicateButton having its own docstring, but could we also add the icon and link parameters to gr.UploadButton and gr.DuplicateButton?

I also think it would be nice if we made the Spaces duplicate logo (screenshotted below) the default icon for the gr.DuplicateButton though we can do that separately.

image

@hannahblair
Copy link
Collaborator

Ah, thank you for fixing that!

@aliabd aliabd changed the title Remove icon and link from DuplicateButton docstring Add icon and link to DuplicateButton Aug 7, 2023
@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Aug 7, 2023

🎉 Chromatic build completed!

There are 0 visual changes to review.
There are 0 failed tests to fix.

@aliabd
Copy link
Collaborator Author

aliabd commented Aug 7, 2023

@abidlabs I added icon/link as params to DuplicateButton but I didn't add them to UploadButton because I realized that actually doesn't inherit from Button

@abidlabs
Copy link
Member

abidlabs commented Aug 7, 2023

LGTM @aliabd!

@abidlabs
Copy link
Member

abidlabs commented Aug 7, 2023

I'll go ahead and merge this in since @aliabd is ooo for a couple hours and this may unblock #5092

@abidlabs abidlabs merged commit b84a35b into main Aug 7, 2023
@abidlabs abidlabs deleted the aliabd/fix-duplicate-button branch August 7, 2023 16:54
@pngwn pngwn mentioned this pull request Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: fix A change that implements a fix v: patch A change that requires a patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants