Skip to content

fix: Loading spinner would flash sometimes on startup#2569

Merged
mofojed merged 4 commits intodeephaven:mainfrom
mofojed:loading-spinner-startup-flash
Nov 11, 2025
Merged

fix: Loading spinner would flash sometimes on startup#2569
mofojed merged 4 commits intodeephaven:mainfrom
mofojed:loading-spinner-startup-flash

Conversation

@mofojed
Copy link
Copy Markdown
Member

@mofojed mofojed commented Nov 10, 2025

  • We were not displaying the loading spinner for a brief moment in ConnectionBootstrap
  • Added unit test to verify spinner was being shown (previously was not verifying the spinner itself was shown, just that the loading overlay was shown)
  • Tested using visual inspection
  • Added Playwright test to poll startup every 5ms to ensure the loading spinner is displayed until the app is loaded
  • Should fix an issue with deephaven-plugins e2e tests currently failing inconsistently

- There were a couple cases where we may not have been displaying a loading spinner while going through the app startup sequence
- Make sure the spinner is displayed consistently
- Tested using visual inspection
  - Testing using Playwright proved to be inconsistent
- That wasn't the part that needed fixing
@mofojed mofojed requested a review from bmingles November 10, 2025 19:17
@mofojed mofojed self-assigned this Nov 10, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.53%. Comparing base (e1c2dc4) to head (8ee4d7d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2569      +/-   ##
==========================================
- Coverage   44.54%   44.53%   -0.01%     
==========================================
  Files         768      768              
  Lines       43367    43367              
  Branches    10973    11154     +181     
==========================================
- Hits        19317    19313       -4     
+ Misses      24034    24007      -27     
- Partials       16       47      +31     
Flag Coverage Δ
unit 44.53% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Polls every 5ms to ensure the loading spinner is visible while the app is loading
mofojed added a commit to mofojed/deephaven-plugins that referenced this pull request Nov 10, 2025
- We would count the number of panels before we were waiting for the "Panels" button to be visible, which would sometimes be incorrect if the loading spinner had flashed/disappeared
- Just count the number of panels right before we click the "Panels" button
- deephaven/web-client-ui#2569 should also fix it from UI side, but this fixes the issue on the deephaven-plugins side
Copy link
Copy Markdown
Contributor

@bmingles bmingles left a comment

Choose a reason for hiding this comment

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

LGTM

@mofojed mofojed merged commit 35b3bf2 into deephaven:main Nov 11, 2025
12 checks passed
@mofojed mofojed deleted the loading-spinner-startup-flash branch November 11, 2025 18:02
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants