Skip to content

Scroll to output#1077

Merged
pngwn merged 13 commits into
mainfrom
scroll-to-output
Apr 26, 2022
Merged

Scroll to output#1077
pngwn merged 13 commits into
mainfrom
scroll-to-output

Conversation

@pngwn
Copy link
Copy Markdown
Member

@pngwn pngwn commented Apr 25, 2022

Implements a loader per component and scrolls the relevant output component into view when an API request has been triggered.

The behaviour has been implemented such that if there are multiple outputs for a given trigger then the component that is closest to the top of the page will be the target of the scroll-into-view functionality.

Additionally, the components have been refactored so that the Block wrapping is done in the APP rather than the component package. This allows for greater flexibility with app-specific styling and cleans up the components somewhat.

Not every component has a laoder as the standard laoder design doesn't work for smaller components.

In order to test, try the fake_gan, chatbot, and kitchen_sink demos.

Workbench probably looks quite messy atm. Will clean that up later.

Closes #1055.

@pngwn pngwn requested review from abidlabs and gary149 April 25, 2022 11:04
@pngwn pngwn mentioned this pull request Apr 25, 2022
31 tasks
@pngwn
Copy link
Copy Markdown
Member Author

pngwn commented Apr 25, 2022

@gary149 @abidlabs This can be reviewed, test failures are just due to some code changes which I'll fix shortly.

@abidlabs
Copy link
Copy Markdown
Member

Very cooool! Love the loading indicator and the scrolling.

As you mentioned, we don't have a loading indicator for the smaller components, such as Textbox. Are you planning on adding them @pngwn?

@abidlabs
Copy link
Copy Markdown
Member

cc @aliabid94 as well

@pngwn
Copy link
Copy Markdown
Member Author

pngwn commented Apr 25, 2022

@abidlabs Yes, but need to chat with @gary149 about those first. I think it is okay to merge this when we're happy and add the others in a follow up. I have some idea for some form inputs but need to think about 'empty' styles + laoders for others.

@abidlabs
Copy link
Copy Markdown
Member

Whoops my bad @pngwn I pushed some changes to this branch when I thought I was working on a different branch. It's just small changes to make sure the kitchen sink and 2 other demos work when run from any path.

If you'd like, I can revert and move to a new branch?

@abidlabs
Copy link
Copy Markdown
Member

abidlabs commented Apr 25, 2022

@abidlabs Yes, but need to chat with @gary149 about those first. I think it is okay to merge this when we're happy and add the others in a follow up. I have some idea for some form inputs but need to think about 'empty' styles + laoders for others.

Edit: If there's a strong reason to merge, that's okay, but I think would be better to wait until we have the loaders for all of the components, because it does worsen the experience significantly (even for us testing/using gradio internally) if we don't have loaders for some demos.

@pngwn
Copy link
Copy Markdown
Member Author

pngwn commented Apr 26, 2022

Main reason to merge is that there a bunch of changes (refactors) in this branch that will be useful for other features + potentially lots of conflicts with other work that would happen in the meantime. Might take a few days to finalise some of the loaders.

The chatbot loader in particular can't happen until the 'custom js' branch is in (and that will need some tweaks to work for that purpose).

It shouldn't be for long anyway, a day or two at most. But that might be long enough to cause problems for other branches. I can put in some default loaders, even if unfinished, for all components in the meantime. At least there will be some feedback. I'll take care of that in the morning.

@pngwn
Copy link
Copy Markdown
Member Author

pngwn commented Apr 26, 2022

And dw about the extra commit.

@pngwn
Copy link
Copy Markdown
Member Author

pngwn commented Apr 26, 2022

Added loaders for every component that can load anything. Going to merge this so I can proceed with other work. Feel free to test it out more thoroughly and let me know if anything is broken.

@pngwn pngwn merged commit 2b0898b into main Apr 26, 2022
@pngwn pngwn deleted the scroll-to-output branch April 26, 2022 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Block wrapper handling

2 participants