refactor(ui): synchronize settings via useSyncExternalStore instead of prop drilling (#31911)

Broken out from https://github.com/microsoft/playwright/pull/31900, part
of https://github.com/microsoft/playwright/issues/31863.

Synchronizes different `useSettings` calls via `useSyncExternalStore`.
This saves us from having to drill down settings props everywhere,
without the big refactoring that a `Context` would be.
This commit is contained in:
Simon Knott 2024-07-30 17:57:31 +02:00 committed by GitHub
parent 8412d973c0
commit b8b562888e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 32 additions and 26 deletions

View File

@ -86,7 +86,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} />
<Workbench model={model} openPage={openPage} showSettings />
{!traceURLs.length && <div className='empty-state'>
<div className='title'>Select test to see the trace</div>
</div>}

View File

@ -25,15 +25,13 @@ import type { ContextEntry } from '../entries';
import type { SourceLocation } from './modelUtil';
import { idForAction, MultiTraceModel } from './modelUtil';
import { Workbench } from './workbench';
import { type Setting } from '@web/uiUtils';
export const TraceView: React.FC<{
showRouteActionsSetting: Setting<boolean>,
item: { treeItem?: TreeItem, testFile?: SourceLocation, testCase?: reporterTypes.TestCase },
rootDir?: string,
onOpenExternally?: (location: SourceLocation) => void,
revealSource?: boolean,
}> = ({ showRouteActionsSetting, item, rootDir, onOpenExternally, revealSource }) => {
}> = ({ 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);
@ -91,7 +89,6 @@ export const TraceView: React.FC<{
return <Workbench
key='workbench'
showRouteActionsSetting={showRouteActionsSetting}
model={model?.model}
showSourcesFirst={true}
rootDir={rootDir}

View File

@ -446,7 +446,6 @@ export const UIModeView: React.FC<{}> = ({
</div>
<div className={'vbox' + (isShowingOutput ? ' hidden' : '')}>
<TraceView
showRouteActionsSetting={showRouteActionsSetting}
item={selectedItem}
rootDir={testModel?.config?.rootDir}
revealSource={revealSource}

View File

@ -36,7 +36,7 @@ import { AttachmentsTab } from './attachmentsTab';
import type { Boundaries } from '../geometry';
import { InspectorTab } from './inspectorTab';
import { ToolbarButton } from '@web/components/toolbarButton';
import { useSetting, msToString, type Setting } from '@web/uiUtils';
import { useSetting, msToString } from '@web/uiUtils';
import type { Entry } from '@trace/har';
import './workbench.css';
import { testStatusIcon, testStatusText } from './testUtils';
@ -53,11 +53,11 @@ export const Workbench: React.FunctionComponent<{
isLive?: boolean,
status?: UITestStatus,
inert?: boolean,
showRouteActionsSetting?: Setting<boolean>,
openPage?: (url: string, target?: string) => Window | any,
onOpenExternally?: (location: modelUtil.SourceLocation) => void,
revealSource?: boolean,
}> = ({ showRouteActionsSetting, model, showSourcesFirst, rootDir, fallbackLocation, initialSelection, onSelectionChanged, isLive, status, inert, openPage, onOpenExternally, revealSource }) => {
showSettings?: boolean,
}> = ({ model, showSourcesFirst, rootDir, fallbackLocation, initialSelection, onSelectionChanged, isLive, status, inert, openPage, onOpenExternally, revealSource, showSettings }) => {
const [selectedAction, setSelectedActionImpl] = React.useState<ActionTraceEventInContext | undefined>(undefined);
const [revealedStack, setRevealedStack] = React.useState<StackFrame[] | undefined>(undefined);
const [highlightedAction, setHighlightedAction] = React.useState<ActionTraceEventInContext | undefined>();
@ -70,11 +70,7 @@ export const Workbench: React.FunctionComponent<{
const activeAction = model ? highlightedAction || selectedAction : undefined;
const [selectedTime, setSelectedTime] = React.useState<Boundaries | undefined>();
const [sidebarLocation, setSidebarLocation] = useSetting<'bottom' | 'right'>('propertiesSidebarLocation', 'bottom');
const [, , showRouteActionsSettingInternal] = useSetting(showRouteActionsSetting ? undefined : 'show-route-actions', true, 'Show route actions');
const showSettings = !showRouteActionsSetting;
showRouteActionsSetting ||= showRouteActionsSettingInternal;
const showRouteActions = showRouteActionsSetting[0];
const [showRouteActions, , showRouteActionsSetting] = useSetting('show-route-actions', true, 'Show route actions');
const filteredActions = React.useMemo(() => {
return (model?.actions || []).filter(action => showRouteActions || action.class !== 'Route');

View File

@ -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} />
<Workbench model={model} inert={showFileUploadDropArea} showSettings />
{fileForLocalModeError && <div className='drop-target'>
<div>Trace Viewer uses Service Workers to show traces. To view trace:</div>
<div style={{ paddingTop: 20 }}>

View File

@ -38,8 +38,14 @@ export const SplitView: React.FC<React.PropsWithChildren<SplitViewProps>> = ({
settingName,
children
}) => {
const [hSize, setHSize] = useSetting<number>(settingName ? settingName + '.' + orientation + ':size' : undefined, Math.max(minSidebarSize, sidebarSize) * window.devicePixelRatio);
const [vSize, setVSize] = useSetting<number>(settingName ? settingName + '.' + orientation + ':size' : undefined, Math.max(minSidebarSize, sidebarSize) * window.devicePixelRatio);
const defaultSize = Math.max(minSidebarSize, sidebarSize) * window.devicePixelRatio;
const hSetting = useSetting<number>((settingName ?? 'unused') + '.' + orientation + ':size', defaultSize);
const vSetting = useSetting<number>((settingName ?? 'unused') + '.' + orientation + ':size', defaultSize);
const hState = React.useState(defaultSize);
const vState = React.useState(defaultSize);
const [hSize, setHSize] = settingName ? hSetting : hState;
const [vSize, setVSize] = settingName ? vSetting : vState;
const [resizing, setResizing] = React.useState<{ offset: number, size: number } | null>(null);
const [measure, ref] = useMeasure<HTMLDivElement>();

View File

@ -141,26 +141,32 @@ export function copy(text: string) {
export type Setting<T> = readonly [T, (value: T) => void, string];
export function useSetting<S>(name: string | undefined, defaultValue: S, title?: string): [S, React.Dispatch<React.SetStateAction<S>>, Setting<S>] {
if (name)
defaultValue = settings.getObject(name, defaultValue);
const [value, setValue] = React.useState<S>(defaultValue);
const setValueWrapper = React.useCallback((value: React.SetStateAction<S>) => {
if (name)
settings.setObject(name, value);
setValue(value);
}, [name, setValue]);
export function useSetting<S>(name: string, defaultValue: S, title?: string): [S, (v: S) => void, Setting<S>] {
const subscribe = React.useCallback((onStoreChange: () => void) => {
settings.onChangeEmitter.addEventListener(name, onStoreChange);
return () => settings.onChangeEmitter.removeEventListener(name, onStoreChange);
}, [name]);
const value = React.useSyncExternalStore(subscribe, () => settings.getObject(name, defaultValue));
const setValueWrapper = React.useCallback((value: S) => {
settings.setObject(name, value);
}, [name]);
const setting = [value, setValueWrapper, title || name || ''] as Setting<S>;
return [value, setValueWrapper, setting];
}
export class Settings {
onChangeEmitter = new EventTarget();
getString(name: string, defaultValue: string): string {
return localStorage[name] || defaultValue;
}
setString(name: string, value: string) {
localStorage[name] = value;
this.onChangeEmitter.dispatchEvent(new Event(name));
if ((window as any).saveSettings)
(window as any).saveSettings();
}
@ -177,6 +183,8 @@ export class Settings {
setObject<T>(name: string, value: T) {
localStorage[name] = JSON.stringify(value);
this.onChangeEmitter.dispatchEvent(new Event(name));
if ((window as any).saveSettings)
(window as any).saveSettings();
}