Skip to content

Commit 248d29e

Browse files
agg23Skn0tt
andauthored
chore(trace-viewer): improve progress indicator when loading trace (microsoft#36678)
Co-authored-by: Simon Knott <[email protected]>
1 parent 1592353 commit 248d29e

File tree

5 files changed

+101
-17
lines changed

5 files changed

+101
-17
lines changed

packages/trace-viewer/src/ui/shared/dialog.tsx

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ export interface DialogProps {
2020
className?: string;
2121
style?: React.CSSProperties;
2222
open: boolean;
23-
width: number;
23+
isModal?: boolean;
24+
width?: number;
2425
verticalOffset?: number;
2526
requestClose?: () => void;
2627
anchor?: React.RefObject<HTMLElement>;
@@ -31,6 +32,7 @@ export const Dialog: React.FC<React.PropsWithChildren<DialogProps>> = ({
3132
className,
3233
style: externalStyle,
3334
open,
35+
isModal,
3436
width,
3537
verticalOffset,
3638
requestClose,
@@ -52,7 +54,7 @@ export const Dialog: React.FC<React.PropsWithChildren<DialogProps>> = ({
5254
position: 'fixed',
5355
margin: 0,
5456
top: bounds.bottom + (verticalOffset ?? 0),
55-
left: buildTopLeftCoord(bounds, width),
57+
left: buildTopLeftCoord(bounds, width ?? 0),
5658
width,
5759
zIndex: 1,
5860
...externalStyle
@@ -96,12 +98,24 @@ export const Dialog: React.FC<React.PropsWithChildren<DialogProps>> = ({
9698
};
9799
}, []);
98100

101+
React.useLayoutEffect(() => {
102+
if (!dialogRef.current)
103+
return;
104+
105+
if (open) {
106+
if (isModal)
107+
dialogRef.current.showModal();
108+
else
109+
dialogRef.current.show();
110+
} else {
111+
dialogRef.current.close();
112+
}
113+
}, [open, isModal]);
114+
99115
return (
100-
open && (
101-
<dialog ref={dialogRef} style={style} className={className} data-testid={dataTestId} open>
102-
{children}
103-
</dialog>
104-
)
116+
<dialog ref={dialogRef} style={style} className={className} data-testid={dataTestId}>
117+
{children}
118+
</dialog>
105119
);
106120
};
107121

packages/trace-viewer/src/ui/workbenchLoader.css

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,17 +65,40 @@ body.dark-mode .drop-target {
6565
cursor: pointer;
6666
}
6767

68-
.progress {
69-
flex: none;
68+
.progress-dialog {
69+
width: 400px;
70+
inset: 0;
71+
border: none;
72+
outline: none;
73+
background-color: var(--vscode-sideBar-background);
74+
}
75+
76+
.progress-dialog::backdrop {
77+
background-color: rgba(0, 0, 0, 0.4);
78+
}
79+
80+
.progress-content {
81+
padding: 16px;
82+
}
83+
84+
.progress-content .title {
85+
/* This is set in common.css */
86+
background-color: unset;
87+
font-size: 18px;
88+
font-weight: bold;
89+
padding: 0;
90+
}
91+
92+
.progress-wrapper {
93+
background-color: var(--vscode-commandCenter-activeBackground);
7094
width: 100%;
71-
height: 3px;
72-
margin-top: -3px;
73-
z-index: 10;
95+
margin-top: 16px;
96+
margin-bottom: 8px;
7497
}
7598

7699
.inner-progress {
77100
background-color: var(--vscode-progressBar-background);
78-
height: 100%;
101+
height: 4px;
79102
}
80103

81104
.header {

packages/trace-viewer/src/ui/workbenchLoader.tsx

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import './workbenchLoader.css';
2121
import { Workbench } from './workbench';
2222
import { TestServerConnection, WebSocketTestServerTransport } from '@testIsomorphic/testServerConnection';
2323
import { SettingsToolbarButton } from './settingsToolbarButton';
24+
import { Dialog } from './shared/dialog';
2425

2526
export const WorkbenchLoader: React.FunctionComponent<{
2627
}> = () => {
@@ -32,6 +33,7 @@ export const WorkbenchLoader: React.FunctionComponent<{
3233
const [dragOver, setDragOver] = React.useState<boolean>(false);
3334
const [processingErrorMessage, setProcessingErrorMessage] = React.useState<string | null>(null);
3435
const [fileForLocalModeError, setFileForLocalModeError] = React.useState<string | null>(null);
36+
const [showProgressDialog, setShowProgressDialog] = React.useState<boolean>(false);
3537

3638
const processTraceFiles = React.useCallback((files: FileList) => {
3739
const blobUrls = [];
@@ -167,6 +169,20 @@ export const WorkbenchLoader: React.FunctionComponent<{
167169
})();
168170
}, [isServer, traceURLs, uploadedTraceNames]);
169171

172+
const showLoading = progress.done !== progress.total && progress.total !== 0;
173+
174+
React.useEffect(() => {
175+
if (showLoading) {
176+
const timeout = setTimeout(() => {
177+
setShowProgressDialog(true);
178+
}, 200);
179+
180+
return () => clearTimeout(timeout);
181+
} else {
182+
setShowProgressDialog(false);
183+
}
184+
}, [showLoading]);
185+
170186
const showFileUploadDropArea = !!(!isServer && !dragOver && !fileForLocalModeError && (!traceURLs.length || processingErrorMessage));
171187

172188
return <div className='vbox workbench-loader' onDragOver={event => { event.preventDefault(); setDragOver(true); }}>
@@ -179,9 +195,6 @@ export const WorkbenchLoader: React.FunctionComponent<{
179195
<div className='spacer'></div>
180196
<SettingsToolbarButton />
181197
</div>
182-
<div className='progress'>
183-
<div className='inner-progress' style={{ width: progress.total ? (100 * progress.done / progress.total) + '%' : 0 }}></div>
184-
</div>
185198
<Workbench model={model} inert={showFileUploadDropArea} />
186199
{fileForLocalModeError && <div className='drop-target'>
187200
<div>Trace Viewer uses Service Workers to show traces. To view trace:</div>
@@ -191,6 +204,14 @@ export const WorkbenchLoader: React.FunctionComponent<{
191204
<div>3. Drop the trace from the download shelf into the page</div>
192205
</div>
193206
</div>}
207+
<Dialog open={showProgressDialog} isModal={true} className='progress-dialog'>
208+
<div className='progress-content'>
209+
<div className='title' role='heading' aria-level={1}>Loading Playwright Trace...</div>
210+
<div className='progress-wrapper'>
211+
<div className='inner-progress' style={{ width: progress.total ? (100 * progress.done / progress.total) + '%' : 0 }}></div>
212+
</div>
213+
</div>
214+
</Dialog>
194215
{showFileUploadDropArea && <div className='drop-target'>
195216
<div className='processing-error' role='alert'>{processingErrorMessage}</div>
196217
<div className='title' role='heading' aria-level={1}>Drop Playwright Trace to load</div>

tests/library/trace-viewer.spec.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import type { TraceViewerFixtures } from '../config/traceViewerFixtures';
2121
import { traceViewerFixtures } from '../config/traceViewerFixtures';
2222
import fs from 'fs';
2323
import path from 'path';
24+
import type http from 'http';
2425
import { pathToFileURL } from 'url';
2526
import { expect, playwrightTest } from '../config/browserTest';
2627
import type { FrameLocator } from '@playwright/test';
@@ -1914,3 +1915,27 @@ test('should render locator descriptions', async ({ runAndTrace, page }) => {
19141915
- treeitem /Click.*input.*first/
19151916
`);
19161917
});
1918+
1919+
test('should load trace from HTTP with progress indicator', async ({ showTraceViewer, server }) => {
1920+
const [traceViewer, res] = await Promise.all([
1921+
showTraceViewer([server.PREFIX]),
1922+
new Promise<http.ServerResponse>(resolve => {
1923+
server.setRoute('/', (req, res) => resolve(res));
1924+
}),
1925+
]);
1926+
1927+
const file = await fs.promises.readFile(traceFile);
1928+
1929+
const dialog = traceViewer.page.locator('dialog', { hasText: 'Loading' });
1930+
1931+
res.setHeader('Access-Control-Allow-Origin', '*');
1932+
res.setHeader('Content-Length', file.byteLength);
1933+
res.writeHead(200);
1934+
await expect(dialog).not.toBeVisible({ timeout: 100 });
1935+
// Should become visible after ~200ms
1936+
await expect(dialog).toBeVisible();
1937+
1938+
res.end(file);
1939+
await expect(dialog).not.toBeVisible();
1940+
await expect(traceViewer.actionTitles).toContainText([/Create page/]);
1941+
});

tests/playwright-test/reporter-html.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,8 @@ for (const useIntermediateMergeReport of [true, false] as const) {
620620
await showReport();
621621
await page.getByRole('link', { name: 'passes' }).click();
622622
await page.click('img');
623-
await expect(page.locator('.workbench-loader .title')).toHaveText('a.test.js:3 › passes');
623+
await expect(page.locator('.progress-dialog')).toBeHidden();
624+
await expect(page.locator('.workbench-loader > .header > .title')).toHaveText('a.test.js:3 › passes');
624625
});
625626

626627
test('should show multi trace source', async ({ runInlineTest, page, server, showReport }) => {

0 commit comments

Comments
 (0)