Reset code on critical onboarding steps (#2727)

* Make sure we always reset the code on important steps no matter what the user did to it

* Convert comments in codeManager to JSDoc comments so they appear in diagnostics

* Was using the wrong codeManager callback

* Make sure editorView is available before resetting code

* Add Playwright test that shows the code being reset

* Fix up text that looks like linksÏ

* fmt

* Skip test on MacOS, make test more reliable on Chrome

* Update cargo-clippy to run based on paths on PRs as well

* playw fix

* try keep reports

* add fix me

* try one last thing

* fmt

---------

Co-authored-by: Kurt Hutten <k.hutten@protonmail.ch>

.github/workflows/cargo-clippy.yml

@@ -9,6 +9,12 @@ on:
       - '**.rs'
       - .github/workflows/cargo-clippy.yml
   pull_request:
+    paths:
+      - '**/Cargo.toml'
+      - '**/Cargo.lock'
+      - '**/rust-toolchain.toml'
+      - '**.rs'
+      - .github/workflows/cargo-clippy.yml
 concurrency:
   group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
   cancel-in-progress: true

.github/workflows/playwright.yml

@@ -133,7 +133,7 @@ jobs:
     - uses: actions/upload-artifact@v3
       if: always()
       with:
-        name: playwright-report
+        name: playwright-report-ubuntu
         path: playwright-report/
         retention-days: 30
 
@@ -204,6 +204,6 @@ jobs:
     - uses: actions/upload-artifact@v3
       if: always()
       with:
-        name: playwright-report
+        name: playwright-report-macos
         path: playwright-report/
         retention-days: 30

e2e/playwright/flow-tests.spec.ts

@@ -19,12 +19,15 @@ import {
   TEST_SETTINGS_ONBOARDING_START,
   TEST_CODE_GIZMO,
   TEST_SETTINGS_ONBOARDING_USER_MENU,
+  TEST_SETTINGS_ONBOARDING_PARAMETRIC_MODELING,
 } from './storageStates'
 import * as TOML from '@iarna/toml'
 import { LineInputsType } from 'lang/std/sketchcombos'
 import { Coords2d } from 'lang/std/sketch'
 import { KCL_DEFAULT_LENGTH } from 'lib/constants'
 import { EngineCommand } from 'lang/std/engineConnection'
+import { onboardingPaths } from 'routes/Onboarding/paths'
+import { bracket } from 'lib/exampleKcl'
 
 /*
 debug helper: unfortunately we do rely on exact coord mouse clicks in a few places
@@ -1396,6 +1399,59 @@ test.describe('Onboarding tests', () => {
     await expect(page.locator('.cm-content')).toHaveText(/.+/)
   })
 
+  test('Onboarding code gets reset to demo on Interactive Numbers step', async ({
+    page,
+  }) => {
+    test.skip(
+      process.platform === 'darwin',
+      "Skip on macOS, because Playwright isn't behaving the same as the actual browser"
+    )
+    const u = await getUtils(page)
+    const badCode = `// This is bad code we shouldn't see`
+    // Override beforeEach test setup
+    await page.addInitScript(
+      async ({ settingsKey, settings, badCode }) => {
+        localStorage.setItem('persistCode', badCode)
+        localStorage.setItem(settingsKey, settings)
+      },
+      {
+        settingsKey: TEST_SETTINGS_KEY,
+        settings: TOML.stringify({
+          settings: TEST_SETTINGS_ONBOARDING_PARAMETRIC_MODELING,
+        }),
+        badCode,
+      }
+    )
+
+    await page.setViewportSize({ width: 1200, height: 1080 })
+    await page.goto('/')
+    await page.waitForURL('**' + onboardingPaths.PARAMETRIC_MODELING, {
+      waitUntil: 'domcontentloaded',
+    })
+
+    const bracketNoNewLines = bracket.replace(/\n/g, '')
+
+    // Check the code got reset on load
+    await expect(page.locator('#code-pane')).toBeVisible()
+    await expect(u.codeLocator).toHaveText(bracketNoNewLines, {
+      timeout: 10_000,
+    })
+
+    // Mess with the code again
+    await u.codeLocator.selectText()
+    await u.codeLocator.fill(badCode)
+    await expect(u.codeLocator).toHaveText(badCode)
+
+    // Click to the next step
+    await page.locator('[data-testid="onboarding-next"]').click()
+    await page.waitForURL('**' + onboardingPaths.INTERACTIVE_NUMBERS, {
+      waitUntil: 'domcontentloaded',
+    })
+
+    // Check that the code has been reset
+    await expect(u.codeLocator).toHaveText(bracketNoNewLines)
+  })
+
   test('Avatar text updates depending on image load success', async ({
     page,
   }) => {
@@ -5489,72 +5545,3 @@ test('Paste should not work unless an input is focused', async ({
     )
   ).toContain(pasteContent)
 })
-
-test('Core dump from keyboard commands success', async ({ page }) => {
-  // This test can run long if it takes a little too long to load
-  // the engine, plus coredump takes bit to process.
-  test.setTimeout(150000)
-  const u = await getUtils(page)
-  await page.addInitScript(async () => {
-    ;(window as any).playwrightSkipFilePicker = true
-    localStorage.setItem(
-      'persistCode',
-      `const topAng = 25
-const bottomAng = 35
-const baseLen = 3.5
-const baseHeight = 1
-const totalHeightHalf = 2
-const armThick = 0.5
-const totalLen = 9.5
-const part001 = startSketchOn('-XZ')
-  |> startProfileAt([0, 0], %)
-  |> yLine(baseHeight, %)
-  |> xLine(baseLen, %)
-  |> angledLineToY({
-        angle: topAng,
-        to: totalHeightHalf,
-      }, %, 'seg04')
-  |> xLineTo(totalLen, %, 'seg03')
-  |> yLine(-armThick, %, 'seg01')
-  |> angledLineThatIntersects({
-        angle: HALF_TURN,
-        offset: -armThick,
-        intersectTag: 'seg04'
-      }, %)
-  |> angledLineToY([segAng('seg04', %) + 180, ZERO], %)
-  |> angledLineToY({
-        angle: -bottomAng,
-        to: -totalHeightHalf - armThick,
-      }, %, 'seg02')
-  |> xLineTo(segEndX('seg03', %) + 0, %)
-  |> yLine(-segLen('seg01', %), %)
-  |> angledLineThatIntersects({
-        angle: HALF_TURN,
-        offset: -armThick,
-        intersectTag: 'seg02'
-      }, %)
-  |> angledLineToY([segAng('seg02', %) + 180, -baseHeight], %)
-  |> xLineTo(ZERO, %)
-  |> close(%)
-  |> extrude(4, %)`
-    )
-  })
-  await page.setViewportSize({ width: 1200, height: 500 })
-  await page.goto('/')
-  await u.waitForAuthSkipAppStart()
-  await u.openDebugPanel()
-  await u.expectCmdLog('[data-message-type="execution-done"]')
-
-  // Start waiting for popup before clicking. Note no await.
-  const popupPromise = page.waitForEvent('popup')
-  await page.keyboard.press('Meta+Shift+.')
-  // after invoking coredump, a loading toast will appear
-  await expect(page.getByText('Starting core dump')).toBeVisible()
-  // Allow time for core dump processing
-  await page.waitForTimeout(1000)
-  await expect(page.getByText('Core dump completed successfully')).toBeVisible()
-  const popup = await popupPromise
-  console.log(await popup.title())
-  // GitHub popup will go to unlogged in page. Can't look for "New Issue" here.
-  await expect(popup).toHaveTitle(/GitHub /)
-})

e2e/playwright/storageStates.ts

@@ -33,6 +33,14 @@ export const TEST_SETTINGS_ONBOARDING_EXPORT = {
   app: { ...TEST_SETTINGS.app, onboardingStatus: onboardingPaths.EXPORT },
 } satisfies Partial<SaveSettingsPayload>
 
+export const TEST_SETTINGS_ONBOARDING_PARAMETRIC_MODELING = {
+  ...TEST_SETTINGS,
+  app: {
+    ...TEST_SETTINGS.app,
+    onboardingStatus: onboardingPaths.PARAMETRIC_MODELING,
+  },
+} satisfies Partial<SaveSettingsPayload>
+
 export const TEST_SETTINGS_ONBOARDING_START = {
   ...TEST_SETTINGS,
   app: { ...TEST_SETTINGS.app, onboardingStatus: '' },

src/lang/codeManager.ts

@@ -68,7 +68,9 @@ export default class CodeManager {
     this._currentFilePath = path
   }
 
-  // This updates the code state and calls the updateState function.
+  /**
+   * This updates the code state and calls the updateState function.
+   */
   updateCodeState(code: string): void {
     if (this._code !== code) {
       this.code = code
@@ -76,7 +78,9 @@ export default class CodeManager {
     }
   }
 
-  // Update the code in the editor.
+  /**
+   * Update the code in the editor.
+   */
   updateCodeEditor(code: string): void {
     this.code = code
     if (editorManager.editorView) {
@@ -90,7 +94,9 @@ export default class CodeManager {
     }
   }
 
-  // Update the code, state, and the code the code mirror editor sees.
+  /**
+   * Update the code, state, and the code the code mirror editor sees.
+   */
   updateCodeStateEditor(code: string): void {
     if (this._code !== code) {
       this.code = code

src/routes/Onboarding/CodeEditor.tsx

@@ -1,8 +1,9 @@
-import { OnboardingButtons, useDismiss, useNextClick } from '.'
+import { OnboardingButtons, useDemoCode, useDismiss, useNextClick } from '.'
 import { onboardingPaths } from 'routes/Onboarding/paths'
 import { useStore } from '../../useStore'
 
-export default function CodeEditor() {
+export default function OnboardingCodeEditor() {
+  useDemoCode()
   const { buttonDownInStream } = useStore((s) => ({
     buttonDownInStream: s.buttonDownInStream,
   }))

src/routes/Onboarding/FutureWork.tsx

@@ -1,24 +1,19 @@
-import { OnboardingButtons, useDismiss } from '.'
+import { OnboardingButtons, useDemoCode, useDismiss } from '.'
 import { useEffect } from 'react'
-import { bracket } from 'lib/exampleKcl'
-import { codeManager, kclManager } from 'lib/singletons'
 import { useModelingContext } from 'hooks/useModelingContext'
 import { APP_NAME } from 'lib/constants'
 import { onboardingPaths } from './paths'
+import { sceneInfra } from 'lib/singletons'
 
 export default function FutureWork() {
   const { send } = useModelingContext()
   const dismiss = useDismiss()
 
+  // Reset the code, the camera, and the modeling state
+  useDemoCode()
   useEffect(() => {
-    // We do want to update both the state and editor here.
-    codeManager.updateCodeEditor(bracket)
-    if (kclManager.engineCommandManager.engineConnection?.isReady()) {
-      // If the engine is ready, promptly execute the loaded code
-      kclManager.executeCode(true, true)
-    }
-
     send({ type: 'Cancel' }) // in case the user hit 'Next' while still in sketch mode
+    sceneInfra.camControls.resetCameraPosition()
   }, [send])
 
   return (

src/routes/Onboarding/InteractiveNumbers.tsx

@@ -1,9 +1,16 @@
-import { OnboardingButtons, kbdClasses, useDismiss, useNextClick } from '.'
+import {
+  OnboardingButtons,
+  kbdClasses,
+  useDemoCode,
+  useDismiss,
+  useNextClick,
+} from '.'
 import { onboardingPaths } from 'routes/Onboarding/paths'
 import { useStore } from '../../useStore'
 import { bracketWidthConstantLine } from 'lib/exampleKcl'
 
-export default function InteractiveNumbers() {
+export default function OnboardingInteractiveNumbers() {
+  useDemoCode()
   const { buttonDownInStream } = useStore((s) => ({
     buttonDownInStream: s.buttonDownInStream,
   }))
@@ -33,8 +40,10 @@ export default function InteractiveNumbers() {
               <kbd className={kbdClasses}>Option</kbd>) key
             </li>
             <li>
-              Hover over the number assigned to <code>width</code> on line{' '}
-              {bracketWidthConstantLine}
+              Hover over the number assigned to "width" on{' '}
+              <em>
+                <strong>line {bracketWidthConstantLine}</strong>
+              </em>
             </li>
             <li>Drag the number left and right to change its value</li>
           </ol>

src/routes/Onboarding/Introduction.tsx

@@ -1,4 +1,4 @@
-import { OnboardingButtons, useDismiss, useNextClick } from '.'
+import { OnboardingButtons, useDemoCode, useDismiss, useNextClick } from '.'
 import { onboardingPaths } from 'routes/Onboarding/paths'
 import { useSettingsAuthContext } from 'hooks/useSettingsAuthContext'
 import { Themes, getSystemTheme } from 'lib/theme'
@@ -10,7 +10,6 @@ import {
 import { isTauri } from 'lib/isTauri'
 import { useNavigate } from 'react-router-dom'
 import { paths } from 'lib/paths'
-import { useEffect } from 'react'
 import { codeManager, kclManager } from 'lib/singletons'
 import { join } from '@tauri-apps/api/path'
 import {
@@ -92,7 +91,7 @@ function OnboardingWithNewFile() {
   )
 }
 
-export default function Introduction() {
+export default function OnboardingIntroduction() {
   const {
     settings: {
       state: {
@@ -112,9 +111,7 @@ export default function Introduction() {
   const currentCode = codeManager.code
   const isStarterCode = currentCode === '' || currentCode === bracket
 
-  useEffect(() => {
-    if (codeManager.code === '') codeManager.updateCodeEditor(bracket)
-  }, [])
+  useDemoCode()
 
   return isStarterCode ? (
     <div className="fixed inset-0 z-50 grid place-content-center bg-chalkboard-110/50">
@@ -159,6 +156,12 @@ export default function Introduction() {
             ! We are trying to release as early as possible to get feedback from
             users like you.
           </p>
+          <p>
+            As you go through the onboarding, we'll be changing and resetting
+            your code occasionally, so that we can reference specific code
+            features. So hold off on writing production KCL code until you're
+            done with the onboarding 😉
+          </p>
         </section>
         <OnboardingButtons
           currentSlug={onboardingPaths.INDEX}

src/routes/Onboarding/ParametricModeling.tsx

@@ -1,11 +1,12 @@
-import { OnboardingButtons, useDismiss, useNextClick } from '.'
+import { OnboardingButtons, useDemoCode, useDismiss, useNextClick } from '.'
 import { onboardingPaths } from 'routes/Onboarding/paths'
 import { useStore } from '../../useStore'
 import { Themes, getSystemTheme } from 'lib/theme'
 import { useSettingsAuthContext } from 'hooks/useSettingsAuthContext'
 import { bracketThicknessCalculationLine } from 'lib/exampleKcl'
 
-export default function ParametricModeling() {
+export default function OnboardingParametricModeling() {
+  useDemoCode()
   const { buttonDownInStream } = useStore((s) => ({
     buttonDownInStream: s.buttonDownInStream,
   }))
@@ -44,8 +45,10 @@ export default function ParametricModeling() {
 
           <p className="my-4">
             We've received this sketch from a designer highlighting an{' '}
-            <em className="text-primary">aluminum bracket</em> they need for
-            this shelf:
+            <em>
+              <strong>aluminum bracket</strong>
+            </em>{' '}
+            they need for this shelf:
           </p>
           <figure className="my-4 w-2/3 mx-auto">
             <img
@@ -59,8 +62,8 @@ export default function ParametricModeling() {
           <p className="my-4">
             We are able to easily calculate the thickness of the material based
             on the width of the bracket to meet a set safety factor on{' '}
-            <em className="text-primary">
-              line {bracketThicknessCalculationLine}
+            <em>
+              <strong>line {bracketThicknessCalculationLine}</strong>
             </em>
             .
           </p>

src/routes/Onboarding/index.tsx

@@ -19,9 +19,11 @@ import { paths } from 'lib/paths'
 import { useAbsoluteFilePath } from 'hooks/useAbsoluteFilePath'
 import { ActionButton } from 'components/ActionButton'
 import { onboardingPaths } from 'routes/Onboarding/paths'
+import { codeManager, editorManager } from 'lib/singletons'
+import { bracket } from 'lib/exampleKcl'
 
 export const kbdClasses =
-  'p-0.5 text-sm rounded-sm bg-chalkboard-10 dark:bg-chalkboard-100 border border-chalkboard-50'
+  'py-0.5 px-1 text-sm rounded bg-chalkboard-10 dark:bg-chalkboard-100 border border-chalkboard-50 border-b-2'
 
 export const onboardingRoutes = [
   {
@@ -75,6 +77,13 @@ export const onboardingRoutes = [
   },
 ]
 
+export function useDemoCode() {
+  useEffect(() => {
+    if (!editorManager.editorView) return
+    setTimeout(() => codeManager.updateCodeStateEditor(bracket))
+  }, [editorManager.editorView, codeManager.updateCodeStateEditor])
+}
+
 export function useNextClick(newStatus: string) {
   const filePath = useAbsoluteFilePath()
   const {