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

🐛 add missing useEffect dependency to dcc.Loading component #2956

Merged
merged 3 commits into from
Aug 22, 2024

Conversation

KoolADE85
Copy link
Contributor

@KoolADE85 KoolADE85 commented Aug 22, 2024

This PR adds a missing dependency to the useEffect in the dcc.Loading component. This fixes a bug where it is possible for the loading spinner to be left enabled, even though loading has finished.

Fixes https://github.com/plotly/notebook-to-app/issues/1674

Note: I haven't been able to write a integration test that covers this code change precisely. The bug does not normally make an appearance - it's only in rare React race conditions that it is possible to get the "infinite loading bars".

@gvwilson since this affects internal projects, I'd love to get your team's eyes on this so we could maybe do a patch release?

Contributor Checklist

  • I have run the tests locally and they passed. (refer to testing section in contributing)
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

optionals

  • I have added entry in the CHANGELOG.md
  • If this PR needs a follow-up in dash docs, community thread, I have mentioned the relevant URLS as follows
    • this GitHub #PR number updates the dash docs
    • here is the show and tell thread in Plotly Dash community

@gvwilson gvwilson added P1 needed for current cycle fix fixes something broken labels Aug 22, 2024
Copy link
Contributor

@T4rk1n T4rk1n left a comment

Choose a reason for hiding this comment

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

💃

Copy link
Member

@ndrezn ndrezn left a comment

Choose a reason for hiding this comment

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

💃

@KoolADE85 KoolADE85 merged commit 64ed7fc into dev Aug 22, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix fixes something broken P1 needed for current cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants