From 08422f0651763f4d9915644d3783e201a7336356 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 21 Mar 2023 18:20:48 -0700 Subject: [PATCH] cherry-pick(#21856): chore: pack codemirror on resize --- packages/trace-viewer/src/ui/filmStrip.tsx | 3 +- packages/trace-viewer/src/ui/helpers.tsx | 55 ------------------- packages/trace-viewer/src/ui/snapshotTab.tsx | 3 +- packages/trace-viewer/src/ui/sourceTab.tsx | 2 +- packages/trace-viewer/src/ui/timeline.tsx | 3 +- packages/web/src/components/DEPS.list | 1 + .../web/src/components/codeMirrorWrapper.tsx | 8 ++- packages/web/src/components/xtermWrapper.tsx | 36 ++++++------ packages/web/src/uiUtils.ts | 38 +++++++++++++ ...rs.spec.ts => ui-mode-test-output.spec.ts} | 28 +++++++++- 10 files changed, 95 insertions(+), 82 deletions(-) delete mode 100644 packages/trace-viewer/src/ui/helpers.tsx rename tests/playwright-test/{ui-mode-load-errors.spec.ts => ui-mode-test-output.spec.ts} (53%) diff --git a/packages/trace-viewer/src/ui/filmStrip.tsx b/packages/trace-viewer/src/ui/filmStrip.tsx index 62349409d86aa..c5db2a57b89b8 100644 --- a/packages/trace-viewer/src/ui/filmStrip.tsx +++ b/packages/trace-viewer/src/ui/filmStrip.tsx @@ -17,8 +17,7 @@ import './filmStrip.css'; import type { Boundaries, Size } from '../geometry'; import * as React from 'react'; -import { useMeasure } from './helpers'; -import { upperBound } from '@web/uiUtils'; +import { useMeasure, upperBound } from '@web/uiUtils'; import type { PageEntry } from '../entries'; import type { MultiTraceModel } from './modelUtil'; diff --git a/packages/trace-viewer/src/ui/helpers.tsx b/packages/trace-viewer/src/ui/helpers.tsx deleted file mode 100644 index a8302c0efa121..0000000000000 --- a/packages/trace-viewer/src/ui/helpers.tsx +++ /dev/null @@ -1,55 +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 * as React from 'react'; - -// Recalculates the value when dependencies change. -export function useAsyncMemo(fn: () => Promise, deps: React.DependencyList, initialValue: T, resetValue?: T) { - const [value, setValue] = React.useState(initialValue); - React.useEffect(() => { - let canceled = false; - if (resetValue !== undefined) - setValue(resetValue); - fn().then(value => { - if (!canceled) - setValue(value); - }); - return () => { - canceled = true; - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, deps); - return value; -} - -// Tracks the element size and returns it's contentRect (always has x=0, y=0). -export function useMeasure() { - const ref = React.useRef(null); - const [measure, setMeasure] = React.useState(new DOMRect(0, 0, 10, 10)); - React.useLayoutEffect(() => { - const target = ref.current; - if (!target) - return; - const resizeObserver = new ResizeObserver((entries: any) => { - const entry = entries[entries.length - 1]; - if (entry && entry.contentRect) - setMeasure(entry.contentRect); - }); - resizeObserver.observe(target); - return () => resizeObserver.unobserve(target); - }, [ref]); - return [measure, ref] as const; -} diff --git a/packages/trace-viewer/src/ui/snapshotTab.tsx b/packages/trace-viewer/src/ui/snapshotTab.tsx index 24df7f83d9b02..732b0dbb8b437 100644 --- a/packages/trace-viewer/src/ui/snapshotTab.tsx +++ b/packages/trace-viewer/src/ui/snapshotTab.tsx @@ -16,13 +16,12 @@ import './snapshotTab.css'; import * as React from 'react'; -import { useMeasure } from './helpers'; import type { ActionTraceEvent } from '@trace/trace'; import { context, prevInList } from './modelUtil'; import { CodeMirrorWrapper } from '@web/components/codeMirrorWrapper'; import { Toolbar } from '@web/components/toolbar'; import { ToolbarButton } from '@web/components/toolbarButton'; -import { copy } from '@web/uiUtils'; +import { copy, useMeasure } from '@web/uiUtils'; import { InjectedScript } from '@injected/injectedScript'; import { Recorder } from '@injected/recorder'; import { asLocator } from '@isomorphic/locatorGenerators'; diff --git a/packages/trace-viewer/src/ui/sourceTab.tsx b/packages/trace-viewer/src/ui/sourceTab.tsx index f0c7fb8600c46..e6a93ba840213 100644 --- a/packages/trace-viewer/src/ui/sourceTab.tsx +++ b/packages/trace-viewer/src/ui/sourceTab.tsx @@ -17,7 +17,7 @@ import type { ActionTraceEvent } from '@trace/trace'; import { SplitView } from '@web/components/splitView'; import * as React from 'react'; -import { useAsyncMemo } from './helpers'; +import { useAsyncMemo } from '@web/uiUtils'; import './sourceTab.css'; import { StackTraceView } from './stackTrace'; import { CodeMirrorWrapper } from '@web/components/codeMirrorWrapper'; diff --git a/packages/trace-viewer/src/ui/timeline.tsx b/packages/trace-viewer/src/ui/timeline.tsx index 797e2bb3e3e29..f756229533c48 100644 --- a/packages/trace-viewer/src/ui/timeline.tsx +++ b/packages/trace-viewer/src/ui/timeline.tsx @@ -16,11 +16,10 @@ */ import type { ActionTraceEvent, EventTraceEvent } from '@trace/trace'; -import { msToString } from '@web/uiUtils'; +import { msToString, useMeasure } from '@web/uiUtils'; import * as React from 'react'; import type { Boundaries } from '../geometry'; import { FilmStrip } from './filmStrip'; -import { useMeasure } from './helpers'; import type { MultiTraceModel } from './modelUtil'; import './timeline.css'; diff --git a/packages/web/src/components/DEPS.list b/packages/web/src/components/DEPS.list index fd95d5136651b..69cd00dc55fa7 100644 --- a/packages/web/src/components/DEPS.list +++ b/packages/web/src/components/DEPS.list @@ -1,6 +1,7 @@ [*] ../theme.ts ../third_party/vscode/codicon.css +../uiUtils.ts [expandable.spec.tsx] *** diff --git a/packages/web/src/components/codeMirrorWrapper.tsx b/packages/web/src/components/codeMirrorWrapper.tsx index e51910f587763..5ff1ab69f3bce 100644 --- a/packages/web/src/components/codeMirrorWrapper.tsx +++ b/packages/web/src/components/codeMirrorWrapper.tsx @@ -18,6 +18,7 @@ import './codeMirrorWrapper.css'; import * as React from 'react'; import type { CodeMirror } from './codeMirrorModule'; import { ansi2htmlMarkup } from './errorMessage'; +import { useMeasure } from '../uiUtils'; export type SourceHighlight = { line: number; @@ -51,7 +52,7 @@ export const CodeMirrorWrapper: React.FC = ({ wrapLines, onChange, }) => { - const codemirrorElement = React.useRef(null); + const [measure, codemirrorElement] = useMeasure(); const [modulePromise] = React.useState>(import('./codeMirrorModule').then(m => m.default)); const codemirrorRef = React.useRef<{ cm: CodeMirror.Editor, highlight?: SourceHighlight[], widgets?: CodeMirror.LineWidget[] } | null>(null); const [codemirror, setCodemirror] = React.useState(); @@ -98,6 +99,11 @@ export const CodeMirrorWrapper: React.FC = ({ })(); }, [modulePromise, codemirror, codemirrorElement, language, lineNumbers, wrapLines, readOnly]); + React.useEffect(() => { + if (codemirrorRef.current) + codemirrorRef.current.cm.setSize(measure.width, measure.height); + }, [measure]); + React.useEffect(() => { if (!codemirror) return; diff --git a/packages/web/src/components/xtermWrapper.tsx b/packages/web/src/components/xtermWrapper.tsx index d5ccab5b010d1..70a3e47114360 100644 --- a/packages/web/src/components/xtermWrapper.tsx +++ b/packages/web/src/components/xtermWrapper.tsx @@ -20,6 +20,7 @@ import type { ITheme, Terminal } from 'xterm'; import type { FitAddon } from 'xterm-addon-fit'; import type { XtermModule } from './xtermModule'; import { currentTheme, addThemeListener, removeThemeListener } from '@web/theme'; +import { useMeasure } from '@web/uiUtils'; export type XtermDataSource = { pending: (string | Uint8Array)[]; @@ -31,10 +32,10 @@ export type XtermDataSource = { export const XtermWrapper: React.FC<{ source: XtermDataSource }> = ({ source, }) => { - const xtermElement = React.useRef(null); + const [measure, xtermElement] = useMeasure(); const [theme, setTheme] = React.useState(currentTheme()); const [modulePromise] = React.useState>(import('./xtermModule').then(m => m.default)); - const terminal = React.useRef<{ terminal: Terminal, fitAddon: FitAddon }| null>(null); + const terminal = React.useRef<{ terminal: Terminal, fitAddon: FitAddon } | null>(null); React.useEffect(() => { addThemeListener(setTheme); @@ -44,7 +45,6 @@ export const XtermWrapper: React.FC<{ source: XtermDataSource }> = ({ React.useEffect(() => { const oldSourceWrite = source.write; const oldSourceClear = source.clear; - let resizeObserver: ResizeObserver | undefined; (async () => { // Always load the module first. @@ -53,15 +53,19 @@ export const XtermWrapper: React.FC<{ source: XtermDataSource }> = ({ if (!element) return; - if (terminal.current && terminal) + const terminalTheme = theme === 'dark-mode' ? darkTheme : lightTheme; + if (terminal.current && terminal.current.terminal.options.theme === terminalTheme) return; + if (terminal.current) + element.textContent = ''; + const newTerminal = new Terminal({ convertEol: true, fontSize: 13, scrollback: 10000, fontFamily: 'var(--vscode-editor-font-family)', - theme: theme === 'dark-mode' ? darkTheme : lightTheme + theme: terminalTheme, }); const fitAddon = new FitAddon(); @@ -79,27 +83,23 @@ export const XtermWrapper: React.FC<{ source: XtermDataSource }> = ({ newTerminal.open(element); fitAddon.fit(); terminal.current = { terminal: newTerminal, fitAddon }; - resizeObserver = new ResizeObserver(() => { - // Fit reads data from the terminal itself, which updates lazily, probably on some timer - // or mutation observer. Work around it. - setTimeout(() => { - fitAddon.fit(); - source.resize(newTerminal.cols, newTerminal.rows); - }, 100); - }); - resizeObserver.observe(element); })(); return () => { source.clear = oldSourceClear; source.write = oldSourceWrite; - resizeObserver?.disconnect(); }; }, [modulePromise, terminal, xtermElement, source, theme]); React.useEffect(() => { - if (terminal.current) - terminal.current.terminal.options.theme = theme === 'dark-mode' ? darkTheme : lightTheme; - }, [theme]); + // Fit reads data from the terminal itself, which updates lazily, probably on some timer + // or mutation observer. Work around it. + setTimeout(() => { + if (!terminal.current) + return; + terminal.current.fitAddon.fit(); + source.resize(terminal.current.terminal.cols, terminal.current.terminal.rows); + }, 250); + }, [measure, source]); return
; }; diff --git a/packages/web/src/uiUtils.ts b/packages/web/src/uiUtils.ts index c978f4161df47..b092ad06141a8 100644 --- a/packages/web/src/uiUtils.ts +++ b/packages/web/src/uiUtils.ts @@ -16,6 +16,44 @@ import React from 'react'; +// Recalculates the value when dependencies change. +export function useAsyncMemo(fn: () => Promise, deps: React.DependencyList, initialValue: T, resetValue?: T) { + const [value, setValue] = React.useState(initialValue); + React.useEffect(() => { + let canceled = false; + if (resetValue !== undefined) + setValue(resetValue); + fn().then(value => { + if (!canceled) + setValue(value); + }); + return () => { + canceled = true; + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, deps); + return value; +} + +// Tracks the element size and returns it's contentRect (always has x=0, y=0). +export function useMeasure() { + const ref = React.useRef(null); + const [measure, setMeasure] = React.useState(new DOMRect(0, 0, 10, 10)); + React.useLayoutEffect(() => { + const target = ref.current; + if (!target) + return; + const resizeObserver = new ResizeObserver((entries: any) => { + const entry = entries[entries.length - 1]; + if (entry && entry.contentRect) + setMeasure(entry.contentRect); + }); + resizeObserver.observe(target); + return () => resizeObserver.disconnect(); + }, [ref]); + return [measure, ref] as const; +} + export function msToString(ms: number): string { if (!isFinite(ms)) return '-'; diff --git a/tests/playwright-test/ui-mode-load-errors.spec.ts b/tests/playwright-test/ui-mode-test-output.spec.ts similarity index 53% rename from tests/playwright-test/ui-mode-load-errors.spec.ts rename to tests/playwright-test/ui-mode-test-output.spec.ts index e3608581db9fb..c606110950246 100644 --- a/tests/playwright-test/ui-mode-load-errors.spec.ts +++ b/tests/playwright-test/ui-mode-test-output.spec.ts @@ -18,7 +18,7 @@ import { test, expect } from './ui-mode-fixtures'; test.describe.configure({ mode: 'parallel' }); -test('should list tests', async ({ runUITest }) => { +test('should print load errors', async ({ runUITest }) => { const page = await runUITest({ 'a.test.ts': ` import { test, expect } from '@playwright/test'; @@ -30,3 +30,29 @@ test('should list tests', async ({ runUITest }) => { await page.getByTitle('Toggle output').click(); await expect(page.getByTestId('output')).toContainText(`Unexpected reserved word 'await'`); }); + +test('should work after theme switch', async ({ runUITest, writeFiles }) => { + const page = await runUITest({ + 'a.test.ts': ` + import { test, expect } from '@playwright/test'; + test('syntax error', async () => { + console.log('Hello world 1'); + }); + `, + }); + await page.getByTitle('Toggle output').click(); + await page.getByTitle('Run all').click(); + await expect(page.getByTestId('output')).toContainText(`Hello world 1`); + + await page.getByTitle('Toggle color mode').click(); + writeFiles({ + 'a.test.ts': ` + import { test, expect } from '@playwright/test'; + test('syntax error', async () => { + console.log('Hello world 2'); + }); + `, + }); + await page.getByTitle('Run all').click(); + await expect(page.getByTestId('output')).toContainText(`Hello world 2`); +});