Skip to content

Commit 35b3bf2

Browse files
authored
fix: Loading spinner would flash sometimes on startup (#2569)
- 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
1 parent e1c2dc4 commit 35b3bf2

4 files changed

Lines changed: 42 additions & 2 deletions

File tree

packages/app-utils/src/components/AppBootstrap.test.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ it('should log in automatically when the anonymous handler is supported', async
165165
expectMockChild().toBeNull();
166166
expect(mockLogin).toHaveBeenCalled();
167167
expect(screen.queryByTestId('auth-base-loading')).not.toBeNull();
168+
expect(screen.queryByTestId('auth-base-loading-spinner')).not.toBeNull();
168169

169170
// Wait for login to complete
170171
await act(async () => {
@@ -173,6 +174,9 @@ it('should log in automatically when the anonymous handler is supported', async
173174

174175
expect(screen.queryByTestId('auth-base-loading')).toBeNull();
175176
expect(screen.queryByTestId('connection-bootstrap-loading')).not.toBeNull();
177+
expect(
178+
screen.queryByTestId('connection-bootstrap-loading-spinner')
179+
).not.toBeNull();
176180
expect(screen.queryByText(mockChildText)).toBeNull();
177181
expect(mockChannel.postMessage).toHaveBeenCalledWith(
178182
expect.objectContaining({

packages/app-utils/src/components/ConnectionBootstrap.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ export function ConnectionBootstrap({
206206
return (
207207
<LoadingOverlay
208208
data-testid="connection-bootstrap-loading"
209-
isLoading={false}
209+
isLoading={error == null}
210210
errorMessage={error != null ? `${error}` : undefined}
211211
/>
212212
);

packages/code-studio/src/main/App.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import AppMainContainer from './AppMainContainer';
44

55
function App(): ReactElement {
66
return (
7-
<div className="app">
7+
<div className="app" data-testid="app-loaded">
88
<AppMainContainer />
99
<ToastContainer />
1010
</div>

tests/app.spec.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { test, expect } from '@playwright/test';
2+
import { gotoPage } from './utils';
3+
4+
test.describe('app loading tests', () => {
5+
test.beforeEach(async ({ page }) => {
6+
await gotoPage(page, '');
7+
});
8+
9+
test('initial dashboard is immediately ready after `gotoPage` has completed', async ({
10+
page,
11+
}) => {
12+
await page.goto('');
13+
14+
// Now poll every 5ms to see if the app has finished loading. Until it has finished loading, a loading spinner should be visible.
15+
await expect
16+
.poll(
17+
async () => {
18+
const isSpinnerVisible = await page
19+
.getByRole('progressbar', { name: 'Loading...', exact: true })
20+
.isVisible();
21+
const isAppLoaded =
22+
(await page.getByTestId('app-loaded').count()) === 1;
23+
if (!isSpinnerVisible && !isAppLoaded) {
24+
throw new Error(
25+
'App is in an invalid state: no spinner and app not loaded'
26+
);
27+
}
28+
return isAppLoaded;
29+
},
30+
{
31+
intervals: [5],
32+
}
33+
)
34+
.toBe(true);
35+
});
36+
});

0 commit comments

Comments
 (0)