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

Improve styling of parameter tables in the docs #8618

Merged
merged 30 commits into from
Jul 16, 2024
Merged

Improve styling of parameter tables in the docs #8618

merged 30 commits into from
Jul 16, 2024

Conversation

abidlabs
Copy link
Member

@abidlabs abidlabs commented Jun 25, 2024

Change the styling of the parameter tables in the docs to be consistent with the custom component docs, which is a lot more compact than our current docs. Because we are using the new ParamViewer component, the sections expand when a user is searching for a term that appears in a docstring:

Screen.Recording.2024-07-15.at.2.37.36.PM.mov

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Jun 25, 2024

🪼 branch checks and previews

Name Status URL
Spaces ready! Spaces preview
Website ready! Website preview
Storybook ready! Storybook preview
🦄 Changes detected! Details

Install Gradio from this PR

pip install https://gradio-builds.s3.amazonaws.com/afa11926c914ea76448dc8b9937278104be3a203/gradio-4.38.1-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@afa11926c914ea76448dc8b9937278104be3a203#subdirectory=client/python"

Install Gradio JS Client from this PR

npm install https://gradio-builds.s3.amazonaws.com/afa11926c914ea76448dc8b9937278104be3a203/gradio-client-1.3.0.tgz

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Jun 25, 2024

🦄 change detected

This Pull Request includes changes to the following packages.

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

With the following changelog entry.

Improve styling of parameter tables in the docs

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.

@abidlabs abidlabs changed the title param table Style parameter tables in the docs Jun 25, 2024
@abidlabs abidlabs added the v: patch A change that requires a patch release label Jun 25, 2024
@pngwn
Copy link
Member

pngwn commented Jun 25, 2024

I don't really love this for long types, they are unreadable of which we have many: https://gradio-9e0pzuzft-hugging-face.vercel.app/docs/gradio/annotatedimage

I think the ParamViewer offers a better compromise: https://huggingface.co/spaces/pngwn/gradio_imageslider

@abidlabs
Copy link
Member Author

@pngwn I can split the types across multiple lines when its a union of types to avoid the issue you're mentioning

Stylistically, I think gr.ParamViewer would look out of place on our docs page. And one thing I don't like like about it is how the docstrings are hidden by default -- they should always be visible to make it easier to cmd+f and find what you're looking for

@abidlabs
Copy link
Member Author

Actually I just made it wrap to the next line and I think this looks pretty good:

image image

wdyt @pngwn @aliabd?

@aliabd
Copy link
Collaborator

aliabd commented Jun 26, 2024

@abidlabs this looks great, though i wish there was better styling for the Default: / Required part. Maybe keep that as it is currently?

Screen Shot 2024-06-25 at 8 18 10 PM

@abidlabs abidlabs marked this pull request as ready for review June 26, 2024 17:11
@abidlabs abidlabs changed the title Style parameter tables in the docs Improve styling of parameter tables in the docs Jun 26, 2024
@freddyaboulton
Copy link
Collaborator

The proposed styling looks nice! I do think it would be best if we could consolidate the desired "param table style" in the ParamViewer component. That way all parameter tables (custom components, docs, api docs) would be consistent.

Copy link
Collaborator

@aliabd aliabd left a comment

Choose a reason for hiding this comment

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

Changes look good to me @abidlabs. I agree with @freddyaboulton that it would be nice to be consistent everywhere.

You have to make the changes to FunctionDoc.svelte as well (like you did for EventListeners.svelte). Right now it's still the old table styling.

Screen Shot 2024-06-27 at 12 02 41 PM

@pngwn
Copy link
Member

pngwn commented Jun 28, 2024

I think don't think the compound/ complex types are really that readable. You can read them now but it is still almost impossible to understand the relationship between the different parts (ie nested bits, unions, etc) without some proper formatting.

@pngwn
Copy link
Member

pngwn commented Jun 28, 2024

I personally think the docstrings are too long to be visible all of the time, we have too many kwargs for that. If you want to see anything else then you have an awful experience.

@abidlabs
Copy link
Member Author

I think don't think the compound/ complex types are really that readable. You can read them now but it is still almost impossible to understand the relationship between the different parts (ie nested bits, unions, etc) without some proper formatting.

Ok with changing this, but how would you suggest formatting them @pngwn?

I personally think the docstrings are too long to be visible all of the time, we have too many kwargs for that. If you want to see anything else then you have an awful experience.

I don't think that's the case, you can easily navigate to the different sections using the sidebar on the right. Having all of the docstrings makes it much easier to CMD+F and find what you're looking for. I strongly dislike when documentation is partially hidden as it makes search completely unreliable

You have to make the changes to FunctionDoc.svelte as well (like you did for EventListeners.svelte). Right now it's still the old table styling.

Can make this change!

@pngwn
Copy link
Member

pngwn commented Jul 8, 2024

Ok with changing this, but how would you suggest formatting them @pngwn?

For the custom component docs I used ruff: https://github.com/gradio-app/gradio/blob/main/gradio/cli/commands/components/_docs_utils.py#L18-L47 we could possible reuse/ modify this function.

@pngwn
Copy link
Member

pngwn commented Jul 8, 2024

I don't think that's the case, you can easily navigate to the different sections using the sidebar on the right. Having all of the docstrings makes it much easier to CMD+F and find what you're looking for. I strongly dislike when documentation is partially hidden as it makes search completely unreliable

You are only thinking of your own usage here though. ctrl + f is only useful for power users or people who know exactly what they are looking for. It is useless for new users. Scan-ability is very important in those cases and excessively long document and minute detail make that difficult. We have search now which should support this usecase and we could visually hide the content and expand it when it gets focused.

@abidlabs
Copy link
Member Author

abidlabs commented Jul 8, 2024

You are only thinking of your own usage here though. ctrl + f is only useful for power users or people who know exactly what they are looking for. It is useless for new users.

I actually think its the opposite! For example, we recently had a question on Discord about how to change the directory where the Python Client saves files. If you are able to search "directory" in the Python Client docs, https://www.gradio.app/docs/python-client/client, then you'd easily be able to find the answer. However, if you have to read each parameter, its going to take you longer to find the right parameter. Even if Search supported navigating to the right section, that's global search, which is not as convenient as same-page search if you are already on the right page. I really think we should always keep parameter descriptions expanded so we can keep users can always default cmd+f.

@abidlabs
Copy link
Member Author

Should be ready for review! Let me know if you think I should many styling changes @aliabd @pngwn

Copy link
Collaborator

@aliabd aliabd left a comment

Choose a reason for hiding this comment

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

Nice! everything looks good and ctrl+f works nicely :)

@abidlabs
Copy link
Member Author

Thanks @pngwn and @aliabd!

@abidlabs abidlabs merged commit aa4b7a7 into main Jul 16, 2024
8 checks passed
@abidlabs abidlabs deleted the param-style branch July 16, 2024 22:53
@pngwn pngwn mentioned this pull request Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v: patch A change that requires a patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants