Skip to content

Restructure guides #1857

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

Merged
merged 12 commits into from
Jul 28, 2022
Merged

Restructure guides #1857

merged 12 commits into from
Jul 28, 2022

Conversation

aliabid94
Copy link
Collaborator

@aliabid94 aliabid94 commented Jul 22, 2022

Guides are reorganized to be in categories and have an order. Sidebar is restored to show subheadings of current guide, as well as all other guides.

Fixes #1760 , Fixes #1757, Fixes #1739, Fixes #1720, Fixes #1755

Edit: also Fixes: #1750

@aliabid94 aliabid94 marked this pull request as draft July 22, 2022 12:07
@abidlabs
Copy link
Member

This is great @aliabid94!

Some initial reactions:

  • I love the sidebar of the Guides. Makes them much more cohesive. I don't like this scrollbar though -- in general two scrollbars on one page are overwhelming:

image

  • Quickstart is working for me, but many of the other Guides are not, e.g.:

image

  • Some guides (like Flagging) are applicable to both Interfaces and Blocks, so we'll have to figure that out...

@aliabid94
Copy link
Collaborator Author

There need to be two scrollbars. The navigation scroll bar automatically scrolls to where you are in the curriculum. It's not possible to keep the navigation pane in sync with the guide if there aren't two scrollbars. See https://svelte.dev/docs for another example of two scrollbars.

I've restructured the main guides and rewritten significant chunks of content, please re-review.

@aliabid94 aliabid94 marked this pull request as ready for review July 25, 2022 07:52
@aliabid94 aliabid94 requested review from pngwn, abidlabs, freddyaboulton, aliabd and dawoodkhan82 and removed request for pngwn and abidlabs July 25, 2022 07:52
Copy link
Collaborator

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

@aliabid94 This is awesome! These are my first thoughts on this, wanted to give you feedback ahead of our synch in a couple of minutes.

I love the new guides landing page as well as the different tracks and the concept of the curriculum.

Thoughts on the tracks:

In my opinion, "Using Blocks as Functions" should be in the Blocks track.

Add a "Developer Experience" track that will include "Developing Faster with Reload Mode", "Embedding Gradio Demos", as well as the new "Adding A New Component" guide that will be merged soon.

Add a "Integrating with Other frameworks" track that will include "Gradio and ONNX on HuggingFace", "Using HuggingFace Integrations", as well as the forthcoming Weights And Biases integration guide.

I think having a separate track on guides will be helpful as we add more integrations in the future, like Ray.

The remaining guides can be recategorized into "Tutorials".

What to do with content that applies to both Interfaces and Blocks?

There's content currently in the guides about Interpretation and Examples as they pertain to blocks apps that's not currently in this PR.

I think this content should still be in the guides page - last friday we saw a case where someone tried to add examples via gr.Dataset instead of gr.Examples and it lead to really slow inference times.

It's not clear where in the current structure they will go because Interpretation and Examples are in the Interface section right now.

Some missing content I noticed:

We should have a place for this I think. Flexible on where. There may be more.

  1. "Dataframes and Graphs" section of quick start
  2. Multi-step demos of Introduction to blocks

Nits

Make sure the docs are still linked to guides after re-naming the guides (e.g. Series and Parallel are not linked to guides anymore)

@@ -0,0 +1,31 @@
# Custom JS and CSS
Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome, I think this closes #1720

@abidlabs
Copy link
Member

@aliabid94 this honestly looks fantastic! I'm going through the guides and will add feedback below!

@abidlabs
Copy link
Member

Feedback (mostly nits since the Guides look great!)


Stylistic feedback

  1. I've said this before (and maybe my vision is just failing me...), but I find the font size for the main prose text in Guides too small. It's smaller than the font size in the code blocks, smaller than the font used on the docs page, and smaller than font size used on other websites). Too small:

image

  1. The font size for inline code is even smaller! It should be the same size as surrounding text:

image

Content feedback

  1. The "Styling" section should mention the css parameter for Interface and Blocks, since we get lots of questions about that. You do mention it later on, but I think it deserves an earlier mention.
  2. This explanation for queuing seems to undersell queuing: "This will queue up calls so only a single call is processed at a time." Shouldn't you add something like it uses "long polling so that requests don't time out after 1 min"
  3. When you say "See the link to the "API" in the app above", it's not clear to me what you're referring to. Can we include another screenshot or something to make it clear?

@freddyaboulton freddyaboulton mentioned this pull request Jul 26, 2022
1 task
@aliabid94
Copy link
Collaborator Author

I believe things are ready to be merged. Responding to all the message above:

  • for now, left "Using Blocks as Functions" where it is bc it also applies to Interfaces. Need to find a the right place for features common to Interface and Blocks.
  • added "Integrating other frameworks" and "other tutorials"
  • I had removed data components section in the getting started intentionally. I don't think it's necessary to explain this directly, there are examples in the guide later that use dataframes.
  • Multi step demos was restored (by abidlabs I think)
  • Restored docs link to Series and Parallel (I also think we should rethink this guides-to-docs link generally)
  • Made guides text larger
  • Added css= to styling section, and expanded queue explanation.
  • demo above "view api" section wasn't working, fixed

Let's get this merged so that other guides can be added with this structure.

@aliabid94 aliabid94 requested a review from freddyaboulton July 27, 2022 06:42
Copy link
Collaborator

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

@aliabid94 Thanks for this awesome PR! I think it's a definite improvement over our previous structure and I like the new "Key Features" app.

Agree that we should get this merged to unblock future guide development.

Let's file a brainstorming issue to track all the nits still left in this PR:

  • Copy-paste button in code snippets in guides is gone
  • Is it possible to display a sidebar in the guide landing page like we do in the guides? I think it may help see all the tracks/guides as they all don't fit on one page (at least on my latpop)

image

  • Figure out new homes for blocks/guides concept.

@abidlabs abidlabs mentioned this pull request Jul 28, 2022
3 tasks
@abidlabs
Copy link
Member

Created #1903 to track the remaining items, but this looks good to merge @aliabid94!

@aliabd aliabd merged commit 35d809c into main Jul 28, 2022
@aliabd aliabd deleted the guides_restructure branch July 28, 2022 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants