diff --git a/e2e/playwright/point-click.spec.ts b/e2e/playwright/point-click.spec.ts index 1bca6e7086d..a7cae4896d1 100644 --- a/e2e/playwright/point-click.spec.ts +++ b/e2e/playwright/point-click.spec.ts @@ -1773,7 +1773,7 @@ extrude001 = extrude(sketch001, length = -12) const filletColor: [number, number, number] = [127, 127, 127] const backgroundColor: [number, number, number] = [30, 30, 30] const lowTolerance = 20 - const highTolerance = 40 + const highTolerance = 70 // TODO: understand why I needed that for edgeColorYellow on macos (local) // Setup await test.step(`Initial test setup`, async () => { @@ -1860,6 +1860,54 @@ extrude001 = extrude(sketch001, length = -12) await scene.expectPixelColor(filletColor, firstEdgeLocation, lowTolerance) }) + // Test 1.1: Edit fillet (segment type) + async function editFillet( + featureTreeIndex: number, + oldValue: string, + newValue: string + ) { + await toolbar.openPane('feature-tree') + const operationButton = await toolbar.getFeatureTreeOperation( + 'Fillet', + featureTreeIndex + ) + await operationButton.dblclick({ button: 'left' }) + await cmdBar.expectState({ + commandName: 'Fillet', + currentArgKey: 'radius', + currentArgValue: oldValue, + headerArguments: { + Radius: oldValue, + }, + highlightedHeaderArg: 'radius', + stage: 'arguments', + }) + await page.keyboard.insertText(newValue) + await cmdBar.progressCmdBar() + await cmdBar.expectState({ + stage: 'review', + headerArguments: { + Radius: newValue, + }, + commandName: 'Fillet', + }) + await cmdBar.progressCmdBar() + await toolbar.closePane('feature-tree') + } + + await test.step('Edit fillet via feature tree selection works', async () => { + const firstFilletFeatureTreeIndex = 0 + const editedRadius = '1' + await editFillet(firstFilletFeatureTreeIndex, '5', editedRadius) + await editor.expectEditor.toContain( + firstFilletDeclaration.replace('radius = 5', 'radius = ' + editedRadius) + ) + + // Edit back to original radius + await editFillet(firstFilletFeatureTreeIndex, editedRadius, '5') + await editor.expectEditor.toContain(firstFilletDeclaration) + }) + // Test 2: Command bar flow without preselected edges await test.step(`Open fillet UI without selecting edges`, async () => { await page.waitForTimeout(100) @@ -1944,6 +1992,23 @@ extrude001 = extrude(sketch001, length = -12) ) }) + // Test 2.1: Edit fillet (edgeSweep type) + await test.step('Edit fillet via feature tree selection works', async () => { + const secondFilletFeatureTreeIndex = 1 + const editedRadius = '2' + await editFillet(secondFilletFeatureTreeIndex, '5', editedRadius) + await editor.expectEditor.toContain( + secondFilletDeclaration.replace( + 'radius = 5', + 'radius = ' + editedRadius + ) + ) + + // Edit back to original radius + await editFillet(secondFilletFeatureTreeIndex, editedRadius, '5') + await editor.expectEditor.toContain(secondFilletDeclaration) + }) + // Test 3: Delete fillets await test.step('Delete fillet via feature tree selection', async () => { await test.step('Open Feature Tree Pane', async () => { @@ -1966,6 +2031,43 @@ extrude001 = extrude(sketch001, length = -12) }) }) + test(`Fillet point-and-click edit rejected when not in pipe`, async ({ + context, + page, + homePage, + scene, + toolbar, + }) => { + const initialCode = `sketch001 = startSketchOn(XY) +profile001 = circle( + sketch001, + center = [0, 0], + radius = 100, + tag = $seg01, +) +extrude001 = extrude(profile001, length = 100) +fillet001 = fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg01)]) +` + await context.addInitScript((initialCode) => { + localStorage.setItem('persistCode', initialCode) + }, initialCode) + await page.setBodyDimensions({ width: 1000, height: 500 }) + await homePage.goToModelingScene() + await scene.waitForExecutionDone() + + await test.step('Double-click in feature tree and expect error toast', async () => { + await toolbar.openPane('feature-tree') + const operationButton = await toolbar.getFeatureTreeOperation('Fillet', 0) + await operationButton.dblclick({ button: 'left' }) + await expect( + page.getByText( + 'Only chamfer and fillet in pipe expressions are supported for edits' + ) + ).toBeVisible() + await page.waitForTimeout(1000) + }) + }) + test(`Fillet point-and-click delete`, async ({ context, page, @@ -2262,7 +2364,7 @@ extrude001 = extrude(sketch001, length = -12) const chamferColor: [number, number, number] = [168, 168, 168] const backgroundColor: [number, number, number] = [30, 30, 30] const lowTolerance = 20 - const highTolerance = 40 + const highTolerance = 70 // TODO: understand why I needed that for edgeColorYellow on macos (local) // Setup await test.step(`Initial test setup`, async () => { @@ -2344,6 +2446,57 @@ extrude001 = extrude(sketch001, length = -12) ) }) + // Test 1.1: Edit sweep + async function editChamfer( + featureTreeIndex: number, + oldValue: string, + newValue: string + ) { + await toolbar.openPane('feature-tree') + const operationButton = await toolbar.getFeatureTreeOperation( + 'Chamfer', + featureTreeIndex + ) + await operationButton.dblclick({ button: 'left' }) + await cmdBar.expectState({ + commandName: 'Chamfer', + currentArgKey: 'length', + currentArgValue: oldValue, + headerArguments: { + Length: oldValue, + }, + highlightedHeaderArg: 'length', + stage: 'arguments', + }) + await page.keyboard.insertText(newValue) + await cmdBar.progressCmdBar() + await cmdBar.expectState({ + stage: 'review', + headerArguments: { + Length: newValue, + }, + commandName: 'Chamfer', + }) + await cmdBar.progressCmdBar() + await toolbar.closePane('feature-tree') + } + + await test.step('Edit chamfer via feature tree selection works', async () => { + const firstChamferFeatureTreeIndex = 0 + const editedLength = '1' + await editChamfer(firstChamferFeatureTreeIndex, '5', editedLength) + await editor.expectEditor.toContain( + firstChamferDeclaration.replace( + 'length = 5', + 'length = ' + editedLength + ) + ) + + // Edit back to original radius + await editChamfer(firstChamferFeatureTreeIndex, editedLength, '5') + await editor.expectEditor.toContain(firstChamferDeclaration) + }) + // Test 2: Command bar flow without preselected edges await test.step(`Open chamfer UI without selecting edges`, async () => { await page.waitForTimeout(100) @@ -2428,6 +2581,23 @@ extrude001 = extrude(sketch001, length = -12) ) }) + // Test 2.1: Edit chamfer (edgeSweep type) + await test.step('Edit chamfer via feature tree selection works', async () => { + const secondChamferFeatureTreeIndex = 1 + const editedLength = '2' + await editChamfer(secondChamferFeatureTreeIndex, '5', editedLength) + await editor.expectEditor.toContain( + secondChamferDeclaration.replace( + 'length = 5', + 'length = ' + editedLength + ) + ) + + // Edit back to original length + await editChamfer(secondChamferFeatureTreeIndex, editedLength, '5') + await editor.expectEditor.toContain(secondChamferDeclaration) + }) + // Test 3: Delete chamfer via feature tree selection await test.step('Open Feature Tree Pane', async () => { await toolbar.openPane('feature-tree') diff --git a/e2e/playwright/snapshot-tests.spec.ts-snapshots/Draft-rectangles-should-look-right-1-Google-Chrome-linux.png b/e2e/playwright/snapshot-tests.spec.ts-snapshots/Draft-rectangles-should-look-right-1-Google-Chrome-linux.png index aa9fb641699..c07ca3f29c9 100644 Binary files a/e2e/playwright/snapshot-tests.spec.ts-snapshots/Draft-rectangles-should-look-right-1-Google-Chrome-linux.png and b/e2e/playwright/snapshot-tests.spec.ts-snapshots/Draft-rectangles-should-look-right-1-Google-Chrome-linux.png differ diff --git a/e2e/playwright/snapshot-tests.spec.ts-snapshots/extrude-on-default-planes-should-be-stable--YZ-1-Google-Chrome-linux.png b/e2e/playwright/snapshot-tests.spec.ts-snapshots/extrude-on-default-planes-should-be-stable--YZ-1-Google-Chrome-linux.png index c984657553a..0716e7ae6c5 100644 Binary files a/e2e/playwright/snapshot-tests.spec.ts-snapshots/extrude-on-default-planes-should-be-stable--YZ-1-Google-Chrome-linux.png and b/e2e/playwright/snapshot-tests.spec.ts-snapshots/extrude-on-default-planes-should-be-stable--YZ-1-Google-Chrome-linux.png differ diff --git a/e2e/playwright/snapshot-tests.spec.ts-snapshots/extrude-on-default-planes-should-be-stable-XY-1-Google-Chrome-linux.png b/e2e/playwright/snapshot-tests.spec.ts-snapshots/extrude-on-default-planes-should-be-stable-XY-1-Google-Chrome-linux.png index ae8c2b2cd28..9b2ad83cda3 100644 Binary files a/e2e/playwright/snapshot-tests.spec.ts-snapshots/extrude-on-default-planes-should-be-stable-XY-1-Google-Chrome-linux.png and b/e2e/playwright/snapshot-tests.spec.ts-snapshots/extrude-on-default-planes-should-be-stable-XY-1-Google-Chrome-linux.png differ diff --git a/src/lang/modifyAst.ts b/src/lang/modifyAst.ts index f6197dd497f..1ea3a469328 100644 --- a/src/lang/modifyAst.ts +++ b/src/lang/modifyAst.ts @@ -73,7 +73,10 @@ import { } from './std/artifactGraph' import { BodyItem } from '@rust/kcl-lib/bindings/BodyItem' import { findKwArg } from './util' -import { deleteEdgeTreatment } from './modifyAst/addEdgeTreatment' +import { + deleteEdgeTreatment, + locateExtrudeDeclarator, +} from './modifyAst/addEdgeTreatment' import { Name } from '@rust/kcl-lib/bindings/Name' export function startSketchOnDefault( @@ -1890,6 +1893,27 @@ export async function deleteFromSelection( return new Error('Selection not recognised, could not delete') } +export function deleteNodeInExtrudePipe( + node: PathToNode, + ast: Node +): Error | void { + const pipeIndex = node.findIndex(([_, type]) => type === 'PipeExpression') + 1 + if (!(node[pipeIndex][0] && typeof node[pipeIndex][0] === 'number')) { + return new Error("Couldn't find node to delete in ast") + } + + const lookup = locateExtrudeDeclarator(ast, node) + if (err(lookup)) { + return lookup + } + + if (lookup.extrudeDeclarator.init.type !== 'PipeExpression') { + return new Error("Couldn't find node to delete in looked up extrusion") + } + + lookup.extrudeDeclarator.init.body.splice(node[pipeIndex][0], 1) +} + export const nonCodeMetaEmpty = () => { return { nonCodeNodes: {}, startNodes: [], start: 0, end: 0 } } diff --git a/src/lib/commandBarConfigs/modelingCommandConfig.ts b/src/lib/commandBarConfigs/modelingCommandConfig.ts index a63d08ba319..a38c982ceaa 100644 --- a/src/lib/commandBarConfigs/modelingCommandConfig.ts +++ b/src/lib/commandBarConfigs/modelingCommandConfig.ts @@ -80,10 +80,16 @@ export type ModelingCommandSchema = { edge: Selections } Fillet: { + // Enables editing workflow + nodeToEdit?: PathToNode + // KCL stdlib arguments selection: Selections radius: KclCommandValue } Chamfer: { + // Enables editing workflow + nodeToEdit?: PathToNode + // KCL stdlib arguments selection: Selections length: KclCommandValue } @@ -607,14 +613,22 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig< status: 'development', needsReview: true, args: { + nodeToEdit: { + description: + 'Path to the node in the AST to edit. Never shown to the user.', + inputType: 'text', + required: false, + hidden: true, + }, selection: { inputType: 'selection', - selectionTypes: ['segment', 'sweepEdge', 'edgeCutEdge'], + selectionTypes: ['segment', 'sweepEdge'], multiple: true, required: true, skip: false, warningMessage: 'Fillets cannot touch other fillets yet. This is under development.', + hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit), }, radius: { inputType: 'kcl', @@ -629,14 +643,22 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig< status: 'development', needsReview: true, args: { + nodeToEdit: { + description: + 'Path to the node in the AST to edit. Never shown to the user.', + inputType: 'text', + required: false, + hidden: true, + }, selection: { inputType: 'selection', - selectionTypes: ['segment', 'sweepEdge', 'edgeCutEdge'], + selectionTypes: ['segment', 'sweepEdge'], multiple: true, required: true, skip: false, warningMessage: 'Chamfers cannot touch other chamfers yet. This is under development.', + hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit), }, length: { inputType: 'kcl', diff --git a/src/lib/operations.ts b/src/lib/operations.ts index fa12e66a2fa..307f885dd72 100644 --- a/src/lib/operations.ts +++ b/src/lib/operations.ts @@ -3,6 +3,7 @@ import { Artifact, getArtifactOfTypes, getCapCodeRef, + getEdgeCutConsumedCodeRef, getSweepEdgeCodeRef, } from 'lang/std/artifactGraph' import { Operation } from '@rust/kcl-lib/bindings/Operation' @@ -122,6 +123,121 @@ const prepareToEditExtrude: PrepareToEditCallback = } } +/** + * Gather up the argument values for the Chamfer or Fillet command + * to be used in the command bar edit flow. + */ +const prepareToEditEdgeTreatment: PrepareToEditCallback = async ({ + operation, + artifact, +}) => { + const isChamfer = + artifact?.type === 'edgeCut' && artifact.subType === 'chamfer' + const isFillet = artifact?.type === 'edgeCut' && artifact.subType === 'fillet' + const baseCommand = { + name: isChamfer ? 'Chamfer' : 'Fillet', + groupId: 'modeling', + } + if ( + operation.type !== 'StdLibCall' || + !operation.labeledArgs || + (!isChamfer && !isFillet) + ) { + return { reason: 'Wrong operation type or artifact' } + } + + // Recreate the selection argument (artiface and codeRef) from what we have + const edgeArtifact = getArtifactOfTypes( + { + key: artifact.consumedEdgeId, + types: ['segment', 'sweepEdge'], + }, + engineCommandManager.artifactGraph + ) + if (err(edgeArtifact)) { + return { reason: "Couldn't find edge artifact" } + } + + let edgeCodeRef = getEdgeCutConsumedCodeRef( + artifact, + engineCommandManager.artifactGraph + ) + if (err(edgeCodeRef)) { + return { reason: "Couldn't find edge coderef" } + } + const selection = { + graphSelections: [ + { + artifact: edgeArtifact, + codeRef: edgeCodeRef, + }, + ], + otherSelections: [], + } + + // Assemble the default argument values for the Fillet command, + // with `nodeToEdit` set, which will let the Fillet actor know + // to edit the node that corresponds to the StdLibCall. + const nodeToEdit = getNodePathFromSourceRange( + kclManager.ast, + sourceRangeFromRust(operation.sourceRange) + ) + const isPipeExpression = nodeToEdit.some( + ([_, type]) => type === 'PipeExpression' + ) + if (!isPipeExpression) { + return { + reason: + 'Only chamfer and fillet in pipe expressions are supported for edits', + } + } + + let argDefaultValues: + | ModelingCommandSchema['Chamfer'] + | ModelingCommandSchema['Fillet'] + | undefined + + if (isChamfer) { + // Convert the length argument from a string to a KCL expression + const length = await stringToKclExpression( + codeManager.code.slice( + operation.labeledArgs?.['length']?.sourceRange[0], + operation.labeledArgs?.['length']?.sourceRange[1] + ) + ) + if (err(length) || 'errors' in length) { + return { reason: 'Error in length argument retrieval' } + } + + argDefaultValues = { + selection, + length, + nodeToEdit, + } + } else if (isFillet) { + const radius = await stringToKclExpression( + codeManager.code.slice( + operation.labeledArgs?.['radius']?.sourceRange[0], + operation.labeledArgs?.['radius']?.sourceRange[1] + ) + ) + if (err(radius) || 'errors' in radius) { + return { reason: 'Error in radius argument retrieval' } + } + + argDefaultValues = { + selection, + radius, + nodeToEdit, + } + } + + return { + ...baseCommand, + argDefaultValues, + } +} + /** * Gather up the argument values for the Shell command * to be used in the command bar edit flow. @@ -636,6 +752,7 @@ export const stdLibMap: Record = { chamfer: { label: 'Chamfer', icon: 'chamfer3d', + prepareToEdit: prepareToEditEdgeTreatment, // modelingEvent: 'Chamfer', }, extrude: { @@ -647,6 +764,7 @@ export const stdLibMap: Record = { fillet: { label: 'Fillet', icon: 'fillet3d', + prepareToEdit: prepareToEditEdgeTreatment, }, helix: { label: 'Helix', diff --git a/src/machines/modelingMachine.ts b/src/machines/modelingMachine.ts index 454d3d65cc4..b1e6c34505b 100644 --- a/src/machines/modelingMachine.ts +++ b/src/machines/modelingMachine.ts @@ -52,6 +52,7 @@ import { addSweep, createLiteral, createLocalName, + deleteNodeInExtrudePipe, extrudeSketch, insertNamedConstant, loftSketches, @@ -2335,7 +2336,14 @@ export const modelingMachine = setup({ // Extract inputs const ast = kclManager.ast - const { selection, radius } = input + const { nodeToEdit, selection, radius } = input + + // If this is an edit flow, first we're going to remove the old node + if (nodeToEdit) { + const oldNodeDeletion = deleteNodeInExtrudePipe(nodeToEdit, ast) + if (err(oldNodeDeletion)) return oldNodeDeletion + } + const parameters: FilletParameters = { type: EdgeTreatmentType.Fillet, radius, @@ -2490,7 +2498,14 @@ export const modelingMachine = setup({ // Extract inputs const ast = kclManager.ast - const { selection, length } = input + const { nodeToEdit, selection, length } = input + + // If this is an edit flow, first we're going to remove the old node + if (nodeToEdit) { + const oldNodeDeletion = deleteNodeInExtrudePipe(nodeToEdit, ast) + if (err(oldNodeDeletion)) return oldNodeDeletion + } + const parameters: ChamferParameters = { type: EdgeTreatmentType.Chamfer, length,