Fix onboarding rendering (#4789)

* fix onboarding rendering Signed-off-by: Jess Frazelle <[email protected]> * updates Signed-off-by: Jess Frazelle <[email protected]> * updates Signed-off-by: Jess Frazelle <[email protected]> * empty string Signed-off-by: Jess Frazelle <[email protected]> * A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest-8-cores) * updates Signed-off-by: Jess Frazelle <[email protected]> * updates Signed-off-by: Jess Frazelle <[email protected]> * updates Signed-off-by: Jess Frazelle <[email protected]> * updates Signed-off-by: Jess Frazelle <[email protected]> * A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores) * empty * A snapshot a day keeps the bugs away! 📷🐛 (OS: ubuntu-latest-8-cores) * empty * can be off by 20 Signed-off-by: Jess Frazelle <[email protected]> * can be off by 20 Signed-off-by: Jess Frazelle <[email protected]> --------- Signed-off-by: Jess Frazelle <[email protected]> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
KittyCAD · Dec 14, 2024 · 96652a0 · 96652a0
1 parent 04e586d
commit 96652a0
Show file tree

Hide file tree

Showing 10 changed files with 90 additions and 28 deletions.
diff --git a/e2e/playwright/fixtures/sceneFixture.ts b/e2e/playwright/fixtures/sceneFixture.ts
@@ -220,23 +220,7 @@ export class SceneFixture {
     coords: { x: number; y: number },
     diff: number
   ) => {
-    let finalValue = colour
-    await expect
-      .poll(async () => {
-        const pixel = (await getPixelRGBs(this.page)(coords, 1))[0]
-        if (!pixel) return null
-        finalValue = pixel
-        return pixel.every(
-          (channel, index) => Math.abs(channel - colour[index]) < diff
-        )
-      })
-      .toBeTruthy()
-      .catch((cause) => {
-        throw new Error(
-          `ExpectPixelColor: expecting ${colour} got ${finalValue}`,
-          { cause }
-        )
-      })
+    await expectPixelColor(this.page, colour, coords, diff)
   }
 
   get gizmo() {
@@ -252,3 +236,28 @@ export class SceneFixture {
     await buttonToTest.click()
   }
 }
+
+export async function expectPixelColor(
+  page: Page,
+  colour: [number, number, number],
+  coords: { x: number; y: number },
+  diff: number
+) {
+  let finalValue = colour
+  await expect
+    .poll(async () => {
+      const pixel = (await getPixelRGBs(page)(coords, 1))[0]
+      if (!pixel) return null
+      finalValue = pixel
+      return pixel.every(
+        (channel, index) => Math.abs(channel - colour[index]) < diff
+      )
+    })
+    .toBeTruthy()
+    .catch((cause) => {
+      throw new Error(
+        `ExpectPixelColor: expecting ${colour} got ${finalValue}`,
+        { cause }
+      )
+    })
+}
diff --git a/e2e/playwright/onboarding-tests.spec.ts b/e2e/playwright/onboarding-tests.spec.ts
@@ -19,6 +19,7 @@ import {
   TEST_SETTINGS_ONBOARDING_USER_MENU,
 } from './storageStates'
 import * as TOML from '@iarna/toml'
+import { expectPixelColor } from './fixtures/sceneFixture'
 
 test.beforeEach(async ({ context, page }, testInfo) => {
   if (testInfo.tags.includes('@electron')) {
@@ -45,7 +46,7 @@ test.describe('Onboarding tests', () => {
       { settingsKey: TEST_SETTINGS_KEY }
     )
 
-    await page.setViewportSize({ width: 1200, height: 500 })
+    await page.setViewportSize({ width: 1200, height: 1000 })
 
     await u.waitForAuthSkipAppStart()
 
@@ -54,6 +55,12 @@ test.describe('Onboarding tests', () => {
 
     // *and* that the code is shown in the editor
     await expect(page.locator('.cm-content')).toContainText('// Shelf Bracket')
+
+    // Make sure the model loaded
+    const XYPlanePoint = { x: 774, y: 116 } as const
+    const modelColor: [number, number, number] = [45, 45, 45]
+    await page.mouse.move(XYPlanePoint.x, XYPlanePoint.y)
+    expect(await u.getGreatestPixDiff(XYPlanePoint, modelColor)).toBeLessThan(8)
   })
 
   test(
@@ -72,7 +79,7 @@ test.describe('Onboarding tests', () => {
 
       const u = await getUtils(page)
 
-      const viewportSize = { width: 1200, height: 500 }
+      const viewportSize = { width: 1200, height: 1000 }
       await page.setViewportSize(viewportSize)
 
       await test.step(`Create a project and open to the onboarding`, async () => {
@@ -92,6 +99,14 @@ test.describe('Onboarding tests', () => {
         await expect(page.locator('.cm-content')).toContainText(
           '// Shelf Bracket'
         )
+
+        // TODO: jess make less shit
+        // Make sure the model loaded
+        //const XYPlanePoint = { x: 986, y: 522 } as const
+        //const modelColor: [number, number, number] = [76, 76, 76]
+        //await page.mouse.move(XYPlanePoint.x, XYPlanePoint.y)
+
+        //await expectPixelColor(page, modelColor, XYPlanePoint, 8)
       })
 
       await electronApp.close()
@@ -108,7 +123,7 @@ test.describe('Onboarding tests', () => {
     }, initialCode)
 
     const u = await getUtils(page)
-    await page.setViewportSize({ width: 1200, height: 500 })
+    await page.setViewportSize({ width: 1200, height: 1000 })
     await u.waitForAuthSkipAppStart()
 
     // Replay the onboarding
@@ -140,6 +155,12 @@ test.describe('Onboarding tests', () => {
         return localStorage.getItem('persistCode')
       })
     ).toContain('// Shelf Bracket')
+
+    // Make sure the model loaded
+    const XYPlanePoint = { x: 986, y: 522 } as const
+    const modelColor: [number, number, number] = [76, 76, 76]
+    await page.mouse.move(XYPlanePoint.x, XYPlanePoint.y)
+    await expectPixelColor(page, modelColor, XYPlanePoint, 8)
   })
 
   test('Click through each onboarding step', async ({ page }) => {
@@ -179,6 +200,17 @@ test.describe('Onboarding tests', () => {
     // Test that the onboarding pane is gone
     await expect(page.getByTestId('onboarding-content')).not.toBeVisible()
     await expect(page.url()).not.toContain('onboarding')
+
+    await u.openAndClearDebugPanel()
+    await u.expectCmdLog('[data-message-type="execution-done"]')
+    await u.closeDebugPanel()
+
+    // TODO: jess to fix
+    // Make sure the model loaded
+    //const XYPlanePoint = { x: 774, y: 516 } as const
+    // const modelColor: [number, number, number] = [129, 129, 129]
+    // await page.mouse.move(XYPlanePoint.x, XYPlanePoint.y)
+    // await expectPixelColor(page, modelColor, XYPlanePoint, 20)
   })
 
   test('Onboarding redirects and code updating', async ({ page }) => {
@@ -439,7 +471,7 @@ test(
     })
 
     await test.step('Navigate into project', async () => {
-      await page.setViewportSize({ width: 1200, height: 500 })
+      await page.setViewportSize({ width: 1200, height: 1000 })
 
       page.on('console', console.log)
 
@@ -462,7 +494,15 @@ test(
     await test.step('Confirm that the onboarding has restarted', async () => {
       await expect(tutorialProjectIndicator).toBeVisible()
       await expect(tutorialModalText).toBeVisible()
+      // Make sure the model loaded
+      const XYPlanePoint = { x: 988, y: 523 } as const
+      const modelColor: [number, number, number] = [76, 76, 76]
+
+      await page.mouse.move(XYPlanePoint.x, XYPlanePoint.y)
+      await expectPixelColor(page, modelColor, XYPlanePoint, 8)
       await tutorialDismissButton.click()
+      // Make sure model still there.
+      await expectPixelColor(page, modelColor, XYPlanePoint, 8)
     })
 
     await test.step('Clear code and restart onboarding from settings', async () => {

diff --git a/...ts/code-color-goober-code-color-goober-opening-window-1-Google-Chrome-linux.png b/...ts/code-color-goober-code-color-goober-opening-window-1-Google-Chrome-linux.png
diff --git a/...apshots/extrude-on-default-planes-should-be-stable-XY-1-Google-Chrome-win32.png b/...apshots/extrude-on-default-planes-should-be-stable-XY-1-Google-Chrome-win32.png
diff --git a/src/components/ModelingSidebar/ModelingPane.tsx b/src/components/ModelingSidebar/ModelingPane.tsx
@@ -6,6 +6,7 @@ import Tooltip from 'components/Tooltip'
 import { CustomIconName } from 'components/CustomIcon'
 import { IconDefinition } from '@fortawesome/free-solid-svg-icons'
 import { ActionIcon } from 'components/ActionIcon'
+import { onboardingPaths } from 'routes/Onboarding/paths'
 
 export interface ModelingPaneProps {
   id: string
@@ -70,7 +71,7 @@ export const ModelingPane = ({
   const { settings } = useSettingsAuthContext()
   const onboardingStatus = settings.context.app.onboardingStatus
   const pointerEventsCssClass =
-    onboardingStatus.current === 'camera'
+    onboardingStatus.current === onboardingPaths.CAMERA
       ? 'pointer-events-none '
       : 'pointer-events-auto '
   return (

diff --git a/src/components/ModelingSidebar/ModelingSidebar.tsx b/src/components/ModelingSidebar/ModelingSidebar.tsx
@@ -19,6 +19,7 @@ import { useCommandsContext } from 'hooks/useCommandsContext'
 import { IconDefinition } from '@fortawesome/free-solid-svg-icons'
 import { useKclContext } from 'lang/KclProvider'
 import { MachineManagerContext } from 'components/MachineManagerProvider'
+import { onboardingPaths } from 'routes/Onboarding/paths'
 
 interface ModelingSidebarProps {
   paneOpacity: '' | 'opacity-20' | 'opacity-40'
@@ -41,7 +42,7 @@ export function ModelingSidebar({ paneOpacity }: ModelingSidebarProps) {
   const onboardingStatus = settings.context.app.onboardingStatus
   const { send, context } = useModelingContext()
   const pointerEventsCssClass =
-    onboardingStatus.current === 'camera' ||
+    onboardingStatus.current === onboardingPaths.CAMERA ||
     context.store?.openPanes.length === 0
       ? 'pointer-events-none '
       : 'pointer-events-auto '

diff --git a/src/lib/routeLoaders.ts b/src/lib/routeLoaders.ts
@@ -15,6 +15,7 @@ import { fileSystemManager } from 'lang/std/fileSystemManager'
 import { getProjectInfo } from './desktop'
 import { createSettings } from './settings/initialSettings'
 import { normalizeLineEndings } from 'lib/codeEditor'
+import { OnboardingStatus } from 'wasm-lib/kcl/bindings/OnboardingStatus'
 
 // The root loader simply resolves the settings and any errors that
 // occurred during the settings load
@@ -53,14 +54,15 @@ export const telemetryLoader: LoaderFunction = async ({
 // Redirect users to the appropriate onboarding page if they haven't completed it
 export const onboardingRedirectLoader: ActionFunction = async (args) => {
   const { settings } = await loadAndValidateSettings()
-  const onboardingStatus = settings.app.onboardingStatus.current || ''
+  const onboardingStatus: OnboardingStatus =
+    settings.app.onboardingStatus.current || ''
   const notEnRouteToOnboarding = !args.request.url.includes(
     PATHS.ONBOARDING.INDEX
   )
-  // '' is the initial state, 'done' and 'dismissed' are the final states
+  // '' is the initial state, 'completed' and 'dismissed' are the final states
   const hasValidOnboardingStatus =
     onboardingStatus.length === 0 ||
-    !(onboardingStatus === 'done' || onboardingStatus === 'dismissed')
+    !(onboardingStatus === 'completed' || onboardingStatus === 'dismissed')
   const shouldRedirectToOnboarding =
     notEnRouteToOnboarding && hasValidOnboardingStatus
 

diff --git a/src/lib/settings/initialSettings.tsx b/src/lib/settings/initialSettings.tsx
@@ -19,6 +19,7 @@ import Tooltip from 'components/Tooltip'
 import { toSync } from 'lib/utils'
 import { reportRejection } from 'lib/trap'
 import { CameraProjectionType } from 'wasm-lib/kcl/bindings/CameraProjectionType'
+import { OnboardingStatus } from 'wasm-lib/kcl/bindings/OnboardingStatus'
 
 /**
  * A setting that can be set at the user or project level
@@ -189,8 +190,10 @@ export function createSettings() {
           inputType: 'boolean',
         },
       }),
-      onboardingStatus: new Setting<string>({
+      onboardingStatus: new Setting<OnboardingStatus>({
         defaultValue: '',
+        // TODO: this could be better but we don't have a TS side real enum
+        // for this yet
         validate: (v) => typeof v === 'string',
         hideOnPlatform: 'both',
       }),

diff --git a/src/routes/Onboarding/paths.ts b/src/routes/Onboarding/paths.ts
@@ -1,4 +1,6 @@
-export const onboardingPaths = {
+import { OnboardingStatus } from 'wasm-lib/kcl/bindings/OnboardingStatus'
+
+export const onboardingPaths: Record<string, OnboardingStatus> = {
   INDEX: '/',
   CAMERA: '/camera',
   STREAMING: '/streaming',

diff --git a/src/wasm-lib/kcl/src/settings/types/mod.rs b/src/wasm-lib/kcl/src/settings/types/mod.rs
@@ -477,6 +477,10 @@ pub struct CommandBarSettings {
 #[serde(rename_all = "snake_case")]
 #[display(style = "snake_case")]
 pub enum OnboardingStatus {
+    /// The unset state.
+    #[serde(rename = "")]
+    #[display("")]
+    Unset,
     /// The user has completed onboarding.
     Completed,
     /// The user has not completed onboarding.