From 9a9b6fbcac0b09a2ea01be255fe828e9953cb19d Mon Sep 17 00:00:00 2001 From: Kevin Nadro Date: Wed, 9 Apr 2025 16:14:13 -0600 Subject: [PATCH 1/5] fix: moving toast application state: --- e2e/playwright/named-views.spec.ts | 22 ++++++++++--- src/lib/commandBarConfigs/namedViewsConfig.ts | 13 +++++--- src/machines/settingsMachine.ts | 33 +++++++++++++++++-- 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/e2e/playwright/named-views.spec.ts b/e2e/playwright/named-views.spec.ts index 28e5b192c5e..7cbf73629ec 100644 --- a/e2e/playwright/named-views.spec.ts +++ b/e2e/playwright/named-views.spec.ts @@ -105,6 +105,9 @@ test.describe('Named view tests', () => { PROJECT_SETTINGS_FILE_NAME ) + const toastMessage = page.getByText('Named view uuid1 created.') + expect(toastMessage).toBeInViewport() + // Expect project.toml to be generated on disk since a named view was created await expect(async () => { let exists = await fileExists(tempProjectSettingsFilePath) @@ -130,7 +133,6 @@ test.describe('Named view tests', () => { }, testInfo) => { const projectName = 'named-views' const myNamedView1 = 'uuid1' - const myNamedView2 = 'uuid2' // Create project and go into the project await createProject({ name: projectName, page }) @@ -142,6 +144,9 @@ test.describe('Named view tests', () => { await cmdBar.argumentInput.fill(myNamedView1) await cmdBar.progressCmdBar(false) + let toastMessage = page.getByText('Named view uuid1 created.') + expect(toastMessage).toBeInViewport() + // Generate file paths for project.toml const projectDirName = testInfo.outputPath('electron-test-projects-dir') const tempProjectSettingsFilePath = join( @@ -170,9 +175,12 @@ test.describe('Named view tests', () => { // Delete a named view await cmdBar.openCmdBar() await cmdBar.chooseCommand('delete named view') - cmdBar.selectOption({ name: myNamedView2 }) + cmdBar.selectOption({ name: myNamedView1 }) await cmdBar.progressCmdBar(false) + toastMessage = page.getByText('Named view uuid1 removed.') + expect(toastMessage).toBeInViewport() + await expect(async () => { // Read project.toml into memory again since we deleted a named view let tomlString = await fsp.readFile(tempProjectSettingsFilePath, 'utf-8') @@ -202,6 +210,9 @@ test.describe('Named view tests', () => { await cmdBar.argumentInput.fill(myNamedView) await cmdBar.progressCmdBar(false) + let toastMessage = page.getByText('Named view uuid1 created.') + expect(toastMessage).toBeInViewport() + // Generate file paths for project.toml const projectDirName = testInfo.outputPath('electron-test-projects-dir') const tempProjectSettingsFilePath = join( @@ -258,7 +269,8 @@ test.describe('Named view tests', () => { await cmdBar.argumentInput.fill(myNamedView1) await cmdBar.progressCmdBar(false) - await page.waitForTimeout(1000) + let toastMessage = page.getByText('Named view uuid1 created.') + expect(toastMessage).toBeInViewport() const orbitMouseStart = { x: 800, y: 130 } const orbitMouseEnd = { x: 0, y: 130 } @@ -276,8 +288,8 @@ test.describe('Named view tests', () => { await cmdBar.argumentInput.fill(myNamedView2) await cmdBar.progressCmdBar(false) - // Wait a moment for the project.toml to get written to disk with the new view point - await page.waitForTimeout(1000) + toastMessage = page.getByText('Named view uuid2 created.') + expect(toastMessage).toBeInViewport() // Generate paths for the project.toml const tempProjectSettingsFilePath = join( diff --git a/src/lib/commandBarConfigs/namedViewsConfig.ts b/src/lib/commandBarConfigs/namedViewsConfig.ts index de08220670a..e020a50456d 100644 --- a/src/lib/commandBarConfigs/namedViewsConfig.ts +++ b/src/lib/commandBarConfigs/namedViewsConfig.ts @@ -160,10 +160,11 @@ export function createNamedViewsCommand() { data: { level: 'project', value: requestedNamedViews, + toastCallback: () => { + toast.success(`Named view ${requestedView.name} created.`) + }, }, }) - - toast.success(`Named view ${requestedView.name} created.`) } } } @@ -210,9 +211,11 @@ export function createNamedViewsCommand() { data: { level: 'project', value: rest, - }, + toastCallback: () => { + toast.success(`Named view ${viewToDelete.name} removed.`) + }, + }, }) - toast.success(`Named view ${viewToDelete.name} removed.`) } else { toast.error(`Unable to delete, could not find the named view`) } @@ -307,6 +310,8 @@ export function createNamedViewsCommand() { type: 'default_camera_get_settings', }, }) + + // We do not have the promise of the engine command for ensuring the camera projection has been completed. toast.success(`Named view ${name} loaded.`) } else { toast.error(`Unable to load named view, could not find named view`) diff --git a/src/machines/settingsMachine.ts b/src/machines/settingsMachine.ts index 5efb6ba9691..b51e96c71ba 100644 --- a/src/machines/settingsMachine.ts +++ b/src/machines/settingsMachine.ts @@ -76,7 +76,14 @@ export const settingsMachine = setup({ level: SettingsLevel } | { type: 'Set all settings'; settings: typeof settings } - | { type: 'set.app.namedViews'; value: NamedView } + | { + type: 'set.app.namedViews' + data: { + value: NamedView + toastCallback: () => void + level: SettingsLevel + } + } | { type: 'load.project'; project?: Project } | { type: 'clear.project' } ) & { doNotPersist?: boolean }, @@ -84,7 +91,11 @@ export const settingsMachine = setup({ actors: { persistSettings: fromPromise< void, - { doNotPersist: boolean; context: SettingsMachineContext } + { + doNotPersist: boolean + context: SettingsMachineContext + toastCallback?: () => void + } >(async ({ input }) => { // Without this, when a user changes the file, it'd // create a detection loop with the file-system watcher. @@ -93,7 +104,12 @@ export const settingsMachine = setup({ codeManager.writeCausedByAppCheckedInFileTreeFileSystemWatcher = true const { currentProject, ...settings } = input.context - return saveSettings(settings, currentProject?.path) + const val = await saveSettings(settings, currentProject?.path) + + if (input.toastCallback) { + input.toastCallback() + } + return val }), loadUserSettings: fromPromise(async () => { const { settings } = await loadAndValidateSettings() @@ -517,6 +533,17 @@ export const settingsMachine = setup({ }, }, input: ({ context, event }) => { + if ( + event.type === 'set.app.namedViews' && + 'toastCallback' in event.data + ) { + return { + doNotPersist: event.doNotPersist ?? false, + context, + toastCallback: event.data.toastCallback, + } + } + return { doNotPersist: event.doNotPersist ?? false, context, From a6cd92b18d3df012a701e4532dea863c0fcc1a2a Mon Sep 17 00:00:00 2001 From: Kevin Nadro Date: Wed, 9 Apr 2025 18:06:41 -0600 Subject: [PATCH 2/5] fix: forgot the await on the expect calls, ts didn't yell at me? --- e2e/playwright/named-views.spec.ts | 19 +++++++++---------- ...ify-named-view-gets-deleted-chromium-linux | 13 +------------ ...two-named-view-gets-created-chromium-linux | 2 +- 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/e2e/playwright/named-views.spec.ts b/e2e/playwright/named-views.spec.ts index 7cbf73629ec..63b11e60072 100644 --- a/e2e/playwright/named-views.spec.ts +++ b/e2e/playwright/named-views.spec.ts @@ -61,7 +61,6 @@ function tomlStringOverWriteNamedViewUuids(toml: string): string { } test.describe('Named view tests', () => { - test.skip() // TODO: Jace is working on these test('Verify project.toml is not created', async ({ page }, testInfo) => { // Create project and load it const projectName = 'named-views' @@ -106,7 +105,7 @@ test.describe('Named view tests', () => { ) const toastMessage = page.getByText('Named view uuid1 created.') - expect(toastMessage).toBeInViewport() + await expect(toastMessage).toBeInViewport() // Expect project.toml to be generated on disk since a named view was created await expect(async () => { @@ -145,7 +144,7 @@ test.describe('Named view tests', () => { await cmdBar.progressCmdBar(false) let toastMessage = page.getByText('Named view uuid1 created.') - expect(toastMessage).toBeInViewport() + await expect(toastMessage).toBeInViewport() // Generate file paths for project.toml const projectDirName = testInfo.outputPath('electron-test-projects-dir') @@ -179,7 +178,7 @@ test.describe('Named view tests', () => { await cmdBar.progressCmdBar(false) toastMessage = page.getByText('Named view uuid1 removed.') - expect(toastMessage).toBeInViewport() + await expect(toastMessage).toBeInViewport() await expect(async () => { // Read project.toml into memory again since we deleted a named view @@ -187,8 +186,8 @@ test.describe('Named view tests', () => { // Rewrite the uuids in the named views to match snapshot otherwise they will be randomly generated from rust and break tomlString = tomlStringOverWriteNamedViewUuids(tomlString) - // // Write the entire tomlString to a snapshot. - // // There are many key/value pairs to check this is a safer match. + // Write the entire tomlString to a snapshot. + // There are many key/value pairs to check this is a safer match. expect(tomlString).toMatchSnapshot('verify-named-view-gets-deleted') }).toPass() }) @@ -211,7 +210,7 @@ test.describe('Named view tests', () => { await cmdBar.progressCmdBar(false) let toastMessage = page.getByText('Named view uuid1 created.') - expect(toastMessage).toBeInViewport() + await expect(toastMessage).toBeInViewport() // Generate file paths for project.toml const projectDirName = testInfo.outputPath('electron-test-projects-dir') @@ -270,7 +269,7 @@ test.describe('Named view tests', () => { await cmdBar.progressCmdBar(false) let toastMessage = page.getByText('Named view uuid1 created.') - expect(toastMessage).toBeInViewport() + await expect(toastMessage).toBeInViewport() const orbitMouseStart = { x: 800, y: 130 } const orbitMouseEnd = { x: 0, y: 130 } @@ -281,7 +280,7 @@ test.describe('Named view tests', () => { }) await page.mouse.up({ button: 'middle' }) - await page.waitForTimeout(1000) + await page.waitForTimeout(5000) await cmdBar.openCmdBar() await cmdBar.chooseCommand('create named view') @@ -289,7 +288,7 @@ test.describe('Named view tests', () => { await cmdBar.progressCmdBar(false) toastMessage = page.getByText('Named view uuid2 created.') - expect(toastMessage).toBeInViewport() + await expect(toastMessage).toBeInViewport() // Generate paths for the project.toml const tempProjectSettingsFilePath = join( diff --git a/e2e/playwright/named-views.spec.ts-snapshots/verify-named-view-gets-deleted-chromium-linux b/e2e/playwright/named-views.spec.ts-snapshots/verify-named-view-gets-deleted-chromium-linux index 04d37214a11..535bdeed198 100644 --- a/e2e/playwright/named-views.spec.ts-snapshots/verify-named-view-gets-deleted-chromium-linux +++ b/e2e/playwright/named-views.spec.ts-snapshots/verify-named-view-gets-deleted-chromium-linux @@ -1,16 +1,5 @@ [settings] +app = { } modeling = { } text_editor = { } command_bar = { } - -[settings.app.named_views.0656fb1a-9640-473e-b334-591dc70c0138] -name = "uuid1" -eye_offset = 1_378.0059 -fov_y = 45 -is_ortho = false -ortho_scale_enabled = true -ortho_scale_factor = 1.6 -pivot_position = [ 0, 0, 0 ] -pivot_rotation = [ 0.5380994, 0.0, 0.0, 0.8428814 ] -world_coord_system = "right_handed_up_z" -version = 1 diff --git a/e2e/playwright/named-views.spec.ts-snapshots/verify-two-named-view-gets-created-chromium-linux b/e2e/playwright/named-views.spec.ts-snapshots/verify-two-named-view-gets-created-chromium-linux index 555dbad81ae..24f1d3d188f 100644 --- a/e2e/playwright/named-views.spec.ts-snapshots/verify-two-named-view-gets-created-chromium-linux +++ b/e2e/playwright/named-views.spec.ts-snapshots/verify-two-named-view-gets-created-chromium-linux @@ -22,7 +22,7 @@ fov_y = 45 is_ortho = false ortho_scale_enabled = true ortho_scale_factor = 1.6 -pivot_position = [ 1_826.5239, 0.0, 0.0 ] +pivot_position = [ 608.8414, 0.0, 0.0 ] pivot_rotation = [ 0.5380994, 0.0, 0.0, 0.8428814 ] world_coord_system = "right_handed_up_z" version = 1 From 60a27347b78f3ac222dfe41c539a61d7d599b8f7 Mon Sep 17 00:00:00 2001 From: Kevin Nadro Date: Wed, 9 Apr 2025 18:29:58 -0600 Subject: [PATCH 3/5] fix: cleaning up the camera move e2e code --- e2e/playwright/named-views.spec.ts | 15 +++++---------- ...ify-two-named-view-gets-created-chromium-linux | 6 +++--- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/e2e/playwright/named-views.spec.ts b/e2e/playwright/named-views.spec.ts index 63b11e60072..4a8a9142ef0 100644 --- a/e2e/playwright/named-views.spec.ts +++ b/e2e/playwright/named-views.spec.ts @@ -271,16 +271,11 @@ test.describe('Named view tests', () => { let toastMessage = page.getByText('Named view uuid1 created.') await expect(toastMessage).toBeInViewport() - const orbitMouseStart = { x: 800, y: 130 } - const orbitMouseEnd = { x: 0, y: 130 } - await page.mouse.move(orbitMouseStart.x, orbitMouseStart.y) - await page.mouse.down({ button: 'middle' }) - await page.mouse.move(orbitMouseEnd.x, orbitMouseEnd.y, { - steps: 3, - }) - await page.mouse.up({ button: 'middle' }) - - await page.waitForTimeout(5000) + await scene.moveCameraTo( + { x: 608, y: 0, z: 0}, + { x: 0, y: 0, z: 0 } + ) + await page.waitForTimeout(2500) await cmdBar.openCmdBar() await cmdBar.chooseCommand('create named view') diff --git a/e2e/playwright/named-views.spec.ts-snapshots/verify-two-named-view-gets-created-chromium-linux b/e2e/playwright/named-views.spec.ts-snapshots/verify-two-named-view-gets-created-chromium-linux index 24f1d3d188f..5723b4ac974 100644 --- a/e2e/playwright/named-views.spec.ts-snapshots/verify-two-named-view-gets-created-chromium-linux +++ b/e2e/playwright/named-views.spec.ts-snapshots/verify-two-named-view-gets-created-chromium-linux @@ -17,12 +17,12 @@ version = 1 [settings.app.named_views.c810cf04-c6cc-4a4a-8b11-17bf445dcab7] name = "uuid2" -eye_offset = 1_378.0059 +eye_offset = 608 fov_y = 45 is_ortho = false ortho_scale_enabled = true ortho_scale_factor = 1.6 -pivot_position = [ 608.8414, 0.0, 0.0 ] -pivot_rotation = [ 0.5380994, 0.0, 0.0, 0.8428814 ] +pivot_position = [ 0, 0, 0 ] +pivot_rotation = [ 0.5, 0.5, 0.5, 0.5 ] world_coord_system = "right_handed_up_z" version = 1 From 86cf79958bf8e94e694063876c4ca06854d3a009 Mon Sep 17 00:00:00 2001 From: Kevin Nadro Date: Wed, 9 Apr 2025 18:38:16 -0600 Subject: [PATCH 4/5] fix: lint,tsc,fmt --- e2e/playwright/named-views.spec.ts | 5 +---- src/lib/commandBarConfigs/namedViewsConfig.ts | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/e2e/playwright/named-views.spec.ts b/e2e/playwright/named-views.spec.ts index 4a8a9142ef0..74b7ed69c74 100644 --- a/e2e/playwright/named-views.spec.ts +++ b/e2e/playwright/named-views.spec.ts @@ -271,10 +271,7 @@ test.describe('Named view tests', () => { let toastMessage = page.getByText('Named view uuid1 created.') await expect(toastMessage).toBeInViewport() - await scene.moveCameraTo( - { x: 608, y: 0, z: 0}, - { x: 0, y: 0, z: 0 } - ) + await scene.moveCameraTo({ x: 608, y: 0, z: 0 }, { x: 0, y: 0, z: 0 }) await page.waitForTimeout(2500) await cmdBar.openCmdBar() diff --git a/src/lib/commandBarConfigs/namedViewsConfig.ts b/src/lib/commandBarConfigs/namedViewsConfig.ts index e020a50456d..997e3cd16b3 100644 --- a/src/lib/commandBarConfigs/namedViewsConfig.ts +++ b/src/lib/commandBarConfigs/namedViewsConfig.ts @@ -214,7 +214,7 @@ export function createNamedViewsCommand() { toastCallback: () => { toast.success(`Named view ${viewToDelete.name} removed.`) }, - }, + }, }) } else { toast.error(`Unable to delete, could not find the named view`) From e6efa1a569a1abe5cf0f7c0e5980de1cbbd80b45 Mon Sep 17 00:00:00 2001 From: Jace Browning Date: Thu, 10 Apr 2025 15:34:01 -0400 Subject: [PATCH 5/5] Unblock on macOS for now --- e2e/playwright/named-views.spec.ts | 9 +++++++-- e2e/playwright/test-utils.ts | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/e2e/playwright/named-views.spec.ts b/e2e/playwright/named-views.spec.ts index 74b7ed69c74..a95028bc65c 100644 --- a/e2e/playwright/named-views.spec.ts +++ b/e2e/playwright/named-views.spec.ts @@ -6,7 +6,9 @@ import type { NamedView } from '@rust/kcl-lib/bindings/NamedView' import { createProject, - perProjectsettingsToToml, + orRunWhenFullSuiteEnabled, + perProjectSettingsToToml, + runningOnMac, tomlToPerProjectSettings, } from '@e2e/playwright/test-utils' import { expect, test } from '@e2e/playwright/zoo-test' @@ -57,10 +59,13 @@ function tomlStringOverWriteNamedViewUuids(toml: string): string { settings.settings.app.named_views = remappedNamedViews } } - return perProjectsettingsToToml(settings) + return perProjectSettingsToToml(settings) } test.describe('Named view tests', () => { + if (runningOnMac()) { + test.fixme(orRunWhenFullSuiteEnabled()) + } test('Verify project.toml is not created', async ({ page }, testInfo) => { // Create project and load it const projectName = 'named-views' diff --git a/e2e/playwright/test-utils.ts b/e2e/playwright/test-utils.ts index 524dda15dfa..5b539aec1b6 100644 --- a/e2e/playwright/test-utils.ts +++ b/e2e/playwright/test-utils.ts @@ -1140,7 +1140,7 @@ export function tomlToPerProjectSettings( return TOML.parse(toml) } -export function perProjectsettingsToToml( +export function perProjectSettingsToToml( settings: DeepPartial ) { // eslint-disable-next-line no-restricted-syntax