From e624c9b12448f185e3fe395d92d8c529b9db99b5 Mon Sep 17 00:00:00 2001 From: Jess Frazelle Date: Thu, 22 Aug 2024 15:00:20 -0700 Subject: [PATCH 1/6] fix failing tests from chalmers pr (#3619) * fix failing tests from chalmers pr Signed-off-by: Jess Frazelle * fix memory pane Signed-off-by: Jess Frazelle --------- Signed-off-by: Jess Frazelle --- .../ModelingSidebar/ModelingPanes/MemoryPane.tsx | 8 ++++---- src/lang/wasm.ts | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/components/ModelingSidebar/ModelingPanes/MemoryPane.tsx b/src/components/ModelingSidebar/ModelingPanes/MemoryPane.tsx index efb52e533..df1d8071b 100644 --- a/src/components/ModelingSidebar/ModelingPanes/MemoryPane.tsx +++ b/src/components/ModelingSidebar/ModelingPanes/MemoryPane.tsx @@ -90,12 +90,12 @@ export const processMemory = (programMemory: ProgramMemory) => { for (const [key, val] of programMemory?.visibleEntries()) { if (typeof val.value !== 'function') { const sg = sketchGroupFromKclValue(val, null) - if (!err(sg)) { - processedMemory[key] = sg.value.map(({ __geoMeta, ...rest }: Path) => { + if (val.type === 'ExtrudeGroup') { + processedMemory[key] = val.value.map(({ ...rest }: ExtrudeSurface) => { return rest }) - } else if (val.type === 'ExtrudeGroup') { - processedMemory[key] = val.value.map(({ ...rest }: ExtrudeSurface) => { + } else if (!err(sg)) { + processedMemory[key] = sg.value.map(({ __geoMeta, ...rest }: Path) => { return rest }) } else if ((val.type as any) === 'Function') { diff --git a/src/lang/wasm.ts b/src/lang/wasm.ts index 66c64fcfe..5ca4ea5bd 100644 --- a/src/lang/wasm.ts +++ b/src/lang/wasm.ts @@ -338,13 +338,16 @@ export function sketchGroupFromKclValue( varName: string | null ): SketchGroup | Error { if (obj?.value?.type === 'SketchGroup') return obj.value + if (obj?.value?.type === 'ExtrudeGroup') return obj.value.sketchGroup + if (obj?.type === 'ExtrudeGroup') return obj.sketchGroup if (!varName) { varName = 'a KCL value' } const actualType = obj?.value?.type ?? obj?.type if (actualType) { + console.log(obj) return new Error( - `Expected ${varName} to be a sketchGroup, but it was ${actualType} instead.` + `Expected ${varName} to be a sketchGroup or extrudeGroup, but it was ${actualType} instead.` ) } else { return new Error(`Expected ${varName} to be a sketchGroup, but it wasn't.`) From acbe92d717fc23eaf7d8f85f972234693bbafa48 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Thu, 22 Aug 2024 19:13:27 -0400 Subject: [PATCH 2/6] Fix keyboard shortcuts to use Control on Windows and Linux (#3620) * Fix keyboard shortcuts to use Control instead of Meta on Windows and Linux * Convert more tests to use Playwright built-in --- e2e/playwright/command-bar-tests.spec.ts | 6 ++-- e2e/playwright/copilot-ghost-test.spec.ts | 23 ++++++-------- e2e/playwright/editor-tests.spec.ts | 13 +++----- e2e/playwright/projects.spec.ts | 13 +++----- e2e/playwright/test-utils.ts | 11 ++----- e2e/playwright/testing-settings.spec.ts | 4 +-- e2e/playwright/text-to-cad-tests.spec.ts | 22 ++++++-------- e2e/playwright/various.spec.ts | 19 +++--------- src/Router.tsx | 2 +- src/components/FileTree.tsx | 4 +-- src/hooks/usePlatform.ts | 37 ++--------------------- src/lib/hotkeyWrapper.ts | 2 +- src/lib/settings/initialKeybindings.ts | 12 ++++++-- src/lib/utils.ts | 36 ++++++++++++++++++++++ 14 files changed, 92 insertions(+), 112 deletions(-) diff --git a/e2e/playwright/command-bar-tests.spec.ts b/e2e/playwright/command-bar-tests.spec.ts index 55b5a5eee..3d4a6ca7e 100644 --- a/e2e/playwright/command-bar-tests.spec.ts +++ b/e2e/playwright/command-bar-tests.spec.ts @@ -124,7 +124,7 @@ const extrude001 = extrude(-10, sketch001)` await expect(cmdSearchBar).not.toBeVisible() // Now try the same, but with the keyboard shortcut, check focus - await page.keyboard.press('Meta+K') + await page.keyboard.press('ControlOrMeta+K') await expect(cmdSearchBar).toBeVisible() await expect(cmdSearchBar).toBeFocused() @@ -185,7 +185,7 @@ const extrude001 = extrude(-10, sketch001)` await page.locator('.cm-content').click() // Now try the same, but with the keyboard shortcut, check focus - await page.keyboard.press('Meta+K') + await page.keyboard.press('ControlOrMeta+K') let cmdSearchBar = page.getByPlaceholder('Search commands') await expect(cmdSearchBar).toBeVisible() @@ -250,7 +250,7 @@ const extrude001 = extrude(-10, sketch001)` await page.getByRole('button', { name: 'Extrude' }).isEnabled() let cmdSearchBar = page.getByPlaceholder('Search commands') - await page.keyboard.press('Meta+K') + await page.keyboard.press('ControlOrMeta+K') await expect(cmdSearchBar).toBeVisible() // Search for extrude command and choose it diff --git a/e2e/playwright/copilot-ghost-test.spec.ts b/e2e/playwright/copilot-ghost-test.spec.ts index 8520dfaa1..2ba2ac407 100644 --- a/e2e/playwright/copilot-ghost-test.spec.ts +++ b/e2e/playwright/copilot-ghost-test.spec.ts @@ -332,7 +332,6 @@ test.describe('Copilot ghost text', () => { await page.setViewportSize({ width: 1200, height: 500 }) await u.waitForAuthSkipAppStart() - const CtrlKey = process.platform === 'darwin' ? 'Meta' : 'Control' await u.codeLocator.click() await expect(page.locator('.cm-content')).toHaveText(``) @@ -349,10 +348,10 @@ test.describe('Copilot ghost text', () => { ) // Going elsewhere in the code should hide the ghost text. - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.down('Shift') await page.keyboard.press('KeyZ') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') await page.keyboard.up('Shift') await expect(page.locator('.cm-ghostText').first()).not.toBeVisible() @@ -368,8 +367,6 @@ test.describe('Copilot ghost text', () => { await u.waitForAuthSkipAppStart() - const CtrlKey = process.platform === 'darwin' ? 'Meta' : 'Control' - await page.waitForTimeout(800) await u.codeLocator.click() await expect(page.locator('.cm-content')).toHaveText(``) @@ -382,17 +379,17 @@ test.describe('Copilot ghost text', () => { await page.waitForTimeout(800) // Ctrl+z - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('KeyZ') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') await expect(page.locator('.cm-content')).toHaveText(``) // Ctrl+shift+z - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.down('Shift') await page.keyboard.press('KeyZ') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') await page.keyboard.up('Shift') await expect(page.locator('.cm-content')).toHaveText(`{thing: "blah"}`) @@ -411,14 +408,14 @@ test.describe('Copilot ghost text', () => { ) // Once for the enter. - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('KeyZ') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') // Once for the text. - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('KeyZ') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') await expect(page.locator('.cm-ghostText').first()).not.toBeVisible() diff --git a/e2e/playwright/editor-tests.spec.ts b/e2e/playwright/editor-tests.spec.ts index fda4cc36d..80c5cbb7f 100644 --- a/e2e/playwright/editor-tests.spec.ts +++ b/e2e/playwright/editor-tests.spec.ts @@ -16,7 +16,6 @@ test.describe('Editor tests', () => { await page.setViewportSize({ width: 1000, height: 500 }) await u.waitForAuthSkipAppStart() - const CtrlKey = process.platform === 'darwin' ? 'Meta' : 'Control' // check no error to begin with await expect(page.locator('.cm-lint-marker-error')).not.toBeVisible() @@ -29,9 +28,9 @@ test.describe('Editor tests', () => { |> line([-20, 0], %) |> close(%)`) - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('/') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') await expect(page.locator('.cm-content')) .toHaveText(`const sketch001 = startSketchOn('XY') @@ -42,9 +41,9 @@ test.describe('Editor tests', () => { // |> close(%)`) // uncomment the code - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('/') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') await expect(page.locator('.cm-content')) .toHaveText(`const sketch001 = startSketchOn('XY') @@ -148,9 +147,7 @@ test.describe('Editor tests', () => { // Delete all the code. await page.locator('.cm-content').click() // Select all - await page.keyboard.press('Control+A') - await page.keyboard.press('Backspace') - await page.keyboard.press('Meta+A') + await page.keyboard.press('ControlOrMeta+A') await page.keyboard.press('Backspace') await expect(page.locator('.cm-content')).toHaveText(``) diff --git a/e2e/playwright/projects.spec.ts b/e2e/playwright/projects.spec.ts index 7cb5bd5a9..1fcc86867 100644 --- a/e2e/playwright/projects.spec.ts +++ b/e2e/playwright/projects.spec.ts @@ -1233,18 +1233,13 @@ test( await page.getByText('mike_stress_test').click() - const modifier = - process.platform === 'win32' || process.platform === 'linux' - ? 'Control' - : 'Meta' - await test.step('select all in code editor, check its length', async () => { await u.codeLocator.click() // expect u.codeLocator to have some text await expect(u.codeLocator).toContainText('line(') - await page.keyboard.down(modifier) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('KeyA') - await page.keyboard.up(modifier) + await page.keyboard.up('ControlOrMeta') // check the length of the selected text const selectedText = await page.evaluate(() => { @@ -1260,9 +1255,9 @@ test( await test.step('delete all the text, select again and verify there are no characters left', async () => { await page.keyboard.press('Backspace') - await page.keyboard.down(modifier) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('KeyA') - await page.keyboard.up(modifier) + await page.keyboard.up('ControlOrMeta') // check the length of the selected text const selectedText = await page.evaluate(() => { diff --git a/e2e/playwright/test-utils.ts b/e2e/playwright/test-utils.ts index 7e8e8778e..39617874e 100644 --- a/e2e/playwright/test-utils.ts +++ b/e2e/playwright/test-utils.ts @@ -9,7 +9,6 @@ import { test, } from '@playwright/test' import { EngineCommand } from 'lang/std/artifactGraph' -import os from 'os' import fsp from 'fs/promises' import fsSync from 'fs' import { join } from 'path' @@ -78,11 +77,10 @@ async function waitForPageLoad(page: Page) { } async function removeCurrentCode(page: Page) { - const hotkey = process.platform === 'darwin' ? 'Meta' : 'Control' await page.locator('.cm-content').click() - await page.keyboard.down(hotkey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('a') - await page.keyboard.up(hotkey) + await page.keyboard.up('ControlOrMeta') await page.keyboard.press('Backspace') await expect(page.locator('.cm-content')).toHaveText('') } @@ -745,11 +743,6 @@ export const doExport = async ( } } -/** - * Gets the appropriate modifier key for the platform. - */ -export const metaModifier = os.platform() === 'darwin' ? 'Meta' : 'Control' - export async function tearDown(page: Page, testInfo: TestInfo) { if (testInfo.status === 'skipped') return if (testInfo.status === 'failed') return diff --git a/e2e/playwright/testing-settings.spec.ts b/e2e/playwright/testing-settings.spec.ts index 57669ac27..bca8a4e97 100644 --- a/e2e/playwright/testing-settings.spec.ts +++ b/e2e/playwright/testing-settings.spec.ts @@ -72,7 +72,7 @@ test.describe('Testing settings', () => { const inputLocator = page.locator('input[name="modeling-showDebugPanel"]') // Open the settings modal with the browser keyboard shortcut - await page.keyboard.press('Meta+Shift+,') + await page.keyboard.press('ControlOrMeta+Shift+,') await expect(headingLocator).toBeVisible() await page.locator('#showDebugPanel').getByText('OffOn').click() @@ -82,7 +82,7 @@ test.describe('Testing settings', () => { await test.step('Open settings with keyboard shortcut', async () => { await page.getByTestId('settings-close-button').click() await page.locator('.cm-content').click() - await page.keyboard.press('Meta+Shift+,') + await page.keyboard.press('ControlOrMeta+Shift+,') await expect(headingLocator).toBeVisible() }) diff --git a/e2e/playwright/text-to-cad-tests.spec.ts b/e2e/playwright/text-to-cad-tests.spec.ts index 640acc624..162d8bf05 100644 --- a/e2e/playwright/text-to-cad-tests.spec.ts +++ b/e2e/playwright/text-to-cad-tests.spec.ts @@ -9,8 +9,6 @@ test.afterEach(async ({ page }, testInfo) => { await tearDown(page, testInfo) }) -const CtrlKey = process.platform === 'darwin' ? 'Meta' : 'Control' - test.describe('Text-to-CAD tests', () => { test('basic lego happy case', async ({ page }) => { const u = await getUtils(page) @@ -298,9 +296,9 @@ test.describe('Text-to-CAD tests', () => { await expect(page.locator('textarea')).toContainText(badPrompt) // Select all and start a new prompt. - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('KeyA') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') await page.keyboard.type('a 2x4 lego') // Submit the new prompt. @@ -520,9 +518,9 @@ test.describe('Text-to-CAD tests', () => { await page.locator('.cm-content').click({ position: { x: 10, y: 10 } }) // Paste the code. - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('KeyV') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') // Expect the code to be pasted. await expect(page.locator('.cm-content')).toContainText(`2x8`) @@ -549,13 +547,13 @@ test.describe('Text-to-CAD tests', () => { await page.locator('.cm-content').click({ position: { x: 10, y: 10 } }) // Paste the code. - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('KeyA') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') await page.keyboard.press('Backspace') - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('KeyV') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') // Expect the code to be pasted. await expect(page.locator('.cm-content')).toContainText(`2x4`) @@ -636,9 +634,9 @@ test.describe('Text-to-CAD tests', () => { await page.locator('.cm-content').click({ position: { x: 10, y: 10 } }) // Paste the code. - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('KeyV') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') // Expect the code to be pasted. await expect(page.locator('.cm-content')).toContainText(`2x4`) diff --git a/e2e/playwright/various.spec.ts b/e2e/playwright/various.spec.ts index 8ef9abf0b..fb947eca6 100644 --- a/e2e/playwright/various.spec.ts +++ b/e2e/playwright/various.spec.ts @@ -1,13 +1,6 @@ import { test, expect } from '@playwright/test' -import { - doExport, - getUtils, - makeTemplate, - metaModifier, - setup, - tearDown, -} from './test-utils' +import { doExport, getUtils, makeTemplate, setup, tearDown } from './test-utils' test.beforeEach(async ({ context, page }) => { await setup(context, page) @@ -17,8 +10,6 @@ test.afterEach(async ({ page }, testInfo) => { await tearDown(page, testInfo) }) -const CtrlKey = process.platform === 'darwin' ? 'Meta' : 'Control' - test('Units menu', async ({ page }) => { const u = await getUtils(page) await page.setViewportSize({ width: 1200, height: 500 }) @@ -157,7 +148,7 @@ test('Paste should not work unless an input is focused', async ({ // Paste without the code pane focused await codeEditorText.blur() - await page.keyboard.press(`${metaModifier}+KeyV`) + await page.keyboard.press('ControlOrMeta+KeyV') // Show that the paste didn't work but typing did await expect(codeEditorText).not.toContainText(pasteContent) @@ -166,7 +157,7 @@ test('Paste should not work unless an input is focused', async ({ // Paste with the code editor focused // Following this guidance: https://github.com/microsoft/playwright/issues/8114 await codeEditorText.focus() - await page.keyboard.press(`${metaModifier}+KeyV`) + await page.keyboard.press('ControlOrMeta+KeyV') await expect( await page.evaluate( () => document.querySelector('.cm-content')?.textContent @@ -380,9 +371,9 @@ test('Basic default modeling and sketch hotkeys work', async ({ page }) => { await test.step(`Type code with sketch hotkeys, shouldn't fire`, async () => { // Since there's code now, we have to get to the end of the line await page.locator('.cm-line').last().click() - await page.keyboard.down(CtrlKey) + await page.keyboard.down('ControlOrMeta') await page.keyboard.press('ArrowRight') - await page.keyboard.up(CtrlKey) + await page.keyboard.up('ControlOrMeta') await page.keyboard.press('Enter') await page.keyboard.type('//') diff --git a/src/Router.tsx b/src/Router.tsx index 6ffde2534..a7a80af97 100644 --- a/src/Router.tsx +++ b/src/Router.tsx @@ -190,7 +190,7 @@ function CoreDump() { () => new CoreDumpManager(engineCommandManager, codeManager, token), [] ) - useHotkeyWrapper(['meta + shift + .'], () => { + useHotkeyWrapper(['mod + shift + .'], () => { toast.promise( coreDump(coreDumpManager, true), { diff --git a/src/components/FileTree.tsx b/src/components/FileTree.tsx index cc1b2c32d..13ec6ca75 100644 --- a/src/components/FileTree.tsx +++ b/src/components/FileTree.tsx @@ -396,8 +396,8 @@ export const FileTreeMenu = () => { }) } - useHotkeyWrapper(['meta + n'], createFile) - useHotkeyWrapper(['meta + shift + n'], createFolder) + useHotkeyWrapper(['mod + n'], createFile) + useHotkeyWrapper(['mod + shift + n'], createFolder) return ( <> diff --git a/src/hooks/usePlatform.ts b/src/hooks/usePlatform.ts index cd751f013..d74944745 100644 --- a/src/hooks/usePlatform.ts +++ b/src/hooks/usePlatform.ts @@ -1,44 +1,11 @@ -import { isDesktop } from 'lib/isDesktop' +import { Platform, platform } from 'lib/utils' import { useEffect, useState } from 'react' -export type Platform = 'macos' | 'windows' | 'linux' | '' - export default function usePlatform() { const [platformName, setPlatformName] = useState('') useEffect(() => { - function getPlatform(): Platform { - const platform = window.electron.platform ?? '' - // https://nodejs.org/api/process.html#processplatform - switch (platform) { - case 'darwin': - return 'macos' - case 'win32': - return 'windows' - // We don't currently care to distinguish between these. - case 'android': - case 'freebsd': - case 'linux': - case 'openbsd': - case 'sunos': - return 'linux' - default: - console.error('Unknown platform:', platform) - return '' - } - } - - if (isDesktop()) { - setPlatformName(getPlatform()) - } else { - if (navigator.userAgent.indexOf('Mac') !== -1) { - setPlatformName('macos') - } else if (navigator.userAgent.indexOf('Win') !== -1) { - setPlatformName('windows') - } else if (navigator.userAgent.indexOf('Linux') !== -1) { - setPlatformName('linux') - } - } + setPlatformName(platform()) }, [setPlatformName]) return platformName diff --git a/src/lib/hotkeyWrapper.ts b/src/lib/hotkeyWrapper.ts index 11c10a637..13b4c9ede 100644 --- a/src/lib/hotkeyWrapper.ts +++ b/src/lib/hotkeyWrapper.ts @@ -31,7 +31,7 @@ function mapHotkeyToCodeMirrorHotkey(hotkey: string): string { return hotkey .replaceAll('+', '-') .replaceAll(' ', '') - .replaceAll('mod', 'Meta') + .replaceAll('mod', 'Mod') .replaceAll('meta', 'Meta') .replaceAll('ctrl', 'Ctrl') .replaceAll('shift', 'Shift') diff --git a/src/lib/settings/initialKeybindings.ts b/src/lib/settings/initialKeybindings.ts index 9c51bfdc3..dda9c1a8e 100644 --- a/src/lib/settings/initialKeybindings.ts +++ b/src/lib/settings/initialKeybindings.ts @@ -1,4 +1,5 @@ import { isDesktop } from 'lib/isDesktop' +import { platform } from 'lib/utils' export type InteractionMapItem = { name: string @@ -24,6 +25,11 @@ export const interactionMapCategories = [ type InteractionMapCategory = (typeof interactionMapCategories)[number] +/** + * Primary modifier key for the current platform. + */ +const PRIMARY = platform() === 'macos' ? 'Command' : 'Control' + /** * A temporary implementation of the interaction map for * display purposes only. @@ -38,7 +44,7 @@ export const interactionMap: Record< Settings: [ { name: 'toggle-settings', - sequence: isDesktop() ? 'Meta+,' : 'Shift+Meta+,', + sequence: isDesktop() ? `${PRIMARY}+,` : `Shift+${PRIMARY}+,`, title: 'Toggle Settings', description: 'Opens the settings dialog. Always available.', }, @@ -53,7 +59,7 @@ export const interactionMap: Record< 'Command Palette': [ { name: 'toggle-command-palette', - sequence: 'Meta+K', + sequence: `${PRIMARY}+K`, title: 'Toggle Command Palette', description: 'Always available. Use Ctrl+/ on Windows/Linux.', }, @@ -159,7 +165,7 @@ export const interactionMap: Record< }, { name: 'delete-file', - sequence: 'Meta+Backspace', + sequence: `${PRIMARY}+Backspace`, title: 'Delete File/Folder', description: 'Available when a file or folder is selected in the file tree.', diff --git a/src/lib/utils.ts b/src/lib/utils.ts index b9085d6d6..454686ba8 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -1,6 +1,7 @@ import { SourceRange } from '../lang/wasm' import { v4 } from 'uuid' +import { isDesktop } from './isDesktop' export const uuidv4 = v4 @@ -126,6 +127,41 @@ export function getNormalisedCoordinates({ } } +// TODO: Remove the empty platform type. +export type Platform = 'macos' | 'windows' | 'linux' | '' + +export function platform(): Platform { + if (isDesktop()) { + const platform = window.electron.platform ?? '' + // https://nodejs.org/api/process.html#processplatform + switch (platform) { + case 'darwin': + return 'macos' + case 'win32': + return 'windows' + // We don't currently care to distinguish between these. + case 'android': + case 'freebsd': + case 'linux': + case 'openbsd': + case 'sunos': + return 'linux' + default: + console.error('Unknown platform:', platform) + return '' + } + } + if (navigator.userAgent.indexOf('Mac') !== -1) { + return 'macos' + } else if (navigator.userAgent.indexOf('Win') !== -1) { + return 'windows' + } else if (navigator.userAgent.indexOf('Linux') !== -1) { + return 'linux' + } + console.error('Unknown platform userAgent:', navigator.userAgent) + return '' +} + export function isReducedMotion(): boolean { return ( typeof window !== 'undefined' && From ebed10bc766664c58118df8858336cefa79caae9 Mon Sep 17 00:00:00 2001 From: Frank Noirot Date: Thu, 22 Aug 2024 19:42:21 -0400 Subject: [PATCH 3/6] Franknoirot/fix file deletion (#3569) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Don't chop off file name from file path * Add a test to confirm file deletion works (as long as you have a main.kcl) * Add TODO test for when main.kcl doesn't exist * A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest) * Make the bad prompt test generate a new prompt each run --------- Co-authored-by: github-actions[bot] Co-authored-by: Jess Frazelle Co-authored-by: Jonathan Tran Co-authored-by: 49fl --- e2e/playwright/projects.spec.ts | 72 +++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/e2e/playwright/projects.spec.ts b/e2e/playwright/projects.spec.ts index 1fcc86867..51cf72c1a 100644 --- a/e2e/playwright/projects.spec.ts +++ b/e2e/playwright/projects.spec.ts @@ -1682,6 +1682,78 @@ test.describe('Renaming in the file tree', () => { ) }) +test.describe('Deleting files from the file pane', () => { + test( + `when main.kcl exists, navigate to main.kcl`, + { tag: '@electron' }, + async ({ browserName }, testInfo) => { + test.skip( + process.platform === 'win32', + 'TODO: remove this skip https://github.com/KittyCAD/modeling-app/issues/3557' + ) + const { electronApp, page } = await setupElectron({ + testInfo, + folderSetupFn: async (dir) => { + await fsp.mkdir(`${dir}/testProject`, { recursive: true }) + await fsp.copyFile( + 'src/wasm-lib/tests/executor/inputs/cylinder.kcl', + `${dir}/testProject/main.kcl` + ) + await fsp.copyFile( + 'src/wasm-lib/tests/executor/inputs/basic_fillet_cube_end.kcl', + `${dir}/testProject/fileToDelete.kcl` + ) + }, + }) + const u = await getUtils(page) + await page.setViewportSize({ width: 1200, height: 500 }) + page.on('console', console.log) + + // Constants and locators + const projectCard = page.getByText('testProject') + const projectMenuButton = page.getByTestId('project-sidebar-toggle') + const fileToDelete = page + .getByRole('listitem') + .filter({ has: page.getByRole('button', { name: 'fileToDelete.kcl' }) }) + const deleteMenuItem = page.getByRole('button', { name: 'Delete' }) + const deleteConfirmation = page.getByTestId('delete-confirmation') + + await test.step('Open project and navigate to fileToDelete.kcl', async () => { + await projectCard.click() + await u.waitForPageLoad() + await u.openFilePanel() + + await fileToDelete.click() + await u.waitForPageLoad() + await u.openKclCodePanel() + await expect(u.codeLocator).toContainText('getOppositeEdge(thing)') + await u.closeKclCodePanel() + }) + + await test.step('Delete fileToDelete.kcl', async () => { + await fileToDelete.click({ button: 'right' }) + await expect(deleteMenuItem).toBeVisible() + await deleteMenuItem.click() + await expect(deleteConfirmation).toBeVisible() + await deleteConfirmation.click() + }) + + await test.step('Check deletion and navigation', async () => { + await u.waitForPageLoad() + await expect(fileToDelete).not.toBeVisible() + await u.closeFilePanel() + await u.openKclCodePanel() + await expect(u.codeLocator).toContainText('circle(') + await expect(projectMenuButton).toContainText('main.kcl') + }) + + await electronApp.close() + } + ) + + test.fixme('TODO - when main.kcl does not exist', async () => {}) +}) + test( 'Original project name persist after onboarding', { tag: '@electron' }, From 713a30ed725fe500bf6cea6b8faf5b476e76687e Mon Sep 17 00:00:00 2001 From: Frank Noirot Date: Thu, 22 Aug 2024 19:51:26 -0400 Subject: [PATCH 4/6] Fix initial default app settings behavior when the user has no settings yet (#3601) Fix initial default app settings behavior when the user has no settings yet. Co-authored-by: Jess Frazelle --- src/lib/desktop.ts | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/lib/desktop.ts b/src/lib/desktop.ts index 842db440b..0190935cd 100644 --- a/src/lib/desktop.ts +++ b/src/lib/desktop.ts @@ -441,28 +441,34 @@ export const readProjectSettingsFile = async ( return configObj } +/** + * Read the app settings file, or creates an initial one if it doesn't exist. + */ export const readAppSettingsFile = async () => { let settingsPath = await getAppSettingsFilePath() - try { - await window.electron.stat(settingsPath) - } catch (e) { - if (e === 'ENOENT') { - const config = defaultAppSettings() - if (err(config)) return Promise.reject(config) - if (!config.settings?.app) - return Promise.reject(new Error('config.app is falsey')) - config.settings.app.project_directory = await getInitialDefaultDir() - return config + // The file exists, read it and parse it. + if (window.electron.exists(settingsPath)) { + const configToml = await window.electron.readFile(settingsPath) + const configObj = parseAppSettings(configToml) + if (err(configObj)) { + return Promise.reject(configObj) } - } - const configToml = await window.electron.readFile(settingsPath) - const configObj = parseAppSettings(configToml) - if (err(configObj)) { - return Promise.reject(configObj) + + return configObj } - return configObj + // The file doesn't exist, create a new one. + // This defaultAppConfig is truly an empty object every time. + const defaultAppConfig = defaultAppSettings() + if (err(defaultAppConfig)) { + return Promise.reject(defaultAppConfig) + } + const initialDirConfig: DeepPartial = { + settings: { project: { directory: await getInitialDefaultDir() } }, + } + const config = Object.assign(defaultAppConfig, initialDirConfig) + return config } export const writeAppSettingsFile = async (tomlStr: string) => { From 22a9f4491606deb81867cf49f7dadbe2c4f64f1f Mon Sep 17 00:00:00 2001 From: Jess Frazelle Date: Thu, 22 Aug 2024 17:18:29 -0700 Subject: [PATCH 5/6] fix coredump home page (#3624) updates Signed-off-by: Jess Frazelle --- src/lib/coredump.ts | 6 +++-- src/wasm-lib/kcl/src/coredump/mod.rs | 39 ++++++++++++++++++---------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/lib/coredump.ts b/src/lib/coredump.ts index ba649d54f..1f76b69fc 100644 --- a/src/lib/coredump.ts +++ b/src/lib/coredump.ts @@ -109,11 +109,13 @@ export class CoreDumpManager { getWebrtcStats(): Promise { if (!this.engineCommandManager.engineConnection) { - throw new Error('Engine connection not initialized') + // when the engine connection is not available, return an empty object. + return Promise.resolve(JSON.stringify({})) } if (!this.engineCommandManager.engineConnection.webrtcStatsCollector) { - throw new Error('Engine webrtcStatsCollector not initialized') + // when the engine connection is not available, return an empty object. + return Promise.resolve(JSON.stringify({})) } return this.engineCommandManager.engineConnection diff --git a/src/wasm-lib/kcl/src/coredump/mod.rs b/src/wasm-lib/kcl/src/coredump/mod.rs index a48dcb794..9e84afc52 100644 --- a/src/wasm-lib/kcl/src/coredump/mod.rs +++ b/src/wasm-lib/kcl/src/coredump/mod.rs @@ -230,29 +230,42 @@ pub struct OsInfo { #[serde(rename_all = "snake_case")] pub struct WebrtcStats { /// The packets lost. - pub packets_lost: u32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub packets_lost: Option, /// The frames received. - pub frames_received: u32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub frames_received: Option, /// The frame width. - pub frame_width: f32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub frame_width: Option, /// The frame height. - pub frame_height: f32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub frame_height: Option, /// The frame rate. - pub frame_rate: f32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub frame_rate: Option, /// The number of key frames decoded. - pub key_frames_decoded: u32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub key_frames_decoded: Option, /// The number of frames dropped. - pub frames_dropped: u32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub frames_dropped: Option, /// The pause count. - pub pause_count: u32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub pause_count: Option, /// The total pauses duration. - pub total_pauses_duration: f32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub total_pauses_duration: Option, /// The freeze count. - pub freeze_count: u32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub freeze_count: Option, /// The total freezes duration. - pub total_freezes_duration: f32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub total_freezes_duration: Option, /// The pli count. - pub pli_count: u32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub pli_count: Option, /// Packet jitter for this synchronizing source, measured in seconds. - pub jitter: f32, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub jitter: Option, } From 4d2375faace04afd3459ef60a6ecd5c828c1adf2 Mon Sep 17 00:00:00 2001 From: Kurt Hutten Date: Fri, 23 Aug 2024 17:11:17 +1000 Subject: [PATCH 6/6] tests for file manager the stuff that should happen on disk (#3634) tests for file manager the stuff that should happen on disk #3587 --- e2e/playwright/projects.spec.ts | 65 +++++++++++++++++++++++++++++---- e2e/playwright/test-utils.ts | 2 +- 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/e2e/playwright/projects.spec.ts b/e2e/playwright/projects.spec.ts index 51cf72c1a..f45cedc89 100644 --- a/e2e/playwright/projects.spec.ts +++ b/e2e/playwright/projects.spec.ts @@ -1324,7 +1324,7 @@ test.describe('Renaming in the file tree', () => { process.platform === 'win32', 'TODO: remove this skip https://github.com/KittyCAD/modeling-app/issues/3557' ) - const { electronApp, page } = await setupElectron({ + const { electronApp, page, dir } = await setupElectron({ testInfo, folderSetupFn: async (dir) => { await fsp.mkdir(join(dir, 'Test Project'), { recursive: true }) @@ -1352,6 +1352,16 @@ test.describe('Renaming in the file tree', () => { // Constants and locators const projectLink = page.getByText('Test Project') const projectMenuButton = page.getByTestId('project-sidebar-toggle') + const checkUnRenamedFS = () => { + const filePath = join(dir, 'Test Project', 'fileToRename.kcl') + return fs.existsSync(filePath) + } + const newFileName = 'newFileName' + const checkRenamedFS = () => { + const filePath = join(dir, 'Test Project', `${newFileName}.kcl`) + return fs.existsSync(filePath) + } + const fileToRename = page .getByRole('listitem') .filter({ has: page.getByRole('button', { name: 'fileToRename.kcl' }) }) @@ -1360,7 +1370,6 @@ test.describe('Renaming in the file tree', () => { .filter({ has: page.getByRole('button', { name: 'newFileName.kcl' }) }) const renameMenuItem = page.getByRole('button', { name: 'Rename' }) const renameInput = page.getByPlaceholder('fileToRename.kcl') - const newFileName = 'newFileName' const codeLocator = page.locator('.cm-content') await test.step('Open project and file pane', async () => { @@ -1371,6 +1380,8 @@ test.describe('Renaming in the file tree', () => { await u.openFilePanel() await expect(fileToRename).toBeVisible() + expect(checkUnRenamedFS()).toBeTruthy() + expect(checkRenamedFS()).toBeFalsy() await fileToRename.click() await expect(projectMenuButton).toContainText('fileToRename.kcl') await u.openKclCodePanel() @@ -1389,6 +1400,8 @@ test.describe('Renaming in the file tree', () => { await test.step('Verify the file is renamed', async () => { await expect(fileToRename).not.toBeAttached() await expect(renamedFile).toBeVisible() + expect(checkUnRenamedFS()).toBeFalsy() + expect(checkRenamedFS()).toBeTruthy() }) await test.step('Verify we navigated', async () => { @@ -1416,7 +1429,7 @@ test.describe('Renaming in the file tree', () => { process.platform === 'win32', 'TODO: remove this skip https://github.com/KittyCAD/modeling-app/issues/3557' ) - const { electronApp, page } = await setupElectron({ + const { electronApp, page, dir } = await setupElectron({ testInfo, folderSetupFn: async (dir) => { await fsp.mkdir(join(dir, 'Test Project'), { recursive: true }) @@ -1443,6 +1456,14 @@ test.describe('Renaming in the file tree', () => { // Constants and locators const newFileName = 'newFileName' + const checkUnRenamedFS = () => { + const filePath = join(dir, 'Test Project', 'fileToRename.kcl') + return fs.existsSync(filePath) + } + const checkRenamedFS = () => { + const filePath = join(dir, 'Test Project', `${newFileName}.kcl`) + return fs.existsSync(filePath) + } const projectLink = page.getByText('Test Project') const projectMenuButton = page.getByTestId('project-sidebar-toggle') const fileToRename = page @@ -1463,6 +1484,8 @@ test.describe('Renaming in the file tree', () => { await u.openFilePanel() await expect(fileToRename).toBeVisible() + expect(checkUnRenamedFS()).toBeTruthy() + expect(checkRenamedFS()).toBeFalsy() }) await test.step('Rename the file', async () => { @@ -1476,6 +1499,8 @@ test.describe('Renaming in the file tree', () => { await test.step('Verify the file is renamed', async () => { await expect(fileToRename).not.toBeAttached() await expect(renamedFile).toBeVisible() + expect(checkUnRenamedFS()).toBeFalsy() + expect(checkRenamedFS()).toBeTruthy() }) await test.step('Verify we have not navigated', async () => { @@ -1506,7 +1531,7 @@ test.describe('Renaming in the file tree', () => { process.platform === 'win32', 'TODO: remove this skip https://github.com/KittyCAD/modeling-app/issues/3557' ) - const { electronApp, page } = await setupElectron({ + const { electronApp, page, dir } = await setupElectron({ testInfo, folderSetupFn: async (dir) => { await fsp.mkdir(join(dir, 'Test Project'), { recursive: true }) @@ -1543,8 +1568,17 @@ test.describe('Renaming in the file tree', () => { }) const renamedFolder = page.getByRole('button', { name: 'newFolderName' }) const renameMenuItem = page.getByRole('button', { name: 'Rename' }) - const renameInput = page.getByPlaceholder('folderToRename') + const originalFolderName = 'folderToRename' + const renameInput = page.getByPlaceholder(originalFolderName) const newFolderName = 'newFolderName' + const checkUnRenamedFolderFS = () => { + const folderPath = join(dir, 'Test Project', originalFolderName) + return fs.existsSync(folderPath) + } + const checkRenamedFolderFS = () => { + const folderPath = join(dir, 'Test Project', newFolderName) + return fs.existsSync(folderPath) + } await test.step('Open project and file pane', async () => { await expect(projectLink).toBeVisible() @@ -1558,6 +1592,8 @@ test.describe('Renaming in the file tree', () => { await u.openFilePanel() await expect(folderToRename).toBeVisible() + expect(checkUnRenamedFolderFS()).toBeTruthy() + expect(checkRenamedFolderFS()).toBeFalsy() }) await test.step('Rename the folder', async () => { @@ -1577,6 +1613,8 @@ test.describe('Renaming in the file tree', () => { await expect(projectMenuButton).toContainText('main.kcl') await expect(renamedFolder).toBeVisible() await expect(folderToRename).not.toBeAttached() + expect(checkUnRenamedFolderFS()).toBeFalsy() + expect(checkRenamedFolderFS()).toBeTruthy() }) await electronApp.close() @@ -1592,7 +1630,7 @@ test.describe('Renaming in the file tree', () => { 'TODO: remove this skip https://github.com/KittyCAD/modeling-app/issues/3557' ) const exampleDir = join('src', 'wasm-lib', 'tests', 'executor', 'inputs') - const { electronApp, page } = await setupElectron({ + const { electronApp, page, dir } = await setupElectron({ testInfo, folderSetupFn: async (dir) => { await fsp.mkdir(join(dir, 'Test Project'), { recursive: true }) @@ -1625,8 +1663,17 @@ test.describe('Renaming in the file tree', () => { has: page.getByRole('button', { name: 'someFileWithin.kcl' }), }) const renameMenuItem = page.getByRole('button', { name: 'Rename' }) - const renameInput = page.getByPlaceholder('folderToRename') + const originalFolderName = 'folderToRename' + const renameInput = page.getByPlaceholder(originalFolderName) const newFolderName = 'newFolderName' + const checkUnRenamedFolderFS = () => { + const folderPath = join(dir, 'Test Project', originalFolderName) + return fs.existsSync(folderPath) + } + const checkRenamedFolderFS = () => { + const folderPath = join(dir, 'Test Project', newFolderName) + return fs.existsSync(folderPath) + } await test.step('Open project and navigate into folder', async () => { await expect(projectLink).toBeVisible() @@ -1649,6 +1696,8 @@ test.describe('Renaming in the file tree', () => { expect(newUrl).toContain('folderToRename') expect(newUrl).toContain('someFileWithin.kcl') expect(newUrl).not.toContain('main.kcl') + expect(checkUnRenamedFolderFS()).toBeTruthy() + expect(checkRenamedFolderFS()).toBeFalsy() }) await test.step('Rename the folder', async () => { @@ -1675,6 +1724,8 @@ test.describe('Renaming in the file tree', () => { expect(url).not.toContain('main.kcl') expect(url).toContain(newFolderName) expect(url).toContain('someFileWithin.kcl') + expect(checkUnRenamedFolderFS()).toBeFalsy() + expect(checkRenamedFolderFS()).toBeTruthy() }) await electronApp.close() diff --git a/e2e/playwright/test-utils.ts b/e2e/playwright/test-utils.ts index 39617874e..e81274549 100644 --- a/e2e/playwright/test-utils.ts +++ b/e2e/playwright/test-utils.ts @@ -865,7 +865,7 @@ export async function setupElectron({ await setup(context, page) - return { electronApp, page } + return { electronApp, page, dir: projectDirName } } export async function isOutOfViewInScrollContainer(