cherry-pick(#31894): feat(ui mode): ui updates (#31916)

- Update copy to clipboard button.
- Reveal test source in the Source tab instead of external editor.
- New button to reveal in the external editor in the Source tab.
- Move the Pick Locator button next to snapshot tabs.

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

@@ -55,16 +55,16 @@
   overflow: hidden;
   line-height: 18px;
   white-space: nowrap;
+  max-height: 18px;
 }
 
-.call-line .copy-icon {
+.call-line:not(:hover) .toolbar-button.copy {
   display: none;
-  margin-left: 5px;
 }
 
-.call-line:hover .copy-icon {
-  display: block;
-  cursor: pointer;
+.call-line .toolbar-button.copy {
+  margin-left: 5px;
+  transform: scale(0.8);
 }
 
 .call-value {

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

@@ -15,23 +15,24 @@
  */
 
 import * as React from 'react';
+import { ToolbarButton } from '@web/components/toolbarButton';
 
 export const CopyToClipboard: React.FunctionComponent<{
   value: string,
   description?: string,
 }> = ({ value, description }) => {
-  const [iconClassName, setIconClassName] = React.useState('codicon-clippy');
+  const [icon, setIcon] = React.useState('copy');
 
   const handleCopy = React.useCallback(() => {
     navigator.clipboard.writeText(value).then(() => {
-      setIconClassName('codicon-check');
+      setIcon('check');
       setTimeout(() => {
-        setIconClassName('codicon-clippy');
+        setIcon('copy');
       }, 3000);
     }, () => {
-      setIconClassName('codicon-close');
+      setIcon('close');
     });
 
   }, [value]);
-  return <span title={description ? description : 'Copy'} className={`copy-icon codicon ${iconClassName}`} onClick={handleCopy}/>;
-};
+  return <ToolbarButton title={description ? description : 'Copy'} icon={icon} onClick={handleCopy}/>;
+};

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

@@ -29,7 +29,8 @@ const eventsSymbol = Symbol('events');
 export type SourceLocation = {
   file: string;
   line: number;
-  source: SourceModel;
+  column: number;
+  source?: SourceModel;
 };
 
 export type SourceModel = {

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

@@ -28,6 +28,10 @@
   background-color: var(--vscode-sideBar-background);
 }
 
+.snapshot-tab .toolbar .pick-locator {
+  margin: 0 4px;
+}
+
 .snapshot-controls {
   flex: none;
   background-color: var(--vscode-sideBar-background);
@@ -102,29 +106,6 @@ iframe.snapshot-visible[name=snapshot] {
   padding: 50px;
 }
 
-.popout-icon {
-  position: absolute;
-  top: 0;
-  right: 0;
-  color: var(--vscode-sideBarTitle-foreground);
-  font-size: 14px;
-  z-index: 100;
-  width: 24px;
-  height: 24px;
-  display: flex;
-  align-items: center;
-  justify-content: center;
-  text-decoration: none;
-}
-
-.popout-icon:not(.popout-disabled):hover {
-  color: var(--vscode-foreground);
-}
-
-.popout-icon.popout-disabled {
-  opacity: var(--vscode-disabledForeground);
-}
-
 .snapshot-tab .cm-wrapper {
   line-height: 23px;
   margin-right: 4px;

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

@@ -181,6 +181,7 @@ export const SnapshotTab: React.FunctionComponent<{
       iframe={iframeRef1.current}
       iteration={loadingRef.current.iteration} />
     <Toolbar>
+      <ToolbarButton className='pick-locator' title='Pick locator' icon='target' toggled={isInspecting} onClick={() => setIsInspecting(!isInspecting)} />
       {['action', 'before', 'after'].map(tab => {
         return <TabbedPaneTab
           id={tab}

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

@@ -23,21 +23,9 @@
 }
 
 .source-tab-file-name {
-  height: 24px;
-  margin-left: 8px;
+  padding-left: 8px;
+  height: 100%;
   display: flex;
   align-items: center;
-  background-color: var(--vscode-breadcrumb-background);
-  box-shadow: var(--vscode-scrollbar-shadow) 0 6px 6px -6px;
-  z-index: 10;
+  flex: 1 1 auto;
 }
-
-.source-tab-file-name .copy-icon.codicon {
-  display: block;
-  cursor: pointer;
-}
-
-.source-copy-to-clipboard {
-  display: block;
-  padding-left: 4px;
-}

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

@@ -24,6 +24,8 @@ import type { SourceHighlight } from '@web/components/codeMirrorWrapper';
 import type { SourceLocation, SourceModel } from './modelUtil';
 import type { StackFrame } from '@protocol/channels';
 import { CopyToClipboard } from './copyToClipboard';
+import { ToolbarButton } from '@web/components/toolbarButton';
+import { Toolbar } from '@web/components/toolbar';
 
 export const SourceTab: React.FunctionComponent<{
   stack: StackFrame[] | undefined,
@@ -31,7 +33,8 @@ export const SourceTab: React.FunctionComponent<{
   sources: Map<string, SourceModel>,
   rootDir?: string,
   fallbackLocation?: SourceLocation,
-}> = ({ stack, sources, rootDir, fallbackLocation, stackFrameLocation }) => {
+  onOpenExternally?: (location: SourceLocation) => void,
+}> = ({ stack, sources, rootDir, fallbackLocation, stackFrameLocation, onOpenExternally }) => {
   const [lastStack, setLastStack] = React.useState<StackFrame[] | undefined>();
   const [selectedFrame, setSelectedFrame] = React.useState<number>(0);
 
@@ -42,7 +45,7 @@ export const SourceTab: React.FunctionComponent<{
     }
   }, [stack, lastStack, setLastStack, setSelectedFrame]);
 
-  const { source, highlight, targetLine, fileName } = useAsyncMemo<{ source: SourceModel, targetLine?: number, fileName?: string, highlight: SourceHighlight[] }>(async () => {
+  const { source, highlight, targetLine, fileName, location } = useAsyncMemo<{ source: SourceModel, targetLine?: number, fileName?: string, highlight: SourceHighlight[], location?: SourceLocation }>(async () => {
     const actionLocation = stack?.[selectedFrame];
     const shouldUseFallback = !actionLocation?.file;
     if (shouldUseFallback && !fallbackLocation)
@@ -56,6 +59,7 @@ export const SourceTab: React.FunctionComponent<{
       sources.set(file, source);
     }
 
+    const location = shouldUseFallback ? fallbackLocation! : actionLocation;
     const targetLine = shouldUseFallback ? fallbackLocation?.line || source.errors[0]?.line || 0 : actionLocation.line;
     const fileName = rootDir && file.startsWith(rootDir) ? file.substring(rootDir.length + 1) : file;
     const highlight: SourceHighlight[] = source.errors.map(e => ({ type: 'error', line: e.line, message: e.message }));
@@ -76,21 +80,29 @@ export const SourceTab: React.FunctionComponent<{
         source.content = `<Unable to read "${file}">`;
       }
     }
-    return { source, highlight, targetLine, fileName };
+    return { source, highlight, targetLine, fileName, location };
   }, [stack, selectedFrame, rootDir, fallbackLocation], { source: { errors: [], content: 'Loading\u2026' }, highlight: [] });
 
+  const openExternally = React.useCallback(() => {
+    if (!location)
+      return;
+    if (onOpenExternally) {
+      onOpenExternally(location);
+    } else {
+      // This should open an external protocol handler instead of actually navigating away.
+      window.location.href = `vscode://file//${location.file}:${location.line}`;
+    }
+  }, [onOpenExternally, location]);
+
   const showStackFrames = (stack?.length ?? 0) > 1;
 
   return <SplitView sidebarSize={200} orientation={stackFrameLocation === 'bottom' ? 'vertical' : 'horizontal'} sidebarHidden={!showStackFrames}>
     <div className='vbox' data-testid='source-code'>
-      {fileName && (
-        <div className='source-tab-file-name'>
-          {fileName}
-          <span className='source-copy-to-clipboard'>
-            <CopyToClipboard description='Copy filename' value={getFileName(fileName, targetLine)}/>
-          </span>
-        </div>
-      )}
+      { fileName && <Toolbar>
+        <span className='source-tab-file-name'>{fileName}</span>
+        <CopyToClipboard description='Copy filename' value={getFileName(fileName, targetLine)}/>
+        {location && <ToolbarButton icon='link-external' title='Open in VS Code' onClick={openExternally}></ToolbarButton>}
+      </Toolbar> }
       <CodeMirrorWrapper text={source.content || ''} language='javascript' highlight={highlight} revealLine={targetLine} readOnly={true} lineNumbers={true} />
     </div>
     <StackTraceView stack={stack} selectedFrame={selectedFrame} setSelectedFrame={setSelectedFrame} />

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

@@ -47,8 +47,9 @@ export const TestListView: React.FC<{
   isLoading?: boolean,
   onItemSelected: (item: { treeItem?: TreeItem, testCase?: reporterTypes.TestCase, testFile?: SourceLocation }) => void,
   requestedCollapseAllCount: number,
-  setFilterText: (text: string) => void;
-}> = ({ filterText, testModel, testServerConnection, testTree, runTests, runningState, watchAll, watchedTreeIds, setWatchedTreeIds, isLoading, onItemSelected, requestedCollapseAllCount, setFilterText }) => {
+  setFilterText: (text: string) => void,
+  onRevealSource: () => void,
+}> = ({ filterText, testModel, testServerConnection, testTree, runTests, runningState, watchAll, watchedTreeIds, setWatchedTreeIds, isLoading, onItemSelected, requestedCollapseAllCount, setFilterText, onRevealSource }) => {
   const [treeState, setTreeState] = React.useState<TreeState>({ expandedItems: new Map() });
   const [selectedTreeItemId, setSelectedTreeItemId] = React.useState<string | undefined>();
   const [collapseAllCount, setCollapseAllCount] = React.useState(requestedCollapseAllCount);
@@ -91,17 +92,7 @@ export const TestListView: React.FC<{
     if (!testModel)
       return { selectedTreeItem: undefined };
     const selectedTreeItem = selectedTreeItemId ? testTree.treeItemById(selectedTreeItemId) : undefined;
-    let testFile: SourceLocation | undefined;
-    if (selectedTreeItem) {
-      testFile = {
-        file: selectedTreeItem.location.file,
-        line: selectedTreeItem.location.line,
-        source: {
-          errors: testModel.loadErrors.filter(e => e.location?.file === selectedTreeItem.location.file).map(e => ({ line: e.location!.line, message: e.message! })),
-          content: undefined,
-        }
-      };
-    }
+    const testFile = itemLocation(selectedTreeItem, testModel);
     let selectedTest: reporterTypes.TestCase | undefined;
     if (selectedTreeItem?.kind === 'test')
       selectedTest = selectedTreeItem.test;
@@ -164,7 +155,7 @@ export const TestListView: React.FC<{
         {!!treeItem.duration && treeItem.status !== 'skipped' && <div className='ui-mode-list-item-time'>{msToString(treeItem.duration)}</div>}
         <Toolbar noMinHeight={true} noShadow={true}>
           <ToolbarButton icon='play' title='Run' onClick={() => runTreeItem(treeItem)} disabled={!!runningState}></ToolbarButton>
-          <ToolbarButton icon='go-to-file' title='Open in VS Code' onClick={() => testServerConnection?.openNoReply({ location: treeItem.location })} style={(treeItem.kind === 'group' && treeItem.subKind === 'folder') ? { visibility: 'hidden' } : {}}></ToolbarButton>
+          <ToolbarButton icon='go-to-file' title='Show source' onClick={onRevealSource} style={(treeItem.kind === 'group' && treeItem.subKind === 'folder') ? { visibility: 'hidden' } : {}}></ToolbarButton>
           {!watchAll && <ToolbarButton icon='eye' title='Watch' onClick={() => {
             if (watchedTreeIds.value.has(treeItem.id))
               watchedTreeIds.value.delete(treeItem.id);
@@ -187,3 +178,17 @@ export const TestListView: React.FC<{
     autoExpandDepth={filterText ? 5 : 1}
     noItemsMessage={isLoading ? 'Loading\u2026' : 'No tests'} />;
 };
+
+function itemLocation(item: TreeItem | undefined, model: TestModel | undefined): SourceLocation | undefined {
+  if (!item || !model)
+    return;
+  return {
+    file: item.location.file,
+    line: item.location.line,
+    column: item.location.column,
+    source: {
+      errors: model.loadErrors.filter(e => e.location?.file === item.location.file).map(e => ({ line: e.location!.line, message: e.message! })),
+      content: undefined,
+    }
+  };
+}

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

@@ -31,7 +31,9 @@ export const TraceView: React.FC<{
   showRouteActionsSetting: Setting<boolean>,
   item: { treeItem?: TreeItem, testFile?: SourceLocation, testCase?: reporterTypes.TestCase },
   rootDir?: string,
-}> = ({ showRouteActionsSetting, item, rootDir }) => {
+  onOpenExternally?: (location: SourceLocation) => void,
+  revealSource?: boolean,
+}> = ({ showRouteActionsSetting, item, rootDir, onOpenExternally, revealSource }) => {
   const [model, setModel] = React.useState<{ model: MultiTraceModel, isLive: boolean } | undefined>();
   const [counter, setCounter] = React.useState(0);
   const pollTimer = React.useRef<NodeJS.Timeout | null>(null);
@@ -97,7 +99,10 @@ export const TraceView: React.FC<{
     onSelectionChanged={onSelectionChanged}
     fallbackLocation={item.testFile}
     isLive={model?.isLive}
-    status={item.treeItem?.status} />;
+    status={item.treeItem?.status}
+    onOpenExternally={onOpenExternally}
+    revealSource={revealSource}
+  />;
 };
 
 const outputDirForTestCase = (testCase: reporterTypes.TestCase): string | undefined => {

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

@@ -96,6 +96,8 @@ export const UIModeView: React.FC<{}> = ({
   const [testServerConnection, setTestServerConnection] = React.useState<TestServerConnection>();
   const [settingsVisible, setSettingsVisible] = React.useState(false);
   const [testingOptionsVisible, setTestingOptionsVisible] = React.useState(false);
+  const [revealSource, setRevealSource] = React.useState(false);
+  const onRevealSource = React.useCallback(() => setRevealSource(true), [setRevealSource]);
 
   const [runWorkers, setRunWorkers] = React.useState(queryParams.workers);
   const singleWorkerSetting = React.useMemo(() => {
@@ -435,7 +437,13 @@ export const UIModeView: React.FC<{}> = ({
           <XtermWrapper source={xtermDataSource}></XtermWrapper>
         </div>
         <div className={'vbox' + (isShowingOutput ? ' hidden' : '')}>
-          <TraceView showRouteActionsSetting={showRouteActionsSetting} item={selectedItem} rootDir={testModel?.config?.rootDir} />
+          <TraceView
+            showRouteActionsSetting={showRouteActionsSetting}
+            item={selectedItem}
+            rootDir={testModel?.config?.rootDir}
+            revealSource={revealSource}
+            onOpenExternally={location => testServerConnection?.openNoReply({ location: { file: location.file, line: location.line, column: location.column } })}
+          />
         </div>
       </div>
       <div className='vbox ui-mode-sidebar'>
@@ -487,6 +495,7 @@ export const UIModeView: React.FC<{}> = ({
           isLoading={isLoading}
           requestedCollapseAllCount={collapseAllCount}
           setFilterText={setFilterText}
+          onRevealSource={onRevealSource}
         />
         <Toolbar noShadow={true} noMinHeight={true} className='settings-toolbar' onClick={() => setTestingOptionsVisible(!testingOptionsVisible)}>
           <span

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

@@ -55,7 +55,9 @@ export const Workbench: React.FunctionComponent<{
   inert?: boolean,
   showRouteActionsSetting?: Setting<boolean>,
   openPage?: (url: string, target?: string) => Window | any,
-}> = ({ showRouteActionsSetting, model, showSourcesFirst, rootDir, fallbackLocation, initialSelection, onSelectionChanged, isLive, status, inert, openPage }) => {
+  onOpenExternally?: (location: modelUtil.SourceLocation) => void,
+  revealSource?: boolean,
+}> = ({ showRouteActionsSetting, model, showSourcesFirst, rootDir, fallbackLocation, initialSelection, onSelectionChanged, isLive, status, inert, openPage, onOpenExternally, revealSource }) => {
   const [selectedAction, setSelectedActionImpl] = React.useState<ActionTraceEventInContext | undefined>(undefined);
   const [revealedStack, setRevealedStack] = React.useState<StackFrame[] | undefined>(undefined);
   const [highlightedAction, setHighlightedAction] = React.useState<ActionTraceEventInContext | undefined>();
@@ -63,7 +65,7 @@ export const Workbench: React.FunctionComponent<{
   const [highlightedConsoleMessage, setHighlightedConsoleMessage] = React.useState<ConsoleEntry | undefined>();
   const [selectedNavigatorTab, setSelectedNavigatorTab] = React.useState<string>('actions');
   const [selectedPropertiesTab, setSelectedPropertiesTab] = useSetting<string>('propertiesTab', showSourcesFirst ? 'source' : 'call');
-  const [isInspecting, setIsInspecting] = React.useState(false);
+  const [isInspecting, setIsInspectingState] = React.useState(false);
   const [highlightedLocator, setHighlightedLocator] = React.useState<string>('');
   const activeAction = model ? highlightedAction || selectedAction : undefined;
   const [selectedTime, setSelectedTime] = React.useState<Boundaries | undefined>();
@@ -87,6 +89,7 @@ export const Workbench: React.FunctionComponent<{
 
   React.useEffect(() => {
     setSelectedTime(undefined);
+    setRevealedStack(undefined);
   }, [model]);
 
   React.useEffect(() => {
@@ -118,14 +121,25 @@ export const Workbench: React.FunctionComponent<{
   const selectPropertiesTab = React.useCallback((tab: string) => {
     setSelectedPropertiesTab(tab);
     if (tab !== 'inspector')
-      setIsInspecting(false);
+      setIsInspectingState(false);
   }, [setSelectedPropertiesTab]);
 
+  const setIsInspecting = React.useCallback((value: boolean) => {
+    if (!isInspecting && value)
+      selectPropertiesTab('inspector');
+    setIsInspectingState(value);
+  }, [setIsInspectingState, selectPropertiesTab, isInspecting]);
+
   const locatorPicked = React.useCallback((locator: string) => {
     setHighlightedLocator(locator);
     selectPropertiesTab('inspector');
   }, [selectPropertiesTab]);
 
+  React.useEffect(() => {
+    if (revealSource)
+      selectPropertiesTab('source');
+  }, [revealSource, selectPropertiesTab]);
+
   const consoleModel = useConsoleTabModel(model, selectedTime);
   const networkModel = useNetworkTabModel(model, selectedTime);
   const errorsModel = useErrorsTabModel(model);
@@ -174,7 +188,9 @@ export const Workbench: React.FunctionComponent<{
       sources={sources}
       rootDir={rootDir}
       stackFrameLocation={sidebarLocation === 'bottom' ? 'right' : 'bottom'}
-      fallbackLocation={fallbackLocation} />
+      fallbackLocation={fallbackLocation}
+      onOpenExternally={onOpenExternally}
+    />
   };
   const consoleTab: TabbedPaneTabModel = {
     id: 'console',
@@ -302,13 +318,6 @@ export const Workbench: React.FunctionComponent<{
         tabs={tabs}
         selectedTab={selectedPropertiesTab}
         setSelectedTab={selectPropertiesTab}
-        leftToolbar={[
-          <ToolbarButton title='Pick locator' icon='target' toggled={isInspecting} onClick={() => {
-            if (!isInspecting)
-              selectPropertiesTab('inspector');
-            setIsInspecting(!isInspecting);
-          }} />
-        ]}
         rightToolbar={[
           sidebarLocation === 'bottom' ?
             <ToolbarButton title='Dock to right' icon='layout-sidebar-right-off' onClick={() => {

packages/web/src/components/toolbarButton.tsx

@@ -26,6 +26,7 @@ export interface ToolbarButtonProps {
   onClick: (e: React.MouseEvent) => void,
   style?: React.CSSProperties,
   testId?: string,
+  className?: string,
 }
 
 export const ToolbarButton: React.FC<React.PropsWithChildren<ToolbarButtonProps>> = ({
@@ -37,8 +38,9 @@ export const ToolbarButton: React.FC<React.PropsWithChildren<ToolbarButtonProps>
   onClick = () => {},
   style,
   testId,
+  className,
 }) => {
-  let className = `toolbar-button ${icon}`;
+  className = (className || '') + ` toolbar-button ${icon}`;
   if (toggled)
     className += ' toggled';
   return <button

tests/playwright-test/ui-mode-test-update.spec.ts

@@ -217,7 +217,8 @@ test('should update test locations', async ({ runUITest, writeFiles }) => {
 
   const passesItemLocator = page.getByRole('listitem').filter({ hasText: 'passes' });
   await passesItemLocator.hover();
-  await passesItemLocator.getByTitle('Open in VS Code').click();
+  await passesItemLocator.getByTitle('Show source').click();
+  await page.getByTitle('Open in VS Code').click();
 
   expect(messages).toEqual([{
     method: 'open',
@@ -247,7 +248,8 @@ test('should update test locations', async ({ runUITest, writeFiles }) => {
 
   messages.length = 0;
   await passesItemLocator.hover();
-  await passesItemLocator.getByTitle('Open in VS Code').click();
+  await passesItemLocator.getByTitle('Show source').click();
+  await page.getByTitle('Open in VS Code').click();
 
   expect(messages).toEqual([{
     method: 'open',