Skip to content

chore: tv playback teaser#39634

Open
pavelfeldman wants to merge 2 commits intomicrosoft:mainfrom
pavelfeldman:tv_pb
Open

chore: tv playback teaser#39634
pavelfeldman wants to merge 2 commits intomicrosoft:mainfrom
pavelfeldman:tv_pb

Conversation

@pavelfeldman
Copy link
Member

No description provided.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an initial “playback” (scrub/play/step) experience to Trace Viewer, wiring new UI controls into the workbench/timeline and updating tests accordingly.

Changes:

  • Introduces usePlayback, playback buttons, and a timeline scrubber UI (plus styling).
  • Integrates the scrubber into Timeline and adds playback controls to the snapshot toolbar.
  • Updates Trace Viewer tests: removes the old timeline network-bar assertion and adds a dedicated scrub/playback spec.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tests/library/trace-viewer.spec.ts Removes coverage that depended on timeline network bars.
tests/library/trace-viewer-scrub.spec.ts Adds end-to-end coverage for playback controls/scrubber interactions.
packages/web/src/components/treeView.tsx Adjusts selection scrolling behavior for programmatic navigation.
packages/trace-viewer/src/ui/workbenchLoader.tsx Renames header class for styling changes.
packages/trace-viewer/src/ui/workbenchLoader.css Restyles Trace Viewer loader header.
packages/trace-viewer/src/ui/workbench.tsx Wires playback state into timeline scrubber and snapshot toolbar.
packages/trace-viewer/src/ui/workbench.css Minor layout tweak for the action filter container.
packages/trace-viewer/src/ui/timeline.tsx Simplifies timeline rendering and accepts an injected scrubber node.
packages/trace-viewer/src/ui/timeline.css Layout adjustments to accommodate scrubber/window visuals.
packages/trace-viewer/src/ui/snapshotTab.tsx Adds playback buttons to the snapshot tab toolbar.
packages/trace-viewer/src/ui/playbackControl.tsx New playback state + UI (buttons/scrubber) implementation.
packages/trace-viewer/src/ui/playbackControl.css New styling for playback buttons/scrubber/ticks.
packages/trace-viewer/src/ui/filmStrip.tsx Minor markup ordering tweak for hover title rendering.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +245 to +255
const animating = !playing && !dragging;
const displayTime = dragging && dragFraction !== undefined ? fullMin + dragFraction * fullDuration : playing && cursorTime !== undefined ? cursorTime : selectedTime;
const elapsed = msToString(Math.max(0, displayTime - fullMin));
const total = msToString(fullMax - fullMin);
const ticks = actions.length > 0 && actions.length <= 200 ? actions.map(a => ((a.startTime - fullMin) / fullDuration) * 100) : undefined;

return {
playing, speed, currentIndex, percent, animating,
elapsed, total, togglePlay, stop, prev, next, cycleSpeed,
onScrubberMouseDown, scrubberRef, actionsLength: actions.length, ticks,
};
Comment on lines +17 to +28
.playback-time {
font-size: 11px;
white-space: nowrap;
user-select: none;
}

.playback-separator {
font-size: 11px;
margin: 0 2px;
user-select: none;
}

Comment on lines +246 to 247
if (selectedItem?.id === item.id && itemRef.current)
scrollIntoViewIfNeeded(itemRef.current);
Comment on lines +115 to +132
.workbench-loader-header {
display: flex;
background-color: #000;
background-color: var(--vscode-sideBar-background);
flex: none;
flex-basis: 48px;
line-height: 48px;
font-size: 16px;
color: #cccccc;
align-items: center;
box-shadow: var(--vscode-scrollbar-shadow) 0 6px 6px -6px;
}

.workbench-loader {
contain: size;
}

.workbench-loader .header .toolbar-button {
margin: 12px;
padding: 8px 4px;
.workbench-loader .workbench-loader-header {
height: 32px;
}
Comment on lines +262 to +266
<ToolbarButton icon='chevron-left' title='Previous action' onClick={playback.prev} disabled={playback.currentIndex <= 0} />
<ToolbarButton icon={playback.playing ? 'debug-pause' : 'play'} disabled={!playback.actionsLength} title={playback.playing ? 'Pause' : 'Play'} onClick={playback.togglePlay} />
<ToolbarButton icon='debug-stop' title='Stop' onClick={playback.stop} disabled={!playback.playing && playback.currentIndex <= 0} />
<ToolbarButton icon='chevron-right' title='Next action' onClick={playback.next} disabled={playback.currentIndex >= playback.actionsLength - 1} />
<button className='playback-speed' onClick={playback.cycleSpeed} title='Playback speed'>
Comment on lines +89 to +106
const actionIndexAtTime = React.useCallback((t: number): number => {
// Search within the full actions array but clamp to window bounds.
let lo = 0, hi = actions.length - 1;
while (lo < hi) {
const mid = (lo + hi + 1) >> 1;
if (actions[mid].startTime <= t)
lo = mid;
else
hi = mid - 1;
}
if (lo < actions.length - 1) {
const distPrev = t - actions[lo].startTime;
const distNext = actions[lo + 1].startTime - t;
if (distNext < distPrev)
return lo + 1;
}
return lo;
}, [actions]);
Comment on lines +216 to +243
const onScrubberMouseDown = React.useCallback((e: React.MouseEvent) => {
if (!actions.length || e.button !== 0)
return;
e.preventDefault();
e.stopPropagation();
scrubberRef.current?.focus();
setDragging(true);
setPlaying(false);
const fraction = fractionFromMouseEvent(e);
setDragFraction(fraction);
selectActionAtFraction(fraction);

const onMouseMove = (me: MouseEvent) => {
const f = fractionFromMouseEvent(me);
setDragFraction(f);
selectActionAtFraction(f);
};
const onMouseUp = (me: MouseEvent) => {
document.removeEventListener('mousemove', onMouseMove);
document.removeEventListener('mouseup', onMouseUp);
const f = fractionFromMouseEvent(me);
selectActionAtFraction(f);
setDragFraction(undefined);
setDragging(false);
};
document.addEventListener('mousemove', onMouseMove);
document.addEventListener('mouseup', onMouseUp);
}, [actions, selectActionAtFraction, fractionFromMouseEvent]);
const [, setHighlightedConsoleMessageOrdinal] = usePartitionedState<number | undefined>('highlightedConsoleMessageOrdinal');
const [revealedAttachmentCallId, setRevealedAttachmentCallId] = usePartitionedState<{ callId: string } | undefined>('revealedAttachmentCallId');
const [highlightedResourceKey, setHighlightedResourceKey] = usePartitionedState<string | undefined>('highlightedResourceKey');
const [, setHighlightedResourceKey] = usePartitionedState<string | undefined>('highlightedResourceKey');
Copy link
Member

Choose a reason for hiding this comment

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

delete as it's no longer used?

const [highlightedCallId, setHighlightedCallId] = usePartitionedState<string | undefined>('highlightedCallId');
const [revealedErrorKey, setRevealedErrorKey] = usePartitionedState<string | undefined>('revealedErrorKey');
const [highlightedConsoleMessageOrdinal, setHighlightedConsoleMessageOrdinal] = usePartitionedState<number | undefined>('highlightedConsoleMessageOrdinal');
const [, setHighlightedConsoleMessageOrdinal] = usePartitionedState<number | undefined>('highlightedConsoleMessageOrdinal');
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@github-actions
Copy link
Contributor

Test results for "tests 1"

41 failed
❌ [playwright-test] › playwright.unhandled.spec.ts:19 › should produce uncaughtException when page.route raises @macos-latest-node20
❌ [playwright-test] › reporter-html.spec.ts:636 › merged › should show trace title @ubuntu-latest-node20
❌ [playwright-test] › reporter-html.spec.ts:636 › created › should show trace title @ubuntu-latest-node20
❌ [playwright-test] › ui-mode-test-output.spec.ts:215 › should stream console messages live @ubuntu-latest-node20
❌ [playwright-test] › ui-mode-test-run.spec.ts:87 › should show running progress @ubuntu-latest-node20
❌ [playwright-test] › ui-mode-test-run.spec.ts:356 › should stop @ubuntu-latest-node20
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:31 › should run tests @ubuntu-latest-node20
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:63 › should stop tests @ubuntu-latest-node20
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:103 › should toggle Terminal @ubuntu-latest-node20
❌ [playwright-test] › reporter-html.spec.ts:636 › merged › should show trace title @macos-latest-node20
❌ [playwright-test] › reporter-html.spec.ts:636 › created › should show trace title @macos-latest-node20
❌ [playwright-test] › ui-mode-test-output.spec.ts:215 › should stream console messages live @macos-latest-node20
❌ [playwright-test] › ui-mode-test-run.spec.ts:87 › should show running progress @macos-latest-node20
❌ [playwright-test] › ui-mode-test-run.spec.ts:356 › should stop @macos-latest-node20
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:31 › should run tests @macos-latest-node20
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:63 › should stop tests @macos-latest-node20
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:103 › should toggle Terminal @macos-latest-node20
❌ [playwright-test] › reporter-html.spec.ts:636 › merged › should show trace title @ubuntu-latest-node24
❌ [playwright-test] › reporter-html.spec.ts:636 › created › should show trace title @ubuntu-latest-node24
❌ [playwright-test] › ui-mode-test-output.spec.ts:215 › should stream console messages live @ubuntu-latest-node24
❌ [playwright-test] › ui-mode-test-run.spec.ts:87 › should show running progress @ubuntu-latest-node24
❌ [playwright-test] › ui-mode-test-run.spec.ts:356 › should stop @ubuntu-latest-node24
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:31 › should run tests @ubuntu-latest-node24
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:63 › should stop tests @ubuntu-latest-node24
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:103 › should toggle Terminal @ubuntu-latest-node24
❌ [playwright-test] › reporter-html.spec.ts:636 › merged › should show trace title @ubuntu-latest-node22
❌ [playwright-test] › reporter-html.spec.ts:636 › created › should show trace title @ubuntu-latest-node22
❌ [playwright-test] › ui-mode-test-output.spec.ts:215 › should stream console messages live @ubuntu-latest-node22
❌ [playwright-test] › ui-mode-test-run.spec.ts:87 › should show running progress @ubuntu-latest-node22
❌ [playwright-test] › ui-mode-test-run.spec.ts:356 › should stop @ubuntu-latest-node22
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:31 › should run tests @ubuntu-latest-node22
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:63 › should stop tests @ubuntu-latest-node22
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:103 › should toggle Terminal @ubuntu-latest-node22
❌ [playwright-test] › ui-mode-test-output.spec.ts:215 › should stream console messages live @windows-latest-node20
❌ [playwright-test] › ui-mode-test-run.spec.ts:87 › should show running progress @windows-latest-node20
❌ [playwright-test] › ui-mode-test-run.spec.ts:356 › should stop @windows-latest-node20
❌ [playwright-test] › reporter-html.spec.ts:636 › merged › should show trace title @windows-latest-node20
❌ [playwright-test] › reporter-html.spec.ts:636 › created › should show trace title @windows-latest-node20
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:31 › should run tests @windows-latest-node20
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:63 › should stop tests @windows-latest-node20
❌ [playwright-test] › ui-mode-test-shortcut.spec.ts:103 › should toggle Terminal @windows-latest-node20

5 flaky ⚠️ [chromium-library] › library/trace-viewer.spec.ts:1223 › should display language-specific locators `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/popup.spec.ts:260 › should not throw when click closes popup `@chromium-ubuntu-22.04-node22`
⚠️ [firefox-library] › library/browsertype-connect.spec.ts:758 › launchServer › should upload a folder `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:1080 › cli codegen › should not throw csp directive violation errors `@firefox-ubuntu-22.04-node20`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:433 › should work behind reverse proxy `@macos-latest-node20`

38705 passed, 846 skipped


Merge workflow run.

@github-actions
Copy link
Contributor

Test results for "MCP"

5275 passed, 172 skipped


Merge workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants