-
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
Multipage #10433
base: main
Are you sure you want to change the base?
Conversation
🪼 branch checks and previews
Install Gradio from this PR pip install https://gradio-pypi-previews.s3.amazonaws.com/0c10c409d744142f7463a583e97cb4755de016c5/gradio-5.13.1-py3-none-any.whl Install Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@0c10c409d744142f7463a583e97cb4755de016c5#subdirectory=client/python" Install Gradio JS Client from this PR npm install https://gradio-npm-previews.s3.amazonaws.com/0c10c409d744142f7463a583e97cb4755de016c5/gradio-client-1.10.0.tgz Use Lite from this PR <script type="module" src="https://gradio-lite-previews.s3.amazonaws.com/0c10c409d744142f7463a583e97cb4755de016c5/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.
|
I get this error when trying
|
Whoops fixed |
Thanks @aliabid94, the demo is working for me now in regular mode.
Btw without SSR, multipage does feel a bit slow on Spaces: https://huggingface.co/spaces/abidlabs/multipage
|
gradio/routes.py
Outdated
@@ -592,6 +609,7 @@ def api_info(request: fastapi.Request): | |||
@app.get("/config", dependencies=[Depends(login_check)]) | |||
def get_config(request: fastapi.Request): | |||
config = utils.safe_deepcopy(app.get_blocks().config) | |||
# del config["page"] |
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 we need to remove this comment?
I'm not sure I agree about moving the links to the right but I do like the background shading idea! I do think we need the nav links to stand out a bit more. I can also experiment a little bit myself. |
SSR mode (and dev mode) now work @abidlabs |
I agree! In terms of a visual heirarchy, if a gradio app has both pages and tabs, the page links in the navbar should be more prominent but right now, they are less prominent than the tabs |
|
||
## Multipage Apps | ||
|
||
Your Gradio app can support multiple pages with the `gr.Blocks().route()` method. |
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.
I would suggest creating a separate guide for multipage apps where we discuss multipages in more depth including the fact that you can't share components, event listeners, otherwise you should use tabs, etc. Its likely we'll expand on multipages as well with dynamic routes and so on so worth having its own section imo and also for seo purposes.
@@ -1741,3 +1762,18 @@ async def new_lifespan(app: FastAPI): | |||
|
|||
app.mount(path, gradio_app) | |||
return app | |||
|
|||
|
|||
EXISTING_ROUTES = [ |
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.
note we have similar logic here, would be good to consolidate:
if (
getattr(blocks, "node_process", None) is not None
and blocks.node_port is not None
and not path.startswith("/gradio_api")
and path not in ["/config", "/favicon.ico"]
and not path.startswith("/theme")
and not path.startswith("/svelte")
and not path.startswith("/static")
and not path.startswith("/login")
and not path.startswith("/logout")
and not path.startswith("/manifest.json")
and not path.startswith("/pwa_icon")
):
num = gr.Number() | ||
... | ||
""" | ||
if path: |
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.
I would probably do some basic validation to ensure that the path does not contain invalid characters such as "?"
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.
Also make sure that a user doesn't accidentally reuse the same route for multiple pages in their Gradio app
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.
""" | ||
if path: | ||
path = path.strip("/") | ||
if path in EXISTING_ROUTES: |
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.
Nit: EXISTING_ROUTES
sounds like routes that are already part of their Gradio app, INTERNAL_ROUTES
would probably be more suitable
@aliabid94 this is working pretty well for me! Just a few minor points: (1) We should probably handle a large number of long page titles better (the navbar needs a bigger redesign anyways as mentioned in the comments above): (2) Perhaps we should add a parameter in Blocks.route() to allow people to exclude a page from the Navbar. Also, is there a way to rename the Home page? (3) I feel like there are small indentation changes that might trip up users as they don't give informative errors. For example, this fails and the resulting Gradio app just hangs: import gradio as gr
with gr.Blocks() as demo:
name = gr.Textbox(label="Name")
with demo.route("Test", "/test"):
num = gr.Number()
with demo.route("Test", "test2"):
num = gr.Number()
demo.launch() |
Co-authored-by: Abubakar Abid <abubakar@huggingface.co>
Co-authored-by: Abubakar Abid <abubakar@huggingface.co>
made the navbar a little more prominent, though feel free to suggest something else entirely @hannahblair |
Closes: #2654
Implemented multipage. This is the syntax:
See
demo/multipage/run.py
below:Initially I had implemented separate routes as separate Blocks but then it got real messy with trying to unify the queue, Stateholder and other backend things across multiple Blocks, so now we just keep the configs separate for different pages but the Blocks is shared.
Did not yet implement shared state or components across pages, because it's still unclear what the intended behaviour should be, still need to discuss.