diff --git a/e2e/playwright/prompt-to-edit-snapshot-tests.spec.ts b/e2e/playwright/prompt-to-edit-snapshot-tests.spec.ts index 9d4e328de..3ccf1206c 100644 --- a/e2e/playwright/prompt-to-edit-snapshot-tests.spec.ts +++ b/e2e/playwright/prompt-to-edit-snapshot-tests.spec.ts @@ -99,6 +99,8 @@ test.describe('edit with AI example snapshots', () => { await test.step('fire off edit prompt', async () => { await cmdBar.captureTextToCadRequestSnapshot(test.info()) await cmdBar.openCmdBar('promptToEdit') + await page.waitForTimeout(100) + await cmdBar.progressCmdBar() // being specific about the color with a hex means asserting pixel color is more stable await page .getByTestId('cmd-bar-arg-value') diff --git a/e2e/playwright/prompt-to-edit.spec.ts b/e2e/playwright/prompt-to-edit.spec.ts index fcb80166e..f16590c7f 100644 --- a/e2e/playwright/prompt-to-edit.spec.ts +++ b/e2e/playwright/prompt-to-edit.spec.ts @@ -88,6 +88,8 @@ test.describe('Prompt-to-edit tests', () => { await test.step('fire off edit prompt', async () => { await cmdBar.openCmdBar('promptToEdit') + await page.waitForTimeout(100) + await cmdBar.progressCmdBar() // being specific about the color with a hex means asserting pixel color is more stable await page .getByTestId('cmd-bar-arg-value') @@ -165,6 +167,8 @@ test.describe('Prompt-to-edit tests', () => { await test.step('fire of bad prompt', async () => { await cmdBar.openCmdBar('promptToEdit') + await page.waitForTimeout(100) + await cmdBar.progressCmdBar() await page .getByTestId('cmd-bar-arg-value') .fill('ansheusha asnthuatshoeuhtaoetuhthaeu laughs in dvorak') diff --git a/src/components/CommandBar/CommandBarHeader.tsx b/src/components/CommandBar/CommandBarHeader.tsx index 12c67676e..d3add0321 100644 --- a/src/components/CommandBar/CommandBarHeader.tsx +++ b/src/components/CommandBar/CommandBarHeader.tsx @@ -102,10 +102,12 @@ function CommandBarHeader({ children }: React.PropsWithChildren) { )}

{Object.entries(nonHiddenArgs || {}) - .filter(([_, argConfig]) => - typeof argConfig.required === 'function' - ? argConfig.required(commandBarState.context) - : argConfig.required + .filter( + ([_, argConfig]) => + argConfig.skip === false || + (typeof argConfig.required === 'function' + ? argConfig.required(commandBarState.context) + : argConfig.required) ) .map(([argName, arg], i) => { const argValue = diff --git a/src/components/CommandBar/CommandBarSelectionInput.tsx b/src/components/CommandBar/CommandBarSelectionInput.tsx index 9b97a2f8b..91e378826 100644 --- a/src/components/CommandBar/CommandBarSelectionInput.tsx +++ b/src/components/CommandBar/CommandBarSelectionInput.tsx @@ -8,6 +8,7 @@ import { canSubmitSelectionArg, getSelectionCountByType, getSelectionTypeDisplayText, + type Selections, } from '@src/lib/selections' import { engineCommandManager, kclManager } from '@src/lib/singletons' import { reportRejection } from '@src/lib/trap' @@ -56,9 +57,13 @@ function CommandBarSelectionInput({ const selectionsByType = useMemo(() => { return getSelectionCountByType(selection) }, [selection]) + const isArgRequired = + arg.required instanceof Function + ? arg.required(commandBarState.context) + : arg.required const canSubmitSelection = useMemo( - () => canSubmitSelectionArg(selectionsByType, arg), - [selectionsByType] + () => !isArgRequired || canSubmitSelectionArg(selectionsByType, arg), + [selectionsByType, arg, isArgRequired] ) useEffect(() => { @@ -110,7 +115,18 @@ function CommandBarSelectionInput({ return } - onSubmit(selection) + /** + * Now that arguments like this can be optional, we need to + * construct an empty selection if it's not required to get it past our validation. + */ + const resolvedSelection: Selections | undefined = isArgRequired + ? selection + : selection || { + graphSelections: [], + otherSelections: [], + } + + onSubmit(resolvedSelection) } // Clear selection if needed diff --git a/src/components/CommandBar/CommandBarSelectionMixedInput.tsx b/src/components/CommandBar/CommandBarSelectionMixedInput.tsx index 2263453b0..728d7f413 100644 --- a/src/components/CommandBar/CommandBarSelectionMixedInput.tsx +++ b/src/components/CommandBar/CommandBarSelectionMixedInput.tsx @@ -6,6 +6,7 @@ import type { Selections } from '@src/lib/selections' import { canSubmitSelectionArg, getSelectionCountByType, + getSelectionTypeDisplayText, } from '@src/lib/selections' import { kclManager } from '@src/lib/singletons' import { commandBarActor, useCommandBarState } from '@src/lib/singletons' @@ -30,8 +31,14 @@ export default function CommandBarSelectionMixedInput({ const selectionsByType = useMemo(() => { return getSelectionCountByType(selection) }, [selection]) + const isArgRequired = + arg.required instanceof Function + ? arg.required(commandBarState.context) + : arg.required const canSubmitSelection = useMemo(() => { + // Don't do additional checks if this argument is not required + if (!isArgRequired) return true if (!selection) return false const isNonZeroRange = selection.graphSelections.some((sel) => { const range = sel.codeRef.range @@ -39,7 +46,7 @@ export default function CommandBarSelectionMixedInput({ }) if (isNonZeroRange) return true return canSubmitSelectionArg(selectionsByType, arg) - }, [selectionsByType, selection]) + }, [selectionsByType, selection, arg, isArgRequired]) useEffect(() => { inputRef.current?.focus() @@ -76,7 +83,18 @@ export default function CommandBarSelectionMixedInput({ return } - onSubmit(selection) + /** + * Now that arguments like this can be optional, we need to + * construct an empty selection if it's not required to get it past our validation. + */ + const resolvedSelection: Selections | undefined = isArgRequired + ? selection + : selection || { + graphSelections: [], + otherSelections: [], + } + + onSubmit(resolvedSelection) } const isMixedSelection = arg.inputType === 'selectionMixed' @@ -92,9 +110,10 @@ export default function CommandBarSelectionMixedInput({ (!hasSubmitted || canSubmitSelection || 'text-destroy-50') } > - {canSubmitSelection - ? 'Select objects in the scene' - : 'Select code or objects in the scene'} + {canSubmitSelection && + (selection.graphSelections.length || selection.otherSelections.length) + ? getSelectionTypeDisplayText(selection) + ' selected' + : 'Select code/objects, or skip'} {showSceneSelection && (
diff --git a/src/lib/commandBarConfigs/modelingCommandConfig.ts b/src/lib/commandBarConfigs/modelingCommandConfig.ts index d9fc367fb..663a969a2 100644 --- a/src/lib/commandBarConfigs/modelingCommandConfig.ts +++ b/src/lib/commandBarConfigs/modelingCommandConfig.ts @@ -22,6 +22,7 @@ import { KCL_DEFAULT_DEGREE, KCL_DEFAULT_LENGTH, KCL_DEFAULT_TRANSFORM, + ML_EXPERIMENTAL_MESSAGE, } from '@src/lib/constants' import type { components } from '@src/lib/machine-api' import type { Selections } from '@src/lib/selections' @@ -957,12 +958,13 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig< 'edgeCutEdge', ], multiple: true, - required: true, + required: false, selectionSource: { allowSceneSelection: true, allowCodeSelection: true, }, - skip: true, + skip: false, + warningMessage: ML_EXPERIMENTAL_MESSAGE, }, prompt: { inputType: 'text', diff --git a/src/machines/commandBarMachine.ts b/src/machines/commandBarMachine.ts index c920a5152..4c6a8a504 100644 --- a/src/machines/commandBarMachine.ts +++ b/src/machines/commandBarMachine.ts @@ -146,8 +146,15 @@ export const commandBarMachine = setup({ typeof argConfig.required === 'function' ? argConfig.required(context) : argConfig.required + /** + * TODO: we need to think harder about the relationship between + * `required`, `skip`, and `hidden`. + * This bit of logic essentially makes "skip false" arguments required. + * We may need a bit of state to mark an argument as "visited" for "skip false" args + * to truly not require any value to continue. + */ const mustNotSkipArg = - argIsRequired && + (argIsRequired || argConfig.skip === false) && (!context.argumentsToSubmit.hasOwnProperty(argName) || context.argumentsToSubmit[argName] === undefined || (rejectedArg &&