streamlit

streamlit copied to clipboard

Describe your changes

Implements user-callable "skeleton" blocks (behave as empties)

GitHub Issue Link (if applicable)

• Closes https://github.com/streamlit/streamlit/issues/8032

Testing Plan

• JS App Test Updated
• Python AppTest pending

---

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

I think I would like to make this switch between the "Skeleton" and "AppSkeleton" react components depending on the height specified, but I am not sure of the best place to put that conditional code -- it would have to be in ElementNodeRenderer but that seems to break the single-responsiblity principle. Would it be better to implement a "SkeletonElement" component which selectively renders either an "AppSkeleton" or a "Skeleton" component based on height?

Thanks for the contribution @Asaurus1! I should have the time to give this a proper review sometime in the next few days.

I'm in favor of accepting this change as currently implemented in the PR (I'm less certain about the fancier callback behavior described in #8032, but I have no reservations around the behavior that's essentially just st.empty with nicer animations). CCing @jrieke and @sfc-gh-jcarroll for additional opinions from the product side in case they want to chime in.

@vdonato You're welcome! I certainly don't think the callback stuff is necessary, especially with the new st.partial stuff that is in the works.

I would like to do some experiments with small and large heights for the skeleton, as well as make sure it works graphically inside things like expanders. And if the UI folks have any input on looks that's welcome as well.

EDIT: I got a chance to perform these tests and everything looks clean as far as I can see. Made a cleanup to the docstring for the python function and I think it's good to go pending any additional feedback.

@sfc-gh-dmatthews or @sfc-gh-jcarroll, I noticed that the Skeleton boxes and the text elements have different radii of rounded corners. I know the overall streamlit theming changed in a recent release (like 1.26 or something), I'm wondering if this was overlooked or intentional. Would be good to have one of your UI folks take a look, maybe.

Hey @Asaurus1, this is a fantastic contribution! The original PR adding the app skeleton did leave open the question of a future Python API (and noted the "funky" nature of not currently having one). So, we're glad you've taken the initiative to write a well-thought-out spec with various API options and submitted this PR with the basic height parameter.

There's value in exposing the app skeleton to developers for the reasons you've laid out in the proposal. How exactly we do that also requires alignment on the product side. We're going to look into it this month and get back to you shortly 😄

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@snehankekre any updates on the product side of things? Would like for this branch to not get too stale if you're still thinking of merging it.

Hey @Asaurus1, sorry for taking a while to get back to you!

The product team is still split on whether to include this feature as-is.

It's blocked internally for the following:

Inserting a placeholder (i.e. an st.skeleton) for an element that actually occupies space while said element isn’t ready to draw is a good problem to solve, but we aren't all sure that the proposed solution is a clear win.

Issues with the current solution:

• The skeleton reappears on every rerun. This can be jarring in its own right! So in order to fix the “first load” problem we’re making every rerun after that worse.
  • Maybe the skeleton shouldn’t reappear on rerun? Or maybe it should remember the size of the thing it replaced?
• Users need to know the exact size of the skeleton ahead of time. In most cases, we don’t have that information, though. So the user will have to eyeball it.
  • …and even then, the eyeballed height will often not be right. Because text will wrap at different screen sizes, and dataframes will grow/shrink based on the data it contains.
  • The main time when the developer does know the height of the element that will appear next is when displaying images or videos. So maybe this should be an image/video-only feature?

As a middle ground, we want to add the height to our internal skeleton implementation (by removing the user-facing parts of this PR) and exposing this functionality via streamlit-extras. I think this is a useful feature and that developers who really want to use skeletons should be able to via extras, until we figure out the right API / solution.

@snehankekre all great points!

I'm happy to rebase this to just the JS side of things and keep the python stuff around in a separate branch to be sprinkled in again later if we desire.

Regarding the first "pop-in" concern, I think that can be fairly easily mitigated with a DelayedSkeleton component, which simply draws itself as an empty in the application for the first 0.5 seconds (similar to how the app skeleton works now) and only displays if that time has elapsed and it hasn't already been replaced.

The second concern about height and when this should be available is definitely more difficult to solve and deserves additional consideration. I can imagine some kind of fancy front end magic that detects when a skeleton is getting replaced and, computes the height of the new content, and then does some kind of CSS transition of the skeleton to the new content height, but that is definitely beyond the scope of this pull request, if it's even technically possible.

Sounds like a great solution to put in the JS hooks and expose the API in streamlit-extras for now. Thanks @Asaurus1 for building this and working with us to drive it through!

I'm happy to rebase this to just the JS side of things and keep the python stuff around in a separate branch to be sprinkled in again later if we desire.

Sounds good 👍 So we need the JS stuff + the height proto property in the skeleton message.

Regarding the first "pop-in" concern, I think that can be fairly easily mitigated with a DelayedSkeleton component, which simply draws itself as an empty in the application for the first 0.5 seconds (similar to how the app skeleton works now) and only displays if that time has elapsed and it hasn't already been replaced.

I think this would be a good idea. But I think it doesn't resolve one important problem we have with our placeholder e.g. st.empty. For example:

import streamlit as st
import time

placeholder = st.empty()
time.sleep(5)
placeholder.dataframe(["A", "B", "C"])

The dataframe in this example will disappear on every rerun for 5 seconds. While in this example it will just go stale (grey out):

import streamlit as st
import time

time.sleep(5)
st.dataframe(["A", "B", "C"])

This is even more problematic if you set the sleep to 1 second in both examples. The one without a placeholder will not show any visible change happening to the user while the placeholder usage will have it disappear. There are also other issues connected to this, e.g. the component might lose state. The reason why this is happening is the way of how we process the delta tree in the frontend and how we invalidate existing frontend components. We probably should start with finding a good solution for st.empty, which does not cause this undesired effect. Which would make a delayed skeleton a very good solution.

Ok I think everything is passing on https://github.com/Asaurus1/streamlit/pull/3 now so it should pass here.

@snehankekre any blockers to merging this? If we're still waiting on product team for the js-only changes that's alright.

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.