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

Increase time until timeout warning is shown for a custom component #8179

Merged
merged 4 commits into from Feb 21, 2024

Conversation

raethlein
Copy link
Collaborator

@raethlein raethlein commented Feb 19, 2024

Close #7046

While the custom component is loaded, we show the loading skeleton. Following changes are made in this PR:

  1. the warning component will be shown after 60s instead of 3s to make it less disruptive in case of slower loading times
  2. after 15s, a console warning is shown in the browser's DevTools as a soft-warning for the developer
  3. the warning texts are modified slightly and are now different based on whether a url is provided (usually dev mode) or not
  4. the existing class component is re-written as a functional component. Since I had to touch the component anyways, I thought it's a good idea to refactor it using the newer React style and make the changes then
  5. Add a couple of more tests for some convenient methods that now live in its own file componentUtils.ts
  6. Add a couple of more types. As a future note: most types are also defined in the component-lib/ module and we could think about creating a shared module so that we do not duplicate the typing etc. However, I consider this to be out of scope for this PR.
  7. The loading skeleton will mimic the same height if we infer from the custom component provides a height value.
Screenshot of warning messages Screenshot 2024-02-16 at 14 50 49
Screenshot of loading skeletons Screenshot 2024-02-19 at 18 43 23

Describe your changes

GitHub Issue Link (if applicable)

Testing Plan

  • Explanation of why no additional tests are needed
  • Unit Tests (JS and/or Python):
    • reuse the existing tests before refactoring
    • add some more tests for convenient methods put into it's own componentUtil.ts file
    • add test to Skeleton.test.ts to check for passed height property
  • E2E Tests
  • Any manual testing needed?

Contribution License Agreement

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

}

const RawSkeleton: FC<React.PropsWithChildren<Props>> = props => {
return <SquareSkeleton data-testid="stSkeleton" {...props} />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change allows us to pass a height to the loading skeleton

@raethlein raethlein force-pushed the 7046-custom-component-loading-indicator branch 2 times, most recently from 366ab45 to 485b4bf Compare February 19, 2024 17:55
@raethlein raethlein marked this pull request as ready for review February 19, 2024 22:31
@naterush
Copy link

@raethlein just opened an issue that I think deals with this PR here: #8285

Is it possible that this PR changed how heights are calculated for custom components inconsistently?

zyxue pushed a commit to zyxue/streamlit that referenced this pull request Apr 16, 2024
…shown (streamlit#8179)

- Rewrites ComponentInstance from React class component to React functional component
- Warning element when component cannot be loaded is shown after 60 seconds instead of 3 seconds
- A warning log is issued after 15 seconds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants