From 99724d0322fc3e1ff63ee4b5ce742e76c6a06cd1 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Wed, 31 Jul 2024 12:12:06 +0200 Subject: [PATCH] refactor(ui): some react refactorings (#31900) Addresses https://github.com/microsoft/playwright/issues/31863. This PR is chonky, but the individual commits should be easy to review. If they're not, i'm happy to break them out into individual PRs. There's two main things this does: 1. Remove some unused imports 2. Add a `clsx`-inspired helper function for classname templating I wasn't able to replace `ReactDOM.render` with `ReactDOM.createRoot`. This is the new recommended way starting with React 18, and the existing one is going to be deprecated at some point. But it somehow breaks our tests, i'll have to investigate that separately. --- packages/html-reporter/src/chip.tsx | 5 +++-- packages/html-reporter/src/links.tsx | 7 ++++--- packages/html-reporter/src/tabbedPane.tsx | 3 ++- packages/html-reporter/src/testCaseView.spec.tsx | 1 - packages/html-reporter/src/testCaseView.tsx | 3 ++- packages/html-reporter/src/testFileView.tsx | 5 +++-- packages/recorder/src/callLog.tsx | 8 ++++---- packages/trace-viewer/src/embedded.tsx | 1 - packages/trace-viewer/src/index.tsx | 1 - packages/trace-viewer/src/ui/callTab.tsx | 4 ++-- packages/trace-viewer/src/ui/consoleTab.tsx | 6 +++--- packages/trace-viewer/src/ui/snapshotTab.tsx | 6 +++--- packages/trace-viewer/src/ui/tag.tsx | 3 ++- packages/trace-viewer/src/ui/timeline.tsx | 13 +++++++------ packages/trace-viewer/src/ui/uiModeView.tsx | 6 +++--- packages/trace-viewer/src/ui/workbench.tsx | 4 ++-- packages/trace-viewer/src/uiMode.tsx | 1 - .../web/src/components/codeMirrorWrapper.spec.tsx | 1 - packages/web/src/components/expandable.spec.tsx | 1 - packages/web/src/components/expandable.tsx | 5 +++-- packages/web/src/components/splitView.spec.tsx | 1 - packages/web/src/components/splitView.tsx | 4 ++-- packages/web/src/components/tabbedPane.tsx | 3 ++- packages/web/src/components/toolbar.tsx | 3 ++- packages/web/src/shared/imageDiffView.spec.tsx | 1 - packages/web/src/uiUtils.ts | 5 +++++ 26 files changed, 54 insertions(+), 47 deletions(-) diff --git a/packages/html-reporter/src/chip.tsx b/packages/html-reporter/src/chip.tsx index d236141475..8f4badf97f 100644 --- a/packages/html-reporter/src/chip.tsx +++ b/packages/html-reporter/src/chip.tsx @@ -19,6 +19,7 @@ import './chip.css'; import './colors.css'; import './common.css'; import * as icons from './icons'; +import { clsx } from '@web/uiUtils'; export const Chip: React.FC<{ header: JSX.Element | string, @@ -31,14 +32,14 @@ export const Chip: React.FC<{ }> = ({ header, expanded, setExpanded, children, noInsets, dataTestId, targetRef }) => { return
setExpanded?.(!expanded)} title={typeof header === 'string' ? header : undefined}> {setExpanded && !!expanded && icons.downArrow()} {setExpanded && !expanded && icons.rightArrow()} {header}
- {(!setExpanded || expanded) &&
{children}
} + {(!setExpanded || expanded) &&
{children}
}
; }; diff --git a/packages/html-reporter/src/links.tsx b/packages/html-reporter/src/links.tsx index 42b9f9b5b9..5db482d2a5 100644 --- a/packages/html-reporter/src/links.tsx +++ b/packages/html-reporter/src/links.tsx @@ -21,6 +21,7 @@ import { TreeItem } from './treeItem'; import { CopyToClipboard } from './copyToClipboard'; import './links.css'; import { linkifyText } from './renderUtils'; +import { clsx } from '@web/uiUtils'; export function navigate(href: string) { window.history.pushState({}, '', href); @@ -48,8 +49,8 @@ export const Link: React.FunctionComponent<{ className?: string, title?: string, children: any, -}> = ({ href, click, ctrlClick, className, children, title }) => { - return { +}> = ({ click, ctrlClick, children, ...rest }) => { + return { if (click) { e.preventDefault(); navigate(e.metaKey || e.ctrlKey ? ctrlClick || click : click); @@ -64,7 +65,7 @@ export const ProjectLink: React.FunctionComponent<{ const encoded = encodeURIComponent(projectName); const value = projectName === encoded ? projectName : `"${encoded.replace(/%22/g, '%5C%22')}"`; return - + {projectName} ; diff --git a/packages/html-reporter/src/tabbedPane.tsx b/packages/html-reporter/src/tabbedPane.tsx index 769622137a..02d0c6f3b1 100644 --- a/packages/html-reporter/src/tabbedPane.tsx +++ b/packages/html-reporter/src/tabbedPane.tsx @@ -14,6 +14,7 @@ * limitations under the License. */ +import { clsx } from '@web/uiUtils'; import './tabbedPane.css'; import * as React from 'react'; @@ -34,7 +35,7 @@ export const TabbedPane: React.FunctionComponent<{
{ tabs.map(tab => ( -
setSelectedTab(tab.id)} key={tab.id}>
{tab.title}
diff --git a/packages/html-reporter/src/testCaseView.spec.tsx b/packages/html-reporter/src/testCaseView.spec.tsx index 624a93805f..afe06cebcb 100644 --- a/packages/html-reporter/src/testCaseView.spec.tsx +++ b/packages/html-reporter/src/testCaseView.spec.tsx @@ -14,7 +14,6 @@ * limitations under the License. */ -import React from 'react'; import { test, expect } from '@playwright/experimental-ct-react'; import { TestCaseView } from './testCaseView'; import type { TestCase, TestResult } from './types'; diff --git a/packages/html-reporter/src/testCaseView.tsx b/packages/html-reporter/src/testCaseView.tsx index f4d76653cf..5647f3bcf1 100644 --- a/packages/html-reporter/src/testCaseView.tsx +++ b/packages/html-reporter/src/testCaseView.tsx @@ -25,6 +25,7 @@ import './testCaseView.css'; import { TestResultView } from './testResultView'; import { linkifyText } from './renderUtils'; import { hashStringToInt, msToString } from './utils'; +import { clsx } from '@web/uiUtils'; export const TestCaseView: React.FC<{ projectNames: string[], @@ -90,7 +91,7 @@ const LabelsLinkView: React.FC {labels.map(label => (
- + {label.slice(1)} diff --git a/packages/html-reporter/src/testFileView.tsx b/packages/html-reporter/src/testFileView.tsx index 184b2b63ba..bd402a74de 100644 --- a/packages/html-reporter/src/testFileView.tsx +++ b/packages/html-reporter/src/testFileView.tsx @@ -23,6 +23,7 @@ import { generateTraceUrl, Link, navigate, ProjectLink } from './links'; import { statusIcon } from './statusIcon'; import './testFileView.css'; import { video, image, trace } from './icons'; +import { clsx } from '@web/uiUtils'; export const TestFileView: React.FC}> {file.tests.filter(t => filter.matches(t)).map(test => -
+
@@ -101,7 +102,7 @@ const LabelsClickView: React.FC 0 ? ( <> {labels.map(label => ( - onClickHandle(e, label)}> + onClickHandle(e, label)}> {label.slice(1)} ))} diff --git a/packages/recorder/src/callLog.tsx b/packages/recorder/src/callLog.tsx index c47b1ac323..eafc9f9826 100644 --- a/packages/recorder/src/callLog.tsx +++ b/packages/recorder/src/callLog.tsx @@ -17,7 +17,7 @@ import './callLog.css'; import * as React from 'react'; import type { CallLog } from './recorderTypes'; -import { msToString } from '@web/uiUtils'; +import { clsx, msToString } from '@web/uiUtils'; import { asLocator } from '@isomorphic/locatorGenerators'; import type { Language } from '@isomorphic/locatorGenerators'; @@ -53,9 +53,9 @@ export const CallLogView: React.FC = ({ titlePrefix = callLog.title + '('; titleSuffix = ')'; } - return
+ return
- { + { const newOverrides = new Map(expandOverrides); newOverrides.set(callLog.id, !isExpanded); setExpandOverrides(newOverrides); @@ -64,7 +64,7 @@ export const CallLogView: React.FC = ({ { callLog.params.url ? {callLog.params.url} : undefined } { locator ? {`page.${locator}`} : undefined } { titleSuffix } - + { typeof callLog.duration === 'number' ? — {msToString(callLog.duration)} : undefined}
{ (isExpanded ? callLog.messages : []).map((message, i) => { diff --git a/packages/trace-viewer/src/embedded.tsx b/packages/trace-viewer/src/embedded.tsx index 8f0a09e560..c916d295c1 100644 --- a/packages/trace-viewer/src/embedded.tsx +++ b/packages/trace-viewer/src/embedded.tsx @@ -17,7 +17,6 @@ import '@web/common.css'; import { applyTheme } from '@web/theme'; import '@web/third_party/vscode/codicon.css'; -import React from 'react'; import * as ReactDOM from 'react-dom'; import { EmbeddedWorkbenchLoader } from './ui/embeddedWorkbenchLoader'; diff --git a/packages/trace-viewer/src/index.tsx b/packages/trace-viewer/src/index.tsx index 3f17856254..993b90a23d 100644 --- a/packages/trace-viewer/src/index.tsx +++ b/packages/trace-viewer/src/index.tsx @@ -17,7 +17,6 @@ import '@web/common.css'; import { applyTheme } from '@web/theme'; import '@web/third_party/vscode/codicon.css'; -import React from 'react'; import * as ReactDOM from 'react-dom'; import { WorkbenchLoader } from './ui/workbenchLoader'; diff --git a/packages/trace-viewer/src/ui/callTab.tsx b/packages/trace-viewer/src/ui/callTab.tsx index b7cd7a8581..1ab3b5b46f 100644 --- a/packages/trace-viewer/src/ui/callTab.tsx +++ b/packages/trace-viewer/src/ui/callTab.tsx @@ -16,7 +16,7 @@ import type { SerializedValue } from '@protocol/channels'; import type { ActionTraceEvent } from '@trace/trace'; -import { msToString } from '@web/uiUtils'; +import { clsx, msToString } from '@web/uiUtils'; import * as React from 'react'; import './callTab.css'; import { CopyToClipboard } from './copyToClipboard'; @@ -71,7 +71,7 @@ function renderProperty(property: Property, key: string) { text = `"${text}"`; return (
- {property.name}:{text} + {property.name}:{text} { ['string', 'number', 'object', 'locator'].includes(property.type) && } diff --git a/packages/trace-viewer/src/ui/consoleTab.tsx b/packages/trace-viewer/src/ui/consoleTab.tsx index 882419ae38..a7b8318386 100644 --- a/packages/trace-viewer/src/ui/consoleTab.tsx +++ b/packages/trace-viewer/src/ui/consoleTab.tsx @@ -20,7 +20,7 @@ import './consoleTab.css'; import type * as modelUtil from './modelUtil'; import { ListView } from '@web/components/listView'; import type { Boundaries } from '../geometry'; -import { msToString } from '@web/uiUtils'; +import { clsx, msToString } from '@web/uiUtils'; import { ansi2html } from '@web/ansi2html'; import { PlaceholderPanel } from './placeholderPanel'; @@ -124,8 +124,8 @@ export const ConsoleTab: React.FunctionComponent<{ render={entry => { const timestamp = msToString(entry.timestamp - boundaries.minimum); const timestampElement = {timestamp}; - const errorSuffix = entry.isError ? ' status-error' : entry.isWarning ? ' status-warning' : ' status-none'; - const statusElement = entry.browserMessage || entry.browserError ? : ; + const errorSuffix = entry.isError ? 'status-error' : entry.isWarning ? 'status-warning' : 'status-none'; + const statusElement = entry.browserMessage || entry.browserError ? : ; let locationText: string | undefined; let messageBody: JSX.Element[] | string | undefined; let messageInnerHTML: string | undefined; diff --git a/packages/trace-viewer/src/ui/snapshotTab.tsx b/packages/trace-viewer/src/ui/snapshotTab.tsx index a5ced023fd..798025cbbd 100644 --- a/packages/trace-viewer/src/ui/snapshotTab.tsx +++ b/packages/trace-viewer/src/ui/snapshotTab.tsx @@ -20,7 +20,7 @@ import type { ActionTraceEvent } from '@trace/trace'; import { context, prevInList } from './modelUtil'; import { Toolbar } from '@web/components/toolbar'; import { ToolbarButton } from '@web/components/toolbarButton'; -import { useMeasure } from '@web/uiUtils'; +import { clsx, useMeasure } from '@web/uiUtils'; import { InjectedScript } from '@injected/injectedScript'; import { Recorder } from '@injected/recorder/recorder'; import ConsoleAPI from '@injected/consoleApi'; @@ -209,8 +209,8 @@ export const SnapshotTab: React.FunctionComponent<{ }}>
- - + +
diff --git a/packages/trace-viewer/src/ui/tag.tsx b/packages/trace-viewer/src/ui/tag.tsx index 29ba1546ba..63a5b32217 100644 --- a/packages/trace-viewer/src/ui/tag.tsx +++ b/packages/trace-viewer/src/ui/tag.tsx @@ -14,11 +14,12 @@ * limitations under the License. */ +import { clsx } from '@web/uiUtils'; import './tag.css'; export const TagView: React.FC<{ tag: string, style?: React.CSSProperties, onClick?: (e: React.MouseEvent) => void }> = ({ tag, style, onClick }) => { return { bars.map((bar, index) => { return
= ({
}
-
+
Output
xtermDataSource.clear()}> @@ -444,7 +444,7 @@ export const UIModeView: React.FC<{}> = ({
-
+
{status &&
- +
{testStatusText(status)}
{time ? msToString(time) : ''}
diff --git a/packages/trace-viewer/src/uiMode.tsx b/packages/trace-viewer/src/uiMode.tsx index 8e9e286e00..f2f846a112 100644 --- a/packages/trace-viewer/src/uiMode.tsx +++ b/packages/trace-viewer/src/uiMode.tsx @@ -14,7 +14,6 @@ * limitations under the License. */ -import React from 'react'; import '@web/common.css'; import { applyTheme } from '@web/theme'; import '@web/third_party/vscode/codicon.css'; diff --git a/packages/web/src/components/codeMirrorWrapper.spec.tsx b/packages/web/src/components/codeMirrorWrapper.spec.tsx index 3fb630ae5d..a09a15f9ad 100644 --- a/packages/web/src/components/codeMirrorWrapper.spec.tsx +++ b/packages/web/src/components/codeMirrorWrapper.spec.tsx @@ -14,7 +14,6 @@ * limitations under the License. */ -import React from 'react'; import { expect, test } from '@playwright/experimental-ct-react'; import { CodeMirrorWrapper } from './codeMirrorWrapper'; diff --git a/packages/web/src/components/expandable.spec.tsx b/packages/web/src/components/expandable.spec.tsx index ad69f33aed..9536004bec 100644 --- a/packages/web/src/components/expandable.spec.tsx +++ b/packages/web/src/components/expandable.spec.tsx @@ -14,7 +14,6 @@ * limitations under the License. */ -import React from 'react'; import { expect, test } from '@playwright/experimental-ct-react'; import { Expandable } from './expandable'; diff --git a/packages/web/src/components/expandable.tsx b/packages/web/src/components/expandable.tsx index cb3a9b2254..01e064373f 100644 --- a/packages/web/src/components/expandable.tsx +++ b/packages/web/src/components/expandable.tsx @@ -16,6 +16,7 @@ import * as React from 'react'; import './expandable.css'; +import { clsx } from '../uiUtils'; export const Expandable: React.FunctionComponent> = ({ title, children, setExpanded, expanded, expandOnTitleClick }) => { - return
+ return
expandOnTitleClick && setExpanded(!expanded)}>
!expandOnTitleClick && setExpanded(!expanded)} /> {title} diff --git a/packages/web/src/components/splitView.spec.tsx b/packages/web/src/components/splitView.spec.tsx index 605205059e..fc376bd2ef 100644 --- a/packages/web/src/components/splitView.spec.tsx +++ b/packages/web/src/components/splitView.spec.tsx @@ -14,7 +14,6 @@ * limitations under the License. */ -import React from 'react'; import { expect, test } from '@playwright/experimental-ct-react'; import { SplitView } from './splitView'; diff --git a/packages/web/src/components/splitView.tsx b/packages/web/src/components/splitView.tsx index 2db241dfc4..4c4513f235 100644 --- a/packages/web/src/components/splitView.tsx +++ b/packages/web/src/components/splitView.tsx @@ -14,7 +14,7 @@ limitations under the License. */ -import { useMeasure, useSetting } from '../uiUtils'; +import { clsx, useMeasure, useSetting } from '../uiUtils'; import './splitView.css'; import * as React from 'react'; @@ -75,7 +75,7 @@ export const SplitView: React.FC> = ({ resizerStyle = { right: resizing ? 0 : size - 4, left: resizing ? 0 : undefined, width: resizing ? 'initial' : 8 }; } - return
+ return
{childrenArray[0]}
{ !sidebarHidden &&
{childrenArray[1]}
} { !sidebarHidden &&
void }> = ({ id, title, count, errorCount, selected, onSelect }) => { - return
onSelect(id)} title={title} key={id}> diff --git a/packages/web/src/components/toolbar.tsx b/packages/web/src/components/toolbar.tsx index 023fad0232..c7dd6843d9 100644 --- a/packages/web/src/components/toolbar.tsx +++ b/packages/web/src/components/toolbar.tsx @@ -14,6 +14,7 @@ limitations under the License. */ +import { clsx } from '@web/uiUtils'; import './toolbar.css'; import * as React from 'react'; @@ -31,5 +32,5 @@ export const Toolbar: React.FC> = ({ className, onClick, }) => { - return
{children}
; + return
{children}
; }; diff --git a/packages/web/src/shared/imageDiffView.spec.tsx b/packages/web/src/shared/imageDiffView.spec.tsx index 79d256096d..4fefaa1083 100644 --- a/packages/web/src/shared/imageDiffView.spec.tsx +++ b/packages/web/src/shared/imageDiffView.spec.tsx @@ -14,7 +14,6 @@ * limitations under the License. */ -import React from 'react'; import { test, expect } from '@playwright/experimental-ct-react'; import type { ImageDiff } from './imageDiffView'; import { ImageDiffView } from './imageDiffView'; diff --git a/packages/web/src/uiUtils.ts b/packages/web/src/uiUtils.ts index 4499ed81f5..6abe445749 100644 --- a/packages/web/src/uiUtils.ts +++ b/packages/web/src/uiUtils.ts @@ -191,3 +191,8 @@ export class Settings { } export const settings = new Settings(); + +// inspired by https://www.npmjs.com/package/clsx +export function clsx(...classes: (string | undefined | false)[]) { + return classes.filter(Boolean).join(' '); +} \ No newline at end of file