From 4d5f3a3c9d5cbabcf72ca0f68c918ab83624d11d Mon Sep 17 00:00:00 2001 From: Pierre Jacquier Date: Wed, 25 Jun 2025 12:59:39 -0400 Subject: [PATCH] WIP improving helix flows and fixing tests --- e2e/playwright/point-click.spec.ts | 28 ++++++------------- src/lang/modifyAst.ts | 7 +++-- .../modelingCommandConfig.ts | 25 ++++++++--------- 3 files changed, 25 insertions(+), 35 deletions(-) diff --git a/e2e/playwright/point-click.spec.ts b/e2e/playwright/point-click.spec.ts index 9841ce3bf..541a4cd2b 100644 --- a/e2e/playwright/point-click.spec.ts +++ b/e2e/playwright/point-click.spec.ts @@ -1151,8 +1151,7 @@ openSketch = startSketchOn(XY) cmdBar, }) => { // One dumb hardcoded screen pixel value - const testPoint = { x: 620, y: 257 } - const expectedOutput = `helix001 = helix( axis = X, radius = 5, length = 5, revolutions = 1, angleStart = 270, ccw = false,)` + const expectedOutput = `helix001 = helix( axis = X, radius = 5, length = 5, revolutions = 1, angleStart = 270,)` const expectedLine = `axis=X,` await homePage.goToModelingScene() @@ -1169,7 +1168,6 @@ openSketch = startSketchOn(XY) AngleStart: '', Revolutions: '', Radius: '', - CounterClockWise: '', }, highlightedHeaderArg: 'mode', commandName: 'Helix', @@ -1190,7 +1188,6 @@ openSketch = startSketchOn(XY) AngleStart: '', Length: '', Radius: '', - CounterClockWise: '', }, commandName: 'Helix', }) @@ -1207,11 +1204,10 @@ openSketch = startSketchOn(XY) Revolutions: '1', Length: '5', Radius: '5', - CounterClockWise: '', }, commandName: 'Helix', }) - await cmdBar.progressCmdBar() + await cmdBar.submit() }) await test.step(`Confirm code is added to the editor, scene has changed`, async () => { @@ -1221,8 +1217,6 @@ openSketch = startSketchOn(XY) activeLines: [expectedLine], highlightedCode: '', }) - // Red plane is now gone, white helix is there - await scene.expectPixelColor([250, 250, 250], testPoint, 15) }) await test.step(`Edit helix through the feature tree`, async () => { @@ -1234,21 +1228,18 @@ openSketch = startSketchOn(XY) await cmdBar.expectState({ commandName: 'Helix', stage: 'arguments', - currentArgKey: 'CounterClockWise', - currentArgValue: '', + currentArgKey: 'length', + currentArgValue: '5', headerArguments: { Axis: 'X', AngleStart: '270', Revolutions: '1', Radius: '5', Length: initialInput, - CounterClockWise: '', }, - highlightedHeaderArg: 'CounterClockWise', + highlightedHeaderArg: 'length', }) - await page.keyboard.press('Shift+Backspace') - await expect(cmdBar.currentArgumentInput).toBeVisible() - await cmdBar.currentArgumentInput.locator('.cm-content').fill(newInput) + await page.keyboard.insertText(newInput) await cmdBar.progressCmdBar() await cmdBar.expectState({ stage: 'review', @@ -1258,11 +1249,10 @@ openSketch = startSketchOn(XY) Revolutions: '1', Radius: '5', Length: newInput, - CounterClockWise: '', }, commandName: 'Helix', }) - await cmdBar.progressCmdBar() + await cmdBar.submit() await toolbar.closeFeatureTreePane() await editor.openPane() await editor.expectEditor.toContain('length = ' + newInput) @@ -1273,8 +1263,8 @@ openSketch = startSketchOn(XY) const operationButton = await toolbar.getFeatureTreeOperation('Helix', 0) await operationButton.click({ button: 'left' }) await page.keyboard.press('Delete') - // Red plane is back - await scene.expectPixelColor([96, 52, 52], testPoint, 15) + await scene.settled(cmdBar) + await editor.expectEditor.not.toContain('helix') }) }) diff --git a/src/lang/modifyAst.ts b/src/lang/modifyAst.ts index 13d4b7825..2665e009d 100644 --- a/src/lang/modifyAst.ts +++ b/src/lang/modifyAst.ts @@ -590,7 +590,7 @@ export function addHelix({ angleStart: Expr radius?: Expr length?: Expr - ccw: boolean + ccw?: boolean insertIndex?: number variableName?: string }): { modifiedAst: Node; pathToNode: PathToNode } { @@ -610,6 +610,9 @@ export function addHelix({ ) } + // Extra labeled args expressions + const ccwExpr = ccw ? [createLabeledArg('ccw', createLiteral(ccw))] : [] + const variable = createVariableDeclaration( name, createCallExpressionStdLibKw( @@ -619,7 +622,7 @@ export function addHelix({ ...modeArgs, createLabeledArg('revolutions', revolutions), createLabeledArg('angleStart', angleStart), - createLabeledArg('ccw', createLiteral(ccw)), + ...ccwExpr, ] ) ) diff --git a/src/lib/commandBarConfigs/modelingCommandConfig.ts b/src/lib/commandBarConfigs/modelingCommandConfig.ts index 86023afc8..78a21a6bb 100644 --- a/src/lib/commandBarConfigs/modelingCommandConfig.ts +++ b/src/lib/commandBarConfigs/modelingCommandConfig.ts @@ -690,14 +690,15 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig< }, axis: { inputType: 'options', - required: (commandContext) => - ['Axis'].includes(commandContext.argumentsToSubmit.mode as string), options: [ { name: 'X Axis', value: 'X' }, { name: 'Y Axis', value: 'Y' }, { name: 'Z Axis', value: 'Z' }, ], - hidden: false, // for consistency here, we can actually edit here since it's not a selection + required: (context) => + ['Axis'].includes(context.argumentsToSubmit.mode as string), + hidden: (context) => + !['Axis'].includes(context.argumentsToSubmit.mode as string), }, edge: { inputType: 'selection', @@ -732,34 +733,30 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig< radius: { inputType: 'kcl', defaultValue: KCL_DEFAULT_LENGTH, - required: (commandContext) => - !['Cylinder'].includes( - commandContext.argumentsToSubmit.mode as string - ), + required: (context) => + !['Cylinder'].includes(context.argumentsToSubmit.mode as string), + hidden: (context) => + ['Cylinder'].includes(context.argumentsToSubmit.mode as string), }, length: { inputType: 'kcl', defaultValue: KCL_DEFAULT_LENGTH, required: (commandContext) => ['Axis'].includes(commandContext.argumentsToSubmit.mode as string), + // No need for hidden here, as it works with all modes }, ccw: { inputType: 'options', - skip: true, - required: true, - defaultValue: false, - valueSummary: (value) => String(value), + required: false, displayName: 'CounterClockWise', - options: (commandContext) => [ + options: [ { name: 'False', value: false, - isCurrent: !Boolean(commandContext.argumentsToSubmit.ccw), }, { name: 'True', value: true, - isCurrent: Boolean(commandContext.argumentsToSubmit.ccw), }, ], },