From bbe252a3d7292b84aed9178308ce3b26fa4ecce3 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Thu, 1 Aug 2024 05:36:19 -0700
Subject: [PATCH] fix(ui mode): api review feedback (#31952)

- Hide "Testing Options" as not ready.
- Update SettingsView margins.
- Include `page.route` and similar methods into "Show route actions".
---
 packages/trace-viewer/src/ui/modelUtil.ts   |  8 ++++++
 packages/trace-viewer/src/ui/uiModeView.css |  2 +-
 packages/trace-viewer/src/ui/uiModeView.tsx | 29 ++++++++++++---------
 packages/trace-viewer/src/ui/workbench.tsx  | 18 ++++++-------
 tests/library/trace-viewer.spec.ts          |  1 -
 5 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/packages/trace-viewer/src/ui/modelUtil.ts b/packages/trace-viewer/src/ui/modelUtil.ts
index 313ec700c7449..5949f4406cbba 100644
--- a/packages/trace-viewer/src/ui/modelUtil.ts
+++ b/packages/trace-viewer/src/ui/modelUtil.ts
@@ -409,3 +409,11 @@ function collectSources(actions: trace.ActionTraceEvent[], errorDescriptors: Err
   }
   return result;
 }
+
+const kRouteMethods = new Set([
+  'page.route', 'page.routefromhar', 'page.unroute', 'page.unrouteall',
+  'browsercontext.route', 'browsercontext.routefromhar', 'browsercontext.unroute', 'browsercontext.unrouteall',
+]);
+export function isRouteAction(action: ActionTraceEventInContext) {
+  return action.class === 'Route' || kRouteMethods.has(action.apiName.toLowerCase());
+}
diff --git a/packages/trace-viewer/src/ui/uiModeView.css b/packages/trace-viewer/src/ui/uiModeView.css
index f7463aaaac2b8..de89b911c33b7 100644
--- a/packages/trace-viewer/src/ui/uiModeView.css
+++ b/packages/trace-viewer/src/ui/uiModeView.css
@@ -24,7 +24,7 @@
 }
 
 .ui-mode-sidebar > .settings-view {
-  margin: 0 0 3px 23px;
+  margin: 0 0 8px 23px;
 }
 
 .ui-mode-sidebar input[type=search] {
diff --git a/packages/trace-viewer/src/ui/uiModeView.tsx b/packages/trace-viewer/src/ui/uiModeView.tsx
index 3484f3c749178..244a25ca5e2e7 100644
--- a/packages/trace-viewer/src/ui/uiModeView.tsx
+++ b/packages/trace-viewer/src/ui/uiModeView.tsx
@@ -99,6 +99,7 @@ export const UIModeView: React.FC<{}> = ({
   const [testingOptionsVisible, setTestingOptionsVisible] = React.useState(false);
   const [revealSource, setRevealSource] = React.useState(false);
   const onRevealSource = React.useCallback(() => setRevealSource(true), [setRevealSource]);
+  const showTestingOptions = false;
 
   const [runWorkers, setRunWorkers] = React.useState(queryParams.workers);
   const singleWorkerSetting = React.useMemo(() => {
@@ -509,19 +510,21 @@ export const UIModeView: React.FC<{}> = ({
           setFilterText={setFilterText}
           onRevealSource={onRevealSource}
         />
-        <Toolbar noShadow={true} noMinHeight={true} className='settings-toolbar' onClick={() => setTestingOptionsVisible(!testingOptionsVisible)}>
-          <span
-            className={`codicon codicon-${testingOptionsVisible ? 'chevron-down' : 'chevron-right'}`}
-            style={{ marginLeft: 5 }}
-            title={testingOptionsVisible ? 'Hide Testing Options' : 'Show Testing Options'}
-          />
-          <div className='section-title'>Testing Options</div>
-        </Toolbar>
-        {testingOptionsVisible && <SettingsView settings={[
-          singleWorkerSetting,
-          showBrowserSetting,
-          updateSnapshotsSetting,
-        ]} />}
+        {showTestingOptions && <>
+          <Toolbar noShadow={true} noMinHeight={true} className='settings-toolbar' onClick={() => setTestingOptionsVisible(!testingOptionsVisible)}>
+            <span
+              className={`codicon codicon-${testingOptionsVisible ? 'chevron-down' : 'chevron-right'}`}
+              style={{ marginLeft: 5 }}
+              title={testingOptionsVisible ? 'Hide Testing Options' : 'Show Testing Options'}
+            />
+            <div className='section-title'>Testing Options</div>
+          </Toolbar>
+          {testingOptionsVisible && <SettingsView settings={[
+            singleWorkerSetting,
+            showBrowserSetting,
+            updateSnapshotsSetting,
+          ]} />}
+        </>}
         <Toolbar noShadow={true} noMinHeight={true} className='settings-toolbar' onClick={() => setSettingsVisible(!settingsVisible)}>
           <span
             className={`codicon codicon-${settingsVisible ? 'chevron-down' : 'chevron-right'}`}
diff --git a/packages/trace-viewer/src/ui/workbench.tsx b/packages/trace-viewer/src/ui/workbench.tsx
index 0a5dd226eb5c8..c2e90754808a5 100644
--- a/packages/trace-viewer/src/ui/workbench.tsx
+++ b/packages/trace-viewer/src/ui/workbench.tsx
@@ -23,7 +23,7 @@ import { ErrorsTab, useErrorsTabModel } from './errorsTab';
 import type { ConsoleEntry } from './consoleTab';
 import { ConsoleTab, useConsoleTabModel } from './consoleTab';
 import type * as modelUtil from './modelUtil';
-import type { ActionTraceEventInContext, MultiTraceModel } from './modelUtil';
+import { isRouteAction } from './modelUtil';
 import type { StackFrame } from '@protocol/channels';
 import { NetworkTab, useNetworkTabModel } from './networkTab';
 import { SnapshotTab } from './snapshotTab';
@@ -45,12 +45,12 @@ import type { UITestStatus } from './testUtils';
 import { SettingsView } from './settingsView';
 
 export const Workbench: React.FunctionComponent<{
-  model?: MultiTraceModel,
+  model?: modelUtil.MultiTraceModel,
   showSourcesFirst?: boolean,
   rootDir?: string,
   fallbackLocation?: modelUtil.SourceLocation,
-  initialSelection?: ActionTraceEventInContext,
-  onSelectionChanged?: (action: ActionTraceEventInContext) => void,
+  initialSelection?: modelUtil.ActionTraceEventInContext,
+  onSelectionChanged?: (action: modelUtil.ActionTraceEventInContext) => void,
   isLive?: boolean,
   status?: UITestStatus,
   annotations?: { type: string; description?: string; }[];
@@ -60,9 +60,9 @@ export const Workbench: React.FunctionComponent<{
   revealSource?: boolean,
   showSettings?: boolean,
 }> = ({ model, showSourcesFirst, rootDir, fallbackLocation, initialSelection, onSelectionChanged, isLive, status, annotations, inert, openPage, onOpenExternally, revealSource, showSettings }) => {
-  const [selectedAction, setSelectedActionImpl] = React.useState<ActionTraceEventInContext | undefined>(undefined);
+  const [selectedAction, setSelectedActionImpl] = React.useState<modelUtil.ActionTraceEventInContext | undefined>(undefined);
   const [revealedStack, setRevealedStack] = React.useState<StackFrame[] | undefined>(undefined);
-  const [highlightedAction, setHighlightedAction] = React.useState<ActionTraceEventInContext | undefined>();
+  const [highlightedAction, setHighlightedAction] = React.useState<modelUtil.ActionTraceEventInContext | undefined>();
   const [highlightedEntry, setHighlightedEntry] = React.useState<Entry | undefined>();
   const [highlightedConsoleMessage, setHighlightedConsoleMessage] = React.useState<ConsoleEntry | undefined>();
   const [selectedNavigatorTab, setSelectedNavigatorTab] = React.useState<string>('actions');
@@ -75,10 +75,10 @@ export const Workbench: React.FunctionComponent<{
   const [showRouteActions, , showRouteActionsSetting] = useSetting('show-route-actions', true, 'Show route actions');
 
   const filteredActions = React.useMemo(() => {
-    return (model?.actions || []).filter(action => showRouteActions || action.class !== 'Route');
+    return (model?.actions || []).filter(action => showRouteActions || !isRouteAction(action));
   }, [model, showRouteActions]);
 
-  const setSelectedAction = React.useCallback((action: ActionTraceEventInContext | undefined) => {
+  const setSelectedAction = React.useCallback((action: modelUtil.ActionTraceEventInContext | undefined) => {
     setSelectedActionImpl(action);
     setRevealedStack(action?.stack);
   }, [setSelectedActionImpl, setRevealedStack]);
@@ -111,7 +111,7 @@ export const Workbench: React.FunctionComponent<{
     }
   }, [model, selectedAction, setSelectedAction, initialSelection]);
 
-  const onActionSelected = React.useCallback((action: ActionTraceEventInContext) => {
+  const onActionSelected = React.useCallback((action: modelUtil.ActionTraceEventInContext) => {
     setSelectedAction(action);
     onSelectionChanged?.(action);
   }, [setSelectedAction, onSelectionChanged]);
diff --git a/tests/library/trace-viewer.spec.ts b/tests/library/trace-viewer.spec.ts
index 6e79ccf0e469c..43a42512fd62c 100644
--- a/tests/library/trace-viewer.spec.ts
+++ b/tests/library/trace-viewer.spec.ts
@@ -1357,7 +1357,6 @@ test('should allow hiding route actions', {
   await traceViewer.page.getByRole('checkbox', { name: 'Show route actions' }).uncheck();
   await traceViewer.page.getByText('Actions', { exact: true }).click();
   await expect(traceViewer.actionTitles).toHaveText([
-    /page.route/,
     /page.goto.*empty.html/,
   ]);