Address PR comments

packages/trace-viewer/src/ui/DEPS.list

@@ -7,5 +7,4 @@
 ../geometry.ts
 ../../../playwright/src/isomorphic/**
 ../third_party/devtools.ts
-./settings/**
 ./shared/**

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

@@ -17,13 +17,16 @@
 import * as React from 'react';
 import { SettingsView } from './settingsView';
 import { useDarkModeSetting } from '@web/theme';
-import { useShouldPopulateCanvasFromScreenshot } from './settings/useShouldPopulateCanvasFromScreenshot';
+import { useSetting } from '@web/uiUtils';
 
+/**
+ * A view of the collection of standard settings used between various applications
+ */
 export const DefaultSettingsView: React.FC<{}> = () => {
   const [
     shouldPopulateCanvasFromScreenshot,
     setShouldPopulateCanvasFromScreenshot,
-  ] = useShouldPopulateCanvasFromScreenshot();
+  ] = useSetting('shouldPopulateCanvasFromScreenshot', false);
   const [darkMode, setDarkMode] = useDarkModeSetting();
 
   return (

packages/trace-viewer/src/ui/settings/useShouldPopulateCanvasFromScreenshot.ts

@@ -1,22 +0,0 @@
-/**
- * Copyright (c) Microsoft Corporation.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import { useSetting } from '@web/uiUtils';
-
-export const useShouldPopulateCanvasFromScreenshot = (): [
-  boolean,
-  (value: boolean) => void
-] => useSetting('shouldPopulateCanvasFromScreenshot', false);

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

@@ -1,21 +0,0 @@
-/*
-  Copyright (c) Microsoft Corporation.
-
-  Licensed under the Apache License, Version 2.0 (the "License");
-  you may not use this file except in compliance with the License.
-  You may obtain a copy of the License at
-
-      http://www.apache.org/licenses/LICENSE-2.0
-
-  Unless required by applicable law or agreed to in writing, software
-  distributed under the License is distributed on an "AS IS" BASIS,
-  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-  See the License for the specific language governing permissions and
-  limitations under the License.
-*/
-
-.settings-toolbar-dialog {
-  background-color: var(--vscode-sideBar-background);
-
-  padding: 4px 8px;
-}

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

@@ -18,7 +18,6 @@ import * as React from 'react';
 import { Dialog } from './shared/dialog';
 import { ToolbarButton } from '@web/components/toolbarButton';
 import { DefaultSettingsView } from './defaultSettingsView';
-import './settingsToolbar.css';
 
 export const SettingsToolbarButton: React.FC<{}> = () => {
   const hostingRef = React.useRef<HTMLButtonElement>(null);
@@ -34,7 +33,10 @@ export const SettingsToolbarButton: React.FC<{}> = () => {
         onClick={() => setOpen(current => !current)}
       />
       <Dialog
-        className='settings-toolbar-dialog'
+        style={{
+          backgroundColor: 'var(--vscode-sideBar-background)',
+          padding: '4px 8px'
+        }}
         open={open}
         width={200}
         // TODO: Temporary spacing until design of toolbar buttons is revisited

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

@@ -18,6 +18,7 @@ import * as React from 'react';
 
 export interface DialogProps {
   className?: string;
+  style?: React.CSSProperties;
   open: boolean;
   width: number;
   verticalOffset?: number;
@@ -27,6 +28,7 @@ export interface DialogProps {
 
 export const Dialog: React.FC<React.PropsWithChildren<DialogProps>> = ({
   className,
+  style: externalStyle,
   open,
   width,
   verticalOffset,
@@ -39,7 +41,7 @@ export const Dialog: React.FC<React.PropsWithChildren<DialogProps>> = ({
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   const [_, setRecalculateDimensionsCount] = React.useState(0);
 
-  let style: React.CSSProperties | undefined = undefined;
+  let style: React.CSSProperties | undefined = externalStyle;
 
   if (anchor?.current) {
     const bounds = anchor.current.getBoundingClientRect();
@@ -50,6 +52,7 @@ export const Dialog: React.FC<React.PropsWithChildren<DialogProps>> = ({
       left: buildTopLeftCoord(bounds, width),
       width,
       zIndex: 1,
+      ...externalStyle
     };
   }
 

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

@@ -20,7 +20,7 @@ import type { ActionTraceEvent } from '@trace/trace';
 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';
+import { clsx, useMeasure, useSetting } from '@web/uiUtils';
 import { InjectedScript } from '@injected/injectedScript';
 import { Recorder } from '@injected/recorder/recorder';
 import ConsoleAPI from '@injected/consoleApi';
@@ -30,7 +30,6 @@ import { locatorOrSelectorAsSelector } from '@isomorphic/locatorParser';
 import { TabbedPaneTab } from '@web/components/tabbedPane';
 import { BrowserFrame } from './browserFrame';
 import type { ElementInfo } from '@recorder/recorderTypes';
-import { useShouldPopulateCanvasFromScreenshot } from './settings/useShouldPopulateCanvasFromScreenshot';
 
 export const SnapshotTabsView: React.FunctionComponent<{
   action: ActionTraceEvent | undefined,
@@ -45,7 +44,7 @@ export const SnapshotTabsView: React.FunctionComponent<{
   const [snapshotTab, setSnapshotTab] = React.useState<'action'|'before'|'after'>('action');
 
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  const [shouldPopulateCanvasFromScreenshot, _] = useShouldPopulateCanvasFromScreenshot();
+  const [shouldPopulateCanvasFromScreenshot, _] = useSetting('shouldPopulateCanvasFromScreenshot', false);
 
   const snapshots = React.useMemo(() => {
     return collectSnapshots(action);

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

@@ -28,7 +28,6 @@ import { ToolbarButton } from '@web/components/toolbarButton';
 import { Toolbar } from '@web/components/toolbar';
 import type { XtermDataSource } from '@web/components/xtermWrapper';
 import { XtermWrapper } from '@web/components/xtermWrapper';
-import { useDarkModeSetting } from '@web/theme';
 import { clsx, settings, useSetting } from '@web/uiUtils';
 import { statusEx, TestTree } from '@testIsomorphic/testTree';
 import type { TreeItem  } from '@testIsomorphic/testTree';
@@ -105,7 +104,6 @@ export const UIModeView: React.FC<{}> = ({
   const [singleWorker, setSingleWorker] = React.useState(false);
   const [showBrowser, setShowBrowser] = React.useState(false);
   const [updateSnapshots, setUpdateSnapshots] = React.useState(false);
-  const [darkMode, setDarkMode] = useDarkModeSetting();
 
   const inputRef = React.useRef<HTMLInputElement>(null);