From 253a2f9a9c15f62c12640e6a355df5129a5bd1c4 Mon Sep 17 00:00:00 2001
From: Max Schmitt <max@schmitt.mx>
Date: Mon, 25 Mar 2024 19:48:20 +0100
Subject: [PATCH] chore: address UI Mode keyboard shortcut feedback (#30088)

Signed-off-by: Max Schmitt <max@schmitt.mx>
Co-authored-by: Dmitry Gozman <dgozman@gmail.com>
---
 packages/trace-viewer/src/ui/uiModeView.tsx   | 17 +++++---
 .../ui-mode-test-shortcut.spec.ts             | 41 +++++++++++++++----
 2 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/packages/trace-viewer/src/ui/uiModeView.tsx b/packages/trace-viewer/src/ui/uiModeView.tsx
index fe9d7005cb..2cc76ff8fd 100644
--- a/packages/trace-viewer/src/ui/uiModeView.tsx
+++ b/packages/trace-viewer/src/ui/uiModeView.tsx
@@ -63,6 +63,8 @@ const queryParams = {
   reporters: searchParams.has('reporter') ? searchParams.getAll('reporter') : undefined,
 };
 
+const isMac = navigator.platform === 'MacIntel';
+
 export const UIModeView: React.FC<{}> = ({
 }) => {
   const [filterText, setFilterText] = React.useState<string>('');
@@ -319,19 +321,22 @@ export const UIModeView: React.FC<{}> = ({
     if (!testServerConnection)
       return;
     const onShortcutEvent = (e: KeyboardEvent) => {
-      if (e.code === 'F6') {
+      if (e.code === 'Backquote' && e.ctrlKey) {
+        e.preventDefault();
+        setIsShowingOutput(!isShowingOutput);
+      } else if (e.code === 'F5' && e.shiftKey) {
         e.preventDefault();
         testServerConnection?.stopTestsNoReply({});
       } else if (e.code === 'F5') {
         e.preventDefault();
-        reloadTests();
+        runTests('bounce-if-busy', visibleTestIds);
       }
     };
     addEventListener('keydown', onShortcutEvent);
     return () => {
       removeEventListener('keydown', onShortcutEvent);
     };
-  }, [runTests, reloadTests, testServerConnection]);
+  }, [runTests, reloadTests, testServerConnection, visibleTestIds, isShowingOutput]);
 
   const isRunningTest = !!runningState;
   const dialogRef = React.useRef<HTMLDialogElement>(null);
@@ -392,7 +397,7 @@ export const UIModeView: React.FC<{}> = ({
           <div className='section-title'>Playwright</div>
           <ToolbarButton icon='color-mode' title='Toggle color mode' onClick={() => toggleTheme()} />
           <ToolbarButton icon='refresh' title='Reload' onClick={() => reloadTests()} disabled={isRunningTest || isLoading}></ToolbarButton>
-          <ToolbarButton icon='terminal' title='Toggle output' toggled={isShowingOutput} onClick={() => { setIsShowingOutput(!isShowingOutput); }} />
+          <ToolbarButton icon='terminal' title={'Toggle output — ' + (isMac ? '⌃`' : 'Ctrl + `')} toggled={isShowingOutput} onClick={() => { setIsShowingOutput(!isShowingOutput); }} />
           {!hasBrowsers && <ToolbarButton icon='lightbulb-autofix' style={{ color: 'var(--vscode-list-warningForeground)' }} title='Playwright browsers are missing' onClick={openInstallDialog} />}
         </Toolbar>
         <FiltersView
@@ -412,8 +417,8 @@ export const UIModeView: React.FC<{}> = ({
           {isRunningTest && progress && <div data-testid='status-line' className='status-line'>
             <div>Running {progress.passed}/{runningState.testIds.size} passed ({(progress.passed / runningState.testIds.size) * 100 | 0}%)</div>
           </div>}
-          <ToolbarButton icon='play' title='Run all' onClick={() => runTests('bounce-if-busy', visibleTestIds)} disabled={isRunningTest || isLoading}></ToolbarButton>
-          <ToolbarButton icon='debug-stop' title='Stop' onClick={() => testServerConnection?.stopTests({})} disabled={!isRunningTest || isLoading}></ToolbarButton>
+          <ToolbarButton icon='play' title='Run all — F5' onClick={() => runTests('bounce-if-busy', visibleTestIds)} disabled={isRunningTest || isLoading}></ToolbarButton>
+          <ToolbarButton icon='debug-stop' title={'Stop — ' + (isMac ? '⇧F5' : 'Shift + F5')} onClick={() => testServerConnection?.stopTests({})} disabled={!isRunningTest || isLoading}></ToolbarButton>
           <ToolbarButton icon='eye' title='Watch all' toggled={watchAll} onClick={() => {
             setWatchedTreeIds({ value: new Set() });
             setWatchAll(!watchAll);
diff --git a/tests/playwright-test/ui-mode-test-shortcut.spec.ts b/tests/playwright-test/ui-mode-test-shortcut.spec.ts
index 8768ccbca0..586751880e 100644
--- a/tests/playwright-test/ui-mode-test-shortcut.spec.ts
+++ b/tests/playwright-test/ui-mode-test-shortcut.spec.ts
@@ -28,7 +28,29 @@ const basicTestTree = {
   `
 };
 
-test('should stop on F6', async ({ runUITest }) => {
+test('should run tests', async ({ runUITest }) => {
+  const { page } = await runUITest(basicTestTree);
+
+  await expect(page.getByTitle('Run all')).toBeEnabled();
+  await expect(page.getByTitle('Stop')).toBeDisabled();
+
+  await page.getByPlaceholder('Filter (e.g. text, @tag)').fill('test 3');
+  await page.keyboard.press('F5');
+
+  await expect(page.getByTestId('status-line')).toHaveText('1/1 passed (100%)');
+  await page.getByPlaceholder('Filter (e.g. text, @tag)').fill('');
+
+  // Only the filtered test was run.
+  await expect.poll(dumpTestTree(page)).toBe(`
+    ▼ ◯ a.test.ts
+        ◯ test 0
+        ◯ test 1
+        ◯ test 2
+        ✅ test 3
+  `);
+});
+
+test('should stop tests', async ({ runUITest }) => {
   const { page } = await runUITest(basicTestTree);
 
   await expect(page.getByTitle('Run all')).toBeEnabled();
@@ -47,7 +69,7 @@ test('should stop on F6', async ({ runUITest }) => {
   await expect(page.getByTitle('Run all')).toBeDisabled();
   await expect(page.getByTitle('Stop')).toBeEnabled();
 
-  await page.keyboard.press('F6');
+  await page.keyboard.press('Shift+F5');
 
   await expect.poll(dumpTestTree(page)).toBe(`
     ▼ ◯ a.test.ts
@@ -58,12 +80,15 @@ test('should stop on F6', async ({ runUITest }) => {
   `);
 });
 
-test('should reload on F5', async ({ runUITest }) => {
+test('should toggle Terminal', async ({ runUITest }) => {
   const { page } = await runUITest(basicTestTree);
 
-  await page.getByTitle('Run all').click();
-  await expect(page.getByTestId('status-line')).toHaveText('Running 1/4 passed (25%)');
+  await expect(page.getByTitle('Run all')).toBeEnabled();
+  await expect(page.getByTitle('Stop')).toBeDisabled();
 
-  await page.keyboard.press('F5');
-  await expect(page.getByTestId('status-line')).toBeHidden();
-});
\ No newline at end of file
+  await expect(page.getByTestId('output')).toBeHidden();
+
+  await page.keyboard.press(process.platform === 'darwin' ? 'Control+Backquote' : 'Control+Shift+Backquote');
+
+  await expect(page.getByTestId('output')).toBeVisible();
+});