Update Katie's name reference and link in onboarding (#4967)

* [BUG] Shell point and click references the wrong feature
Fixes #4961

* Add test for sketch on face based on extrudes in pipe

* Add no extrude in pipe case

* Lint

* Add scene.waitForExecutionDone()

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* Trigger CI

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* Trigger CI

* Update src/lang/modifyAst/addShell.ts

Co-authored-by: Frank Noirot <frank@zoo.dev>

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Frank Noirot <frank@zoo.dev>

e2e/playwright/point-click.spec.ts

@@ -756,6 +756,17 @@ test(`Offset plane point-and-click`, async ({
     })
     await scene.expectPixelColor([74, 74, 74], testPoint, 15)
   })
+
+  await test.step('Delete offset plane via feature tree selection', async () => {
+    await editor.closePane()
+    const operationButton = await toolbar.getFeatureTreeOperation(
+      'Offset Plane',
+      0
+    )
+    await operationButton.click({ button: 'left' })
+    await page.keyboard.press('Backspace')
+    await scene.expectPixelColor([50, 51, 96], testPoint, 15)
+  })
 })
 
 const loftPointAndClickCases = [
@@ -851,6 +862,75 @@ loftPointAndClickCases.forEach(({ shouldPreselect }) => {
       })
       await scene.expectPixelColor([89, 89, 89], testPoint, 15)
     })
+
+    await test.step('Delete loft via feature tree selection', async () => {
+      await editor.closePane()
+      const operationButton = await toolbar.getFeatureTreeOperation('Loft', 0)
+      await operationButton.click({ button: 'left' })
+      await page.keyboard.press('Backspace')
+      await scene.expectPixelColor([254, 254, 254], testPoint, 15)
+    })
+  })
+})
+
+// TODO: merge with above test. Right now we're not able to delete a loft
+// right after creation via selection for some reason, so we go with a new instance
+test('Loft and offset plane deletion via selection', async ({
+  context,
+  page,
+  homePage,
+  scene,
+}) => {
+  const initialCode = `sketch001 = startSketchOn('XZ')
+  |> circle({ center = [0, 0], radius = 30 }, %)
+  plane001 = offsetPlane('XZ', 50)
+  sketch002 = startSketchOn(plane001)
+  |> circle({ center = [0, 0], radius = 20 }, %)
+loft001 = loft([sketch001, sketch002])
+`
+  await context.addInitScript((initialCode) => {
+    localStorage.setItem('persistCode', initialCode)
+  }, initialCode)
+  await page.setBodyDimensions({ width: 1000, height: 500 })
+  await homePage.goToModelingScene()
+
+  // One dumb hardcoded screen pixel value
+  const testPoint = { x: 575, y: 200 }
+  const [clickOnSketch1] = scene.makeMouseHelpers(testPoint.x, testPoint.y)
+  const [clickOnSketch2] = scene.makeMouseHelpers(testPoint.x, testPoint.y + 80)
+
+  await test.step(`Delete loft`, async () => {
+    // Check for loft
+    await scene.expectPixelColor([89, 89, 89], testPoint, 15)
+    await clickOnSketch1()
+    await expect(page.locator('.cm-activeLine')).toHaveText(`
+      |> circle({ center = [0, 0], radius = 30 }, %)
+    `)
+    await page.keyboard.press('Backspace')
+    // Check for sketch 1
+    await scene.expectPixelColor([254, 254, 254], testPoint, 15)
+  })
+
+  await test.step('Delete sketch002', async () => {
+    await page.waitForTimeout(1000)
+    await clickOnSketch2()
+    await expect(page.locator('.cm-activeLine')).toHaveText(`
+      |> circle({ center = [0, 0], radius = 20 }, %)
+    `)
+    await page.keyboard.press('Backspace')
+    // Check for plane001
+    await scene.expectPixelColor([228, 228, 228], testPoint, 15)
+  })
+
+  await test.step('Delete plane001', async () => {
+    await page.waitForTimeout(1000)
+    await clickOnSketch2()
+    await expect(page.locator('.cm-activeLine')).toHaveText(`
+      plane001 = offsetPlane('XZ', 50)
+    `)
+    await page.keyboard.press('Backspace')
+    // Check for sketch 1
+    await scene.expectPixelColor([254, 254, 254], testPoint, 15)
   })
 })
 
@@ -1030,4 +1110,102 @@ extrude001 = extrude(40, sketch001)
     })
     await scene.expectPixelColor([49, 49, 49], testPoint, 15)
   })
+
+  await test.step('Delete shell via feature tree selection', async () => {
+    await editor.closePane()
+    const operationButton = await toolbar.getFeatureTreeOperation('Shell', 0)
+    await operationButton.click({ button: 'left' })
+    await page.keyboard.press('Backspace')
+    await scene.expectPixelColor([99, 99, 99], testPoint, 15)
+  })
+})
+
+const shellSketchOnFacesCases = [
+  `sketch001 = startSketchOn('XZ')
+  |> circle({ center = [0, 0], radius = 100 }, %)
+  |> extrude(100, %)
+
+sketch002 = startSketchOn(sketch001, 'END')
+  |> circle({ center = [0, 0], radius = 50 }, %)
+  |> extrude(50, %)
+  `,
+  `sketch001 = startSketchOn('XZ')
+  |> circle({ center = [0, 0], radius = 100 }, %)
+extrude001 = extrude(100, sketch001)
+
+sketch002 = startSketchOn(extrude001, 'END')
+  |> circle({ center = [0, 0], radius = 50 }, %)
+extrude002 = extrude(50, sketch002)
+  `,
+]
+shellSketchOnFacesCases.forEach((initialCode, index) => {
+  const hasExtrudesInPipe = index === 0
+  test(`Shell point-and-click sketch on face (extrudes in pipes: ${hasExtrudesInPipe})`, async ({
+    context,
+    page,
+    homePage,
+    scene,
+    editor,
+    toolbar,
+    cmdBar,
+  }) => {
+    await context.addInitScript((initialCode) => {
+      localStorage.setItem('persistCode', initialCode)
+    }, initialCode)
+    await page.setBodyDimensions({ width: 1000, height: 500 })
+    await homePage.goToModelingScene()
+    await scene.waitForExecutionDone()
+
+    // One dumb hardcoded screen pixel value
+    const testPoint = { x: 550, y: 295 }
+    const [clickOnCap] = scene.makeMouseHelpers(testPoint.x, testPoint.y)
+    const shellDeclaration = `shell001 = shell({ faces = ['end'], thickness = 5 }, ${
+      hasExtrudesInPipe ? 'sketch002' : 'extrude002'
+    })`
+
+    await test.step(`Look for the grey of the shape`, async () => {
+      await toolbar.closePane('code')
+      await scene.expectPixelColor([128, 128, 128], testPoint, 15)
+    })
+
+    await test.step(`Go through the command bar flow, selecting a cap and keeping default thickness`, async () => {
+      await toolbar.shellButton.click()
+      await cmdBar.expectState({
+        stage: 'arguments',
+        currentArgKey: 'selection',
+        currentArgValue: '',
+        headerArguments: {
+          Selection: '',
+          Thickness: '',
+        },
+        highlightedHeaderArg: 'selection',
+        commandName: 'Shell',
+      })
+      await clickOnCap()
+      await cmdBar.progressCmdBar()
+      await page.waitForTimeout(500)
+      await cmdBar.progressCmdBar()
+      await cmdBar.expectState({
+        stage: 'review',
+        headerArguments: {
+          Selection: '1 cap',
+          Thickness: '5',
+        },
+        commandName: 'Shell',
+      })
+      await cmdBar.progressCmdBar()
+    })
+
+    await test.step(`Confirm code is added to the editor, scene has changed`, async () => {
+      await toolbar.openPane('code')
+      await editor.expectEditor.toContain(shellDeclaration)
+      await editor.expectState({
+        diagnostics: [],
+        activeLines: [shellDeclaration],
+        highlightedCode: '',
+      })
+      await toolbar.closePane('code')
+      await scene.expectPixelColor([73, 73, 73], testPoint, 15)
+    })
+  })
 })

package.json

@@ -26,7 +26,7 @@
     "@fortawesome/react-fontawesome": "^0.2.0",
     "@headlessui/react": "^1.7.19",
     "@headlessui/tailwindcss": "^0.2.0",
-    "@kittycad/lib": "2.0.12",
+    "@kittycad/lib": "2.0.13",
     "@lezer/highlight": "^1.2.1",
     "@lezer/lr": "^1.4.1",
     "@react-hook/resize-observer": "^2.0.1",

src/components/ModelingMachineProvider.tsx

@@ -271,6 +271,7 @@ export const ModelingMachineProvider = ({
               cmd_id: uuidv4(),
               cmd: {
                 type: 'default_camera_center_to_selection',
+                camera_movement: 'vantage',
               },
             })
             .catch(reportRejection)

src/lang/modifyAst.ts

@@ -1149,11 +1149,17 @@ export async function deleteFromSelection(
     ((selection?.artifact?.type === 'wall' ||
       selection?.artifact?.type === 'cap') &&
       varDec.node.init.type === 'PipeExpression') ||
-    selection.artifact?.type === 'sweep'
+    selection.artifact?.type === 'sweep' ||
+    selection.artifact?.type === 'plane' ||
+    !selection.artifact // aka expected to be a shell at this point
   ) {
     let extrudeNameToDelete = ''
     let pathToNode: PathToNode | null = null
-    if (selection.artifact?.type !== 'sweep') {
+    if (
+      selection.artifact &&
+      selection.artifact.type !== 'sweep' &&
+      selection.artifact.type !== 'plane'
+    ) {
       const varDecName = varDec.node.id.name
       traverse(astClone, {
         enter: (node, path) => {
@@ -1169,6 +1175,17 @@ export async function deleteFromSelection(
               pathToNode = path
               extrudeNameToDelete = dec.id.name
             }
+            if (
+              dec.init.type === 'CallExpression' &&
+              dec.init.callee.name === 'loft' &&
+              dec.init.arguments?.[0].type === 'ArrayExpression' &&
+              dec.init.arguments?.[0].elements.some(
+                (a) => a.type === 'Identifier' && a.name === varDecName
+              )
+            ) {
+              pathToNode = path
+              extrudeNameToDelete = dec.id.name
+            }
           }
         },
       })

src/lang/modifyAst/addShell.ts

@@ -49,17 +49,27 @@ export function addShell({
       return new Error("Couldn't find extrude")
     }
 
-    pathToExtrudeNode = extrudeLookupResult.pathToExtrudeNode
-    // Get the sketch ref from the selection
     // TODO: this assumes the segment is piped directly from the sketch, with no intermediate `VariableDeclarator` between.
     // We must find a technique for these situations that is robust to intermediate declarations
-    const sketchNode = getNodeFromPath<VariableDeclarator>(
+    const extrudeNode = getNodeFromPath<VariableDeclarator>(
       modifiedAst,
-      graphSelection.codeRef.pathToNode,
+      extrudeLookupResult.pathToExtrudeNode,
       'VariableDeclarator'
     )
-    if (err(sketchNode)) {
+    const segmentNode = getNodeFromPath<VariableDeclarator>(
-      return sketchNode
+      modifiedAst,
+      extrudeLookupResult.pathToSegmentNode,
+      'VariableDeclarator'
+    )
+    if (err(extrudeNode) || err(segmentNode)) {
+      return new Error("Couldn't find extrude")
+    }
+    if (extrudeNode.node.init.type === 'CallExpression') {
+      pathToExtrudeNode = extrudeLookupResult.pathToExtrudeNode
+    } else if (segmentNode.node.init.type === 'PipeExpression') {
+      pathToExtrudeNode = extrudeLookupResult.pathToSegmentNode
+    } else {
+      return new Error("Couldn't find extrude")
     }
 
     const selectedArtifact = graphSelection.artifact

src/lang/std/artifactGraph.ts

@@ -77,7 +77,7 @@ interface SegmentArtifactRich extends BaseArtifact {
 /** A Sweep is a more generic term for extrude, revolve, loft and sweep*/
 interface SweepArtifact extends BaseArtifact {
   type: 'sweep'
-  subType: 'extrusion' | 'revolve'
+  subType: 'extrusion' | 'revolve' | 'loft'
   pathId: string
   surfaceIds: Array<string>
   edgeIds: Array<string>
@@ -85,7 +85,7 @@ interface SweepArtifact extends BaseArtifact {
 }
 interface SweepArtifactRich extends BaseArtifact {
   type: 'sweep'
-  subType: 'extrusion' | 'revolve'
+  subType: 'extrusion' | 'revolve' | 'loft'
   path: PathArtifact
   surfaces: Array<WallArtifact | CapArtifact>
   edges: Array<SweepEdge>
@@ -398,6 +398,33 @@ export function getArtifactsToUpdate({
         artifact: { ...path, sweepId: id },
       })
     return returnArr
+  } else if (
+    cmd.type === 'loft' &&
+    response.type === 'modeling' &&
+    response.data.modeling_response.type === 'loft'
+  ) {
+    returnArr.push({
+      id,
+      artifact: {
+        type: 'sweep',
+        subType: 'loft',
+        id,
+        // TODO: make sure to revisit this choice, don't think it matters for now
+        pathId: cmd.section_ids[0],
+        surfaceIds: [],
+        edgeIds: [],
+        codeRef: { range, pathToNode },
+      },
+    })
+    for (const sectionId of cmd.section_ids) {
+      const path = getArtifact(sectionId)
+      if (path?.type === 'path')
+        returnArr.push({
+          id: sectionId,
+          artifact: { ...path, sweepId: id },
+        })
+    }
+    return returnArr
   } else if (
     cmd.type === 'solid3d_get_extrusion_face_info' &&
     response?.type === 'modeling' &&

src/routes/Onboarding/Export.tsx

@@ -41,13 +41,13 @@ export default function Export() {
             export to almost any CAD software.
           </p>
           <p className="my-4">
-            Our teammate David is working on the file format, check out{' '}
+            Our teammate Katie is working on the file format, check out{' '}
             <a
-              href="https://www.youtube.com/watch?v=8SuW0qkYCZo"
+              href="https://github.com/KhronosGroup/glTF/pull/2343"
               target="_blank"
               rel="noreferrer noopener"
             >
-              his talk with the Metaverse Standards Forum
+              her standards proposal on GitHub
             </a>
             !
           </p>

src/wasm-lib/kcl/src/std/loft.rs

@@ -156,5 +156,8 @@ async fn inner_loft(
     .await?;
 
     // Using the first sketch as the base curve, idk we might want to change this later.
-    do_post_extrude(sketches[0].clone(), 0.0, exec_state, args).await
+    let mut sketch = sketches[0].clone();
+    // Override its id with the loft id so we can get its faces later
+    sketch.id = id;
+    do_post_extrude(sketch, 0.0, exec_state, args).await
 }

yarn.lock

@@ -2010,10 +2010,10 @@
     "@jridgewell/resolve-uri" "^3.1.0"
     "@jridgewell/sourcemap-codec" "^1.4.14"
 
-"@kittycad/lib@2.0.12":
+"@kittycad/lib@2.0.13":
-  version "2.0.12"
+  version "2.0.13"
-  resolved "https://registry.yarnpkg.com/@kittycad/lib/-/lib-2.0.12.tgz#517be58ee8b5f59e5c89bb5076492c960b4ef7d8"
+  resolved "https://registry.yarnpkg.com/@kittycad/lib/-/lib-2.0.13.tgz#e20aa17847ab1359065d21bed143ea330cf545d1"
-  integrity sha512-1eXIP+JbFvWSWQe//ijBuhlnCLRUnZzNAiOf7oMI0WcRTTn8SD8A+TY+NgK6OVGG12unyTPCVXxRR4Xtm3ahLQ==
+  integrity sha512-wLn6/iRVdqbRCvf6t2FhNr8No6+I6elpCEVHGUexyHLoE+1XeUS1lHeapQqcfR0pEQiwtGpcKTDfUNSlmnmaFw==
   dependencies:
     openapi-types "^12.0.0"
     ts-node "^10.9.1"