chore(trace): remove screenshot instead of snapshot code (#33225)

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

@@ -91,7 +91,7 @@ export const EmbeddedWorkbenchLoader: React.FunctionComponent = () => {
     <div className='progress'>
       <div className='inner-progress' style={{ width: progress.total ? (100 * progress.done / progress.total) + '%' : 0 }}></div>
     </div>
-    <Workbench model={model} openPage={openPage} onOpenExternally={openSourceLocation} showSettings />
+    <Workbench model={model} openPage={openPage} onOpenExternally={openSourceLocation} />
     {!traceURLs.length && <div className='empty-state'>
       <div className='title'>Select test to see the trace</div>
     </div>}

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

@@ -22,16 +22,15 @@ import * as React from 'react';
 import './sourceTab.css';
 
 export const InspectorTab: React.FunctionComponent<{
-  showScreenshot: boolean,
   sdkLanguage: Language,
   setIsInspecting: (isInspecting: boolean) => void,
   highlightedLocator: string,
   setHighlightedLocator: (locator: string) => void,
-}> = ({ showScreenshot, sdkLanguage, setIsInspecting, highlightedLocator, setHighlightedLocator }) => {
+}> = ({ sdkLanguage, setIsInspecting, highlightedLocator, setHighlightedLocator }) => {
   return <div className='vbox' style={{ backgroundColor: 'var(--vscode-sideBar-background)' }}>
     <div style={{ margin: '10px 0px 10px 10px', color: 'var(--vscode-editorCodeLens-foreground)', flex: 'none' }}>Locator</div>
     <div style={{ margin: '0 10px 10px', flex: 'auto' }}>
-      <CodeMirrorWrapper text={showScreenshot ? '/* disable "show screenshot" setting to pick locator */' : highlightedLocator} language={sdkLanguage} focusOnChange={true} isFocused={true} wrapLines={true} onChange={text => {
+      <CodeMirrorWrapper text={highlightedLocator} language={sdkLanguage} focusOnChange={true} isFocused={true} wrapLines={true} onChange={text => {
         // Updating text needs to go first - react can squeeze a render between the state updates.
         setHighlightedLocator(text);
         setIsInspecting(false);

packages/trace-viewer/src/ui/modelUtil.ts

@@ -22,7 +22,6 @@ import type { ActionEntry, ContextEntry, PageEntry } from '../types/entries';
 import type { StackFrame } from '@protocol/channels';
 
 const contextSymbol = Symbol('context');
-const pageSymbol = Symbol('page');
 const nextInContextSymbol = Symbol('next');
 const prevInListSymbol = Symbol('prev');
 const eventsSymbol = Symbol('events');
@@ -148,7 +147,6 @@ function indexModel(context: ContextEntry) {
   for (let i = 0; i < context.actions.length; ++i) {
     const action = context.actions[i] as any;
     action[contextSymbol] = context;
-    action[pageSymbol] = context.pages.find(page => page.pageId === action.pageId);
   }
   let lastNonRouteAction = undefined;
   for (let i = context.actions.length - 1; i >= 0; i--) {
@@ -353,10 +351,6 @@ export function prevInList(action: ActionTraceEvent): ActionTraceEvent {
   return (action as any)[prevInListSymbol];
 }
 
-export function pageForAction(action: ActionTraceEvent): PageEntry {
-  return (action as any)[pageSymbol];
-}
-
 export function stats(action: ActionTraceEvent): { errors: number, warnings: number } {
   let errors = 0;
   let warnings = 0;

packages/trace-viewer/src/ui/recorder/recorderView.tsx

@@ -213,7 +213,6 @@ const PropertiesView: React.FunctionComponent<{
     id: 'inspector',
     title: 'Locator',
     render: () => <InspectorTab
-      showScreenshot={false}
       sdkLanguage={sdkLanguage}
       setIsInspecting={setIsInspecting}
       highlightedLocator={highlightedLocator}

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

@@ -17,7 +17,7 @@
 import './snapshotTab.css';
 import * as React from 'react';
 import type { ActionTraceEvent } from '@trace/trace';
-import { context, type MultiTraceModel, pageForAction, prevInList } from './modelUtil';
+import { context, type MultiTraceModel, prevInList } from './modelUtil';
 import { Toolbar } from '@web/components/toolbar';
 import { ToolbarButton } from '@web/components/toolbarButton';
 import { clsx, useMeasure } from '@web/uiUtils';
@@ -29,18 +29,8 @@ import type { Language } from '@isomorphic/locatorGenerators';
 import { locatorOrSelectorAsSelector } from '@isomorphic/locatorParser';
 import { TabbedPaneTab } from '@web/components/tabbedPane';
 import { BrowserFrame } from './browserFrame';
-import { ClickPointer } from './clickPointer';
 import type { ElementInfo } from '@recorder/recorderTypes';
 
-function findClosest<T>(items: T[], metric: (v: T) => number, target: number) {
-  return items.find((item, index) => {
-    if (index === items.length - 1)
-      return true;
-    const next = items[index + 1];
-    return Math.abs(metric(item) - target) < Math.abs(metric(next) - target);
-  });
-}
-
 export const SnapshotTabsView: React.FunctionComponent<{
   action: ActionTraceEvent | undefined,
   model?: MultiTraceModel,
@@ -53,7 +43,6 @@ export const SnapshotTabsView: React.FunctionComponent<{
   openPage?: (url: string, target?: string) => Window | any,
 }> = ({ action, sdkLanguage, testIdAttributeName, isInspecting, setIsInspecting, highlightedLocator, setHighlightedLocator, openPage }) => {
   const [snapshotTab, setSnapshotTab] = React.useState<'action'|'before'|'after'>('action');
-  const showScreenshotInsteadOfSnapshot = false;
 
   const snapshots = React.useMemo(() => {
     return collectSnapshots(action);
@@ -65,7 +54,7 @@ export const SnapshotTabsView: React.FunctionComponent<{
 
   return <div className='snapshot-tab vbox'>
     <Toolbar>
-      <ToolbarButton className='pick-locator' title={showScreenshotInsteadOfSnapshot ? 'Disable "screenshots instead of snapshots" to pick a locator' : 'Pick locator'} icon='target' toggled={isInspecting} onClick={() => setIsInspecting(!isInspecting)} disabled={showScreenshotInsteadOfSnapshot} />
+      <ToolbarButton className='pick-locator' title='Pick locator' icon='target' toggled={isInspecting} onClick={() => setIsInspecting(!isInspecting)} />
       {['action', 'before', 'after'].map(tab => {
         return <TabbedPaneTab
           key={tab}
@@ -76,7 +65,7 @@ export const SnapshotTabsView: React.FunctionComponent<{
         ></TabbedPaneTab>;
       })}
       <div style={{ flex: 'auto' }}></div>
-      <ToolbarButton icon='link-external' title={showScreenshotInsteadOfSnapshot ? 'Not available when showing screenshot' : 'Open snapshot in a new tab'} disabled={!snapshotUrls?.popoutUrl || showScreenshotInsteadOfSnapshot} onClick={() => {
+      <ToolbarButton icon='link-external' title='Open snapshot in a new tab' disabled={!snapshotUrls?.popoutUrl} onClick={() => {
         if (!openPage)
           openPage = window.open;
         const win = openPage(snapshotUrls?.popoutUrl || '', '_blank');
@@ -86,7 +75,7 @@ export const SnapshotTabsView: React.FunctionComponent<{
         });
       }} />
     </Toolbar>
-    {!showScreenshotInsteadOfSnapshot && <SnapshotView
+    <SnapshotView
       snapshotUrls={snapshotUrls}
       sdkLanguage={sdkLanguage}
       testIdAttributeName={testIdAttributeName}
@@ -94,11 +83,7 @@ export const SnapshotTabsView: React.FunctionComponent<{
       setIsInspecting={setIsInspecting}
       highlightedLocator={highlightedLocator}
       setHighlightedLocator={setHighlightedLocator}
-    />}
+    />
-    {showScreenshotInsteadOfSnapshot && <ScreenshotView
-      action={action}
-      snapshotUrls={snapshotUrls}
-      snapshot={snapshots[snapshotTab]} />}
   </div>;
 };
 
@@ -194,38 +179,6 @@ export const SnapshotView: React.FunctionComponent<{
   </div>;
 };
 
-export const ScreenshotView: React.FunctionComponent<{
-  action: ActionTraceEvent | undefined,
-  snapshotUrls: SnapshotUrls | undefined,
-  snapshot: Snapshot | undefined,
-}> = ({ action, snapshotUrls, snapshot }) => {
-  const [snapshotInfo, setSnapshotInfo] = React.useState<SnapshotInfo>({ viewport: kDefaultViewport, url: '' });
-  React.useEffect(() => {
-    fetchSnapshotInfo(snapshotUrls?.snapshotInfoUrl).then(setSnapshotInfo);
-  }, [snapshotUrls?.snapshotInfoUrl]);
-
-  const page = action ? pageForAction(action) : undefined;
-  const screencastFrame = React.useMemo(() => {
-    if (snapshotInfo.wallTime && page?.screencastFrames[0]?.frameSwapWallTime)
-      return findClosest(page.screencastFrames, frame => frame.frameSwapWallTime!, snapshotInfo.wallTime);
-
-    if (snapshotInfo.timestamp && page?.screencastFrames)
-      return findClosest(page.screencastFrames, frame => frame.timestamp, snapshotInfo.timestamp);
-  },
-  [page?.screencastFrames, snapshotInfo.timestamp, snapshotInfo.wallTime]);
-
-  const point = snapshot?.point;
-
-  return <SnapshotWrapper snapshotInfo={snapshotInfo}>
-    {screencastFrame && (
-      <>
-        {point && <ClickPointer point={point} />}
-        <img alt={`Screenshot of ${action?.apiName}`} src={`sha1/${screencastFrame.sha1}`} width={screencastFrame.width} height={screencastFrame.height} />
-      </>
-    )}
-  </SnapshotWrapper>;
-};
-
 const SnapshotWrapper: React.FunctionComponent<React.PropsWithChildren<{
   snapshotInfo: SnapshotInfo,
 }>> = ({ snapshotInfo, children }) => {

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

@@ -55,8 +55,7 @@ export const Workbench: React.FunctionComponent<{
   openPage?: (url: string, target?: string) => Window | any,
   onOpenExternally?: (location: modelUtil.SourceLocation) => void,
   revealSource?: boolean,
-  showSettings?: boolean,
+}> = ({ model, showSourcesFirst, rootDir, fallbackLocation, isLive, hideTimeline, status, annotations, inert, openPage, onOpenExternally, revealSource }) => {
-}> = ({ model, showSourcesFirst, rootDir, fallbackLocation, isLive, hideTimeline, status, annotations, inert, openPage, onOpenExternally, revealSource, showSettings }) => {
   const [selectedCallId, setSelectedCallId] = React.useState<string | undefined>(undefined);
   const [revealedError, setRevealedError] = React.useState<ErrorDescription | undefined>(undefined);
   const [highlightedCallId, setHighlightedCallId] = React.useState<string | undefined>();
@@ -68,7 +67,6 @@ export const Workbench: React.FunctionComponent<{
   const [highlightedLocator, setHighlightedLocator] = React.useState<string>('');
   const [selectedTime, setSelectedTime] = React.useState<Boundaries | undefined>();
   const [sidebarLocation, setSidebarLocation] = useSetting<'bottom' | 'right'>('propertiesSidebarLocation', 'bottom');
-  const showScreenshot = false;
 
   const setSelectedAction = React.useCallback((action: modelUtil.ActionTraceEventInContext | undefined) => {
     setSelectedCallId(action?.callId);
@@ -164,7 +162,6 @@ export const Workbench: React.FunctionComponent<{
     id: 'inspector',
     title: 'Locator',
     render: () => <InspectorTab
-      showScreenshot={showScreenshot}
       sdkLanguage={sdkLanguage}
       setIsInspecting={setIsInspecting}
       highlightedLocator={highlightedLocator}

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

@@ -165,7 +165,7 @@ export const WorkbenchLoader: React.FunctionComponent<{
     <div className='progress'>
       <div className='inner-progress' style={{ width: progress.total ? (100 * progress.done / progress.total) + '%' : 0 }}></div>
     </div>
-    <Workbench model={model} inert={showFileUploadDropArea} showSettings />
+    <Workbench model={model} inert={showFileUploadDropArea} />
     {fileForLocalModeError && <div className='drop-target'>
       <div>Trace Viewer uses Service Workers to show traces. To view trace:</div>
       <div style={{ paddingTop: 20 }}>

tests/library/trace-viewer.spec.ts

@@ -1422,24 +1422,6 @@ test('should serve css without content-type', async ({ page, runAndTrace, server
   await expect(snapshotFrame.locator('body')).toHaveCSS('background-color', 'rgb(255, 0, 0)', { timeout: 0 });
 });
 
-test.skip('should allow showing screenshots instead of snapshots', async ({ runAndTrace, page, server }) => {
-  const traceViewer = await runAndTrace(async () => {
-    await page.goto(server.PREFIX + '/one-style.html');
-    await page.waitForTimeout(1000); // ensure we could take a screenshot
-  });
-
-  const screenshot = traceViewer.page.getByAltText(`Screenshot of page.goto`);
-  const snapshot = (await traceViewer.snapshotFrame('page.goto')).owner();
-  await expect(snapshot).toBeVisible();
-  await expect(screenshot).not.toBeVisible();
-
-  await traceViewer.page.getByTitle('Settings').click();
-  await traceViewer.page.getByText('Show screenshot instead of snapshot').setChecked(true);
-
-  await expect(snapshot).not.toBeVisible();
-  await expect(screenshot).toBeVisible();
-});
-
 test('canvas clipping', async ({ runAndTrace, page, server }) => {
   const traceViewer = await runAndTrace(async () => {
     await page.goto(server.PREFIX + '/screenshots/canvas.html#canvas-on-edge');
@@ -1466,18 +1448,6 @@ test('canvas clipping in iframe', async ({ runAndTrace, page, server }) => {
   await expect(canvas).toHaveAttribute('title', `Playwright displays canvas contents on a best-effort basis. It doesn't support canvas elements inside an iframe yet. If this impacts your workflow, please open an issue so we can prioritize.`);
 });
 
-test.skip('should handle case where neither snapshots nor screenshots exist', async ({ runAndTrace, page, server }) => {
-  const traceViewer = await runAndTrace(async () => {
-    await page.goto(server.PREFIX + '/one-style.html');
-  }, { snapshots: false, screenshots: false });
-
-  await traceViewer.page.getByTitle('Settings').click();
-  await traceViewer.page.getByText('Show screenshot instead of snapshot').setChecked(true);
-
-  const screenshot = traceViewer.page.getByAltText(`Screenshot of page.goto > Action`);
-  await expect(screenshot).not.toBeVisible();
-});
-
 test('should show only one pointer with multilevel iframes', async ({ page, runAndTrace, server, browserName }) => {
   test.fixme(browserName === 'firefox', 'Elements in iframe are not marked');