Make commands disable, not unregister, based on their machineActor (#5070)

* Make "Find and select command" global to commandBarMachine

* Make commands not removed based on their actor state, only disabled

* Sort commands better in CommandComboBox

* Break out sort logic, add a few unit tests

* Fix missed name change

* Needed to make one more change from source branch:
since `optionsFromContext` now only gets fired once, I/O-based options need to use the `options` config instead.

---------

Co-authored-by: 49fl <ircsurfer33@gmail.com>
This commit is contained in:
Frank Noirot
2025-01-16 12:08:48 -05:00
committed by GitHub
parent 0a1a6e50cf
commit 3f855d7bad
9 changed files with 149 additions and 36 deletions

View File

@ -22,6 +22,7 @@ export const CommandBar = () => {
// Close the command bar when navigating
useEffect(() => {
if (commandBarState.matches('Closed')) return
commandBarSend({ type: 'Close' })
}, [pathname])

View File

@ -4,6 +4,8 @@ import { useCommandsContext } from 'hooks/useCommandsContext'
import { Command } from 'lib/commandTypes'
import { useEffect, useState } from 'react'
import { CustomIcon } from './CustomIcon'
import { getActorNextEvents } from 'lib/utils'
import { sortCommands } from 'lib/commandUtils'
function CommandComboBox({
options,
@ -18,8 +20,16 @@ function CommandComboBox({
const defaultOption =
options.find((o) => 'isCurrent' in o && o.isCurrent) || null
// sort disabled commands to the bottom
const sortedOptions = options
.map((command) => ({
command,
disabled: optionIsDisabled(command),
}))
.sort(sortCommands)
.map(({ command }) => command)
const fuse = new Fuse(options, {
const fuse = new Fuse(sortedOptions, {
keys: ['displayName', 'name', 'description'],
threshold: 0.3,
ignoreLocation: true,
@ -27,7 +37,7 @@ function CommandComboBox({
useEffect(() => {
const results = fuse.search(query).map((result) => result.item)
setFilteredOptions(query.length > 0 ? results : options)
setFilteredOptions(query.length > 0 ? results : sortedOptions)
}, [query])
function handleSelection(command: Command) {
@ -73,7 +83,8 @@ function CommandComboBox({
<Combobox.Option
key={option.groupId + option.name + (option.displayName || '')}
value={option}
className="flex items-center gap-4 px-4 py-1.5 first:mt-2 last:mb-2 ui-active:bg-primary/10 dark:ui-active:bg-chalkboard-90"
className="flex items-center gap-4 px-4 py-1.5 first:mt-2 last:mb-2 ui-active:bg-primary/10 dark:ui-active:bg-chalkboard-90 ui-disabled:!text-chalkboard-50"
disabled={optionIsDisabled(option)}
>
{'icon' in option && option.icon && (
<CustomIcon name={option.icon} className="w-5 h-5" />
@ -96,3 +107,11 @@ function CommandComboBox({
}
export default CommandComboBox
function optionIsDisabled(option: Command): boolean {
return (
'machineActor' in option &&
option.machineActor !== undefined &&
!getActorNextEvents(option.machineActor.getSnapshot()).includes(option.name)
)
}

View File

@ -1,5 +1,5 @@
import { useEffect } from 'react'
import { AnyStateMachine, Actor, StateFrom } from 'xstate'
import { AnyStateMachine, Actor, StateFrom, EventFrom } from 'xstate'
import { createMachineCommand } from '../lib/createMachineCommand'
import { useCommandsContext } from './useCommandsContext'
import { modelingMachine } from 'machines/modelingMachine'
@ -15,7 +15,6 @@ import { useKclContext } from 'lang/KclProvider'
import { useNetworkContext } from 'hooks/useNetworkContext'
import { NetworkHealthState } from 'hooks/useNetworkStatus'
import { useAppState } from 'AppState'
import { getActorNextEvents } from 'lib/utils'
// This might not be necessary, AnyStateMachine from xstate is working
export type AllMachines =
@ -60,21 +59,21 @@ export default function useStateMachineCommands<
overallState !== NetworkHealthState.Weak) ||
isExecuting ||
!isStreamReady
const newCommands = getActorNextEvents(state)
const newCommands = Object.keys(commandBarConfig || {})
.filter((_) => !allCommandsRequireNetwork || !disableAllButtons)
.filter((e) => !['done.', 'error.'].some((n) => e.includes(n)))
.flatMap((type) =>
createMachineCommand<T, S>({
.flatMap((type) => {
const typeWithProperType = type as EventFrom<T>['type']
return createMachineCommand<T, S>({
// The group is the owner machine's ID.
groupId: machineId,
type,
type: typeWithProperType,
state,
send,
actor,
commandBarConfig,
onCancel,
})
)
})
.filter((c) => c !== null) as Command[] // TS isn't smart enough to know this filter removes nulls
commandBarSend({ type: 'Add commands', data: { commands: newCommands } })
@ -85,5 +84,5 @@ export default function useStateMachineCommands<
data: { commands: newCommands },
})
}
}, [state, overallState, isExecuting, isStreamReady])
}, [overallState, isExecuting, isStreamReady])
}

View File

@ -63,12 +63,11 @@ export const projectsCommandBarConfig: StateMachineCommandSetConfig<
name: {
inputType: 'options',
required: true,
options: [],
optionsFromContext: (context) =>
context.projects.map((p) => ({
options: (_, context) =>
context?.projects.map((p) => ({
name: p.name!,
value: p.name!,
})),
})) || [],
},
},
},
@ -80,12 +79,11 @@ export const projectsCommandBarConfig: StateMachineCommandSetConfig<
oldName: {
inputType: 'options',
required: true,
options: [],
optionsFromContext: (context) =>
context.projects.map((p) => ({
options: (_, context) =>
context?.projects.map((p) => ({
name: p.name!,
value: p.name!,
})),
})) || [],
},
newName: {
inputType: 'string',

View File

@ -76,6 +76,7 @@ export type Command<
| ((
commandBarContext: { argumentsToSubmit: Record<string, unknown> } // Should be the commandbarMachine's context, but it creates a circular dependency
) => string | ReactNode)
machineActor?: Actor<T>
onSubmit: (data?: CommandSchema) => void
onCancel?: () => void
args?: {

View File

@ -0,0 +1,49 @@
import { CommandWithDisabledState, sortCommands } from './commandUtils'
function commandWithDisabled(
name: string,
disabled: boolean,
groupId = 'modeling'
): CommandWithDisabledState {
return {
command: {
name,
groupId,
needsReview: false,
onSubmit: () => {},
},
disabled,
}
}
describe('Command sorting', () => {
it(`Puts modeling commands first`, () => {
const initial = [
commandWithDisabled('a', false, 'settings'),
commandWithDisabled('b', false, 'modeling'),
commandWithDisabled('c', false, 'settings'),
]
const sorted = initial.sort(sortCommands)
expect(sorted[0].command.groupId).toBe('modeling')
})
it(`Puts disabled commands last`, () => {
const initial = [
commandWithDisabled('a', true, 'modeling'),
commandWithDisabled('z', false, 'modeling'),
commandWithDisabled('a', false, 'settings'),
]
const sorted = initial.sort(sortCommands)
expect(sorted[sorted.length - 1].disabled).toBe(true)
})
it(`Puts settings commands second to last`, () => {
const initial = [
commandWithDisabled('a', true, 'modeling'),
commandWithDisabled('z', false, 'modeling'),
commandWithDisabled('a', false, 'settings'),
]
const sorted = initial.sort(sortCommands)
expect(sorted[1].command.groupId).toBe('settings')
})
})

View File

@ -2,6 +2,9 @@
// That object also contains some metadata about what to do with the KCL expression,
// such as whether we need to create a new variable for it.
// This function extracts the value field from those arg payloads and returns
import { Command } from './commandTypes'
// The arg object with all its field as natural values that the command to be executed will expect.
export function getCommandArgumentKclValuesOnly(args: Record<string, unknown>) {
return Object.fromEntries(
@ -13,3 +16,42 @@ export function getCommandArgumentKclValuesOnly(args: Record<string, unknown>) {
})
)
}
export interface CommandWithDisabledState {
command: Command
disabled: boolean
}
/**
* Sorting logic for commands in the command combo box.
*/
export function sortCommands(
a: CommandWithDisabledState,
b: CommandWithDisabledState
) {
// Disabled commands should be at the bottom
if (a.disabled && !b.disabled) {
return 1
}
if (b.disabled && !a.disabled) {
return -1
}
// Settings commands should be next-to-last
if (a.command.groupId === 'settings' && b.command.groupId !== 'settings') {
return 1
}
if (b.command.groupId === 'settings' && a.command.groupId !== 'settings') {
return -1
}
// Modeling commands should be first
if (a.command.groupId === 'modeling' && b.command.groupId !== 'modeling') {
return -1
}
if (b.command.groupId === 'modeling' && a.command.groupId !== 'modeling') {
return 1
}
// Sort alphabetically
return (a.command.displayName || a.command.name).localeCompare(
b.command.displayName || b.command.name
)
}

View File

@ -96,6 +96,7 @@ export function createMachineCommand<
icon,
description: commandConfig.description,
needsReview: commandConfig.needsReview || false,
machineActor: actor,
onSubmit: (data?: S[typeof type]) => {
if (data !== undefined && data !== null) {
send({ type, data })

View File

@ -119,6 +119,9 @@ export const commandBarMachine = setup({
selectedCommand?.onSubmit()
}
},
'Clear selected command': assign({
selectedCommand: undefined,
}),
'Set current argument to first non-skippable': assign({
currentArgument: ({ context, event }) => {
const { selectedCommand } = context
@ -246,6 +249,7 @@ export const commandBarMachine = setup({
context.selectedCommand?.needsReview || false,
'Command has no arguments': () => false,
'All arguments are skippable': () => false,
'Has selected command': ({ context }) => !!context.selectedCommand,
},
actors: {
'Validate argument': fromPromise(
@ -394,7 +398,7 @@ export const commandBarMachine = setup({
),
},
}).createMachine({
/** @xstate-layout N4IgpgJg5mDOIC5QGED2BbdBDAdhABAEJYBOAxMgDaqxgDaADALqKgAONAlgC6eo6sQAD0QBaAJwA6AGwAmAKwBmBoukAWafIAcDcSoA0IAJ6JZDaZIDs8hgzV6AjA61a1DWQF8PhtJlwFiciowUkYWJBAOWB4+AQiRBFF5CwdpcVkHS1lpVyU5QxNEh1lFGTUsrUUtOQd5SwZLLx8MbDwiUkkqGkgyAHk2MBwwwSiY-kEEswZJbPltM0U3eXLZAsRFeQcrerUHRbTFvfkmkF9WgI6u2ggyADFONv98WkowAGNufDeW-2GI0d443iiHESkktUUilkskqdiUWjWCAcDC0kjUqnElkc6lkoK0JzOT0CnWo1zIAEEIARvn48LA-uwuIC4qAEqIHJjJPJcYtxFoYdJFFjVsZEG5pqCquJxJoGvJxOUCT82sSrj0AEpgdCoABuYC+yog9OYIyZsQmYmhWzMmTqLnU0kyikRGVRbj5lg2SmUam5StpFxIkgAymBXh8HlADQGyKHw58aecGZEzUDWYgchY7CisWoSlpQQ5EVDLJJxKkdIpyypUop-ed2kHCW0Xu9uD1kwDzcCEJCtlUNgWhZZ1OlnaKEBpZJItHVzDZ5xlpPWiZdDc8w22O7JwozosyLUj3KiZZY85YtMUNgx5Ii81JuS5xBtPVCCyuVR0AOJYbgACzAEhI3wUgoAAV3QQZuFgCg-1wGAvjAkgSCgkCSHAyCcG4TtUxZYRED2TQZGSOw80qaQ7ARCc9mmZYSjPPFpDSQUP0DSQf3-QDgNAiCoJggAROBNw+aMkxNf5cMPHJSjPWRdhvZ9LGcF0b0kVQ8ycDRNkvNRWMbdjfwAoCcCjHjMOgyRyQAdywGITPwB42DA7hYzAgAjdAeDQjCoJw-du3TJE6mnWwoT0NIUTUAs71sMtdGsRYCyUtI9OJDijO49DeKw2BJAANSwShOAgX9IzICB+DASQHh1VAAGsqp1Qrit-MBySy8y-LGPCElSeoy2lZJcwcNRfXHQpBWmWQsRsPYdDPZJUu-QyuPssy+Py5qSt4EyyEAkhUCDNhKF-AAzQ70EkJqiu2tqOt88S926w9UhhLlykWXFHXcTJixsd60hHGxNj2Jag01HVODAKzXI8rzE1+R6U38tN8IQJSuR0OZ0gvHQXHGxBPWmUdppUWwFV0MHJAhqGYcpAh1qwrqDx7ZFajUmVpulEbqlqREsfBTQ0jcPM5P5KmaehshNW1PVvOy7Cka7VHeoYDkyjSPQtAvPMqkRQU1BnX1+UydFkQvCWwEhqWAFEIC8xnFd3ZHntZuTDZG4ob1nGxPTUfXrBnS8nDmEpT2ObxTnXVUALeOrgPanycvKyrqpwWqGqurbWsThXjWd5WesQZYtmBkplCceplIncK0SrXYqnccxxcj5s2OQWP4-s3PzJgiqcCqmr6sa7P2x7vi6AcAvJJ7XEy0dUFMWsFxlnKfn+S5CL3E9Jiuapjv3i7qNx+T-bDskY6zourObpz+6cuZgK0Y5Ua0TqEvXDkJR-YnR0s1xjkIMnDln3uuVsHwOxT1NCjIuR5nCSBUExJi1huRqyooUTYpYnzeyUFkKsy4Tg4FQBAOAgg26Nmga7QKohshSBtNYXGDonQumtPYaQfsSjLBHDefepJICUJZtQ4oMx8wXj6jebGt4JwbC2OiVwig9hh2hLpVu0cOhxjbMBBGeABFPwSA3ERRMlhKWCn9WRVQzZQirMo0BAYNzxn4RJGBh5MRbGXsHawGQFBmLrpUasOQorOAjs0OxaUVrGVMvfaCuiVYEV2KWTYg1yxyA0i6ZYaIPSL3lOkS8wSo6hOWpxCJ8te6WRsnZKMjlnIxNgSNIiiTF6vVSdI9JM1taXlxLzTwqiClBnSqtSJScLIFVvjtKANSXquENqoJwtgyZzRFIUQ4MhqgNACdNTQCpLbWyshMnsjpSwjXRJYHecgcmImsLIz0odNAaGqPiHpDYY6HwTlE+ATiqHPwohYewWQshmGhHrCcJz5Bck5toZwGhMheC8EAA */
/** @xstate-layout N4IgpgJg5mDOIC5QGED2BbdBDAdhABAEJYBOAxMgDaqxgDaADALqKgAONAlgC6eo6sQAD0QBaAIwB2AHTiAHAE45AZjkAmdcoaSArCoA0IAJ6JxDOdJ2SF4gCySHaqZIa2Avm8NpMuAsXJUYKSMLEggHLA8fAJhIgiiOgBssokKTpJqiXK2OsqJaoYm8eJqytKJ9hqq+eJW2h5eGNh4RKRkAGKcLb74tJRgAMbc+ANNviGCEVH8gnEKubK5ympVrrlyhabm0rZ5CtYM4hVq83ININ7NfqTSVDSQZADybGA4E2FTvDOxiGoMDNJMjo9H9lLYGDpKpsEModOJpA5XOIwakwcidOdLj1-LdqLQIGQAIIQAijHx4WDvdhcL4xUBxURqSTw3LzfYKZSSOQZWzQ5S1crMuTAiElRKuTFjFo4u74sgAJTA6FQADcwCMpRBKcxJjTorMxEzkhouftuXCGGpIdCpADmZJxfzJLY5Cp5pLydcSLj7gSqeE9d96Yh+TppKoXfkGKdlHloUyFIDUvMXBzbFl3J4LprWt6AMpgfpDLpQDWesgFovDMlXf2ffU-BBZZKuczWWylRRwvlM6Q2LIMZQ2QdHZQeq65245vqDbgPOuBunCEP88MqPQchwVNLKaHptTSYUuRI6f4npyJcfYm5Ylozobz8ShamRWkGhBmeTSQeJX+JXQ6H88jQnCMiugoELCpypQKJeWa3l6U6er0hazvOajPgGr4NsGH6WhYsHOkycglLCEJ7iclgaIosKSLGGgKFe0o3AA4lg3AABZgCQJb4KQUAAK7oK83CwBQHG4DAIwCSQJAiXxJCCcJODcAu2FBsuH55GGJ7irYHYqHpGzGKYWiWB2nK2Kcv6wWO8E5jibGcdxvH8UJIliQAInAqFDGWtY6h8i7vlkZREbYZg6JuwEmQgfxhnkHbiHYJ7yHYTGIU5XE8TgpZucponSISADuWBRLl+BdGwAncBWAkAEboDwClKSJanTEucS1Bk36DicDCpOYLoKHu-x9tGuhgoozKpBlk5ZS5FX5R50gAGpYJQnAQOxJZkBA-BgNIXQqqgADWh0qhtW3sWAeYlv0hKKe5KntW+YRFGi5RyOKDrZEaUW8pppTguGuwDRFEO-nNjnsdlrlPQVsBrVd228LlZDcSQqDemwlDsQAZtj6DSJdm2o7d91gI9rUvYFL4dYIH0RV9P0Zv9CiA3EwMAmCWgVHYKVwY0yE4oqKqcGAxV1Y1zU1uMdNYQzjbMpYcgQlFxFq66u6xXRALbkyg7-Bz0bQzcYsS1LxIEMttOYfWGldYcCWwQmNiRrU0Jq2GRxJCbHZqC6ahm96FuSwqSqquqtuqQrDudVs4iJhUyZtn9qjQokYKHjk6hSLsZhciH0hh1LACiEDNTHr04ZpJT6bIEXxcKp50YDRT-mGrrJbUgFDp3xfIFxAynbx1PPaJe0HUdOAnedJMozd4+IzXjuIJCLIQqUWjJS4MVFBByS7BzyJq38WTB-ZIs3sPo8VcvHlTzgh3HWdF2L3OD8qZST66upCdxUTLBJOUUHB6GFPpSQXt1CWEGpaOiv4EyD1vmPBGj9MbY2kLjAmRMF5kyXmg7+q8AFJwbjkXQEVsj5FyO3RAiQjjfi5CReYPck7iA8FmHAqAIBwEEAhXMf8la4VEMsWwgJuTTXNGYK0tC4rwkDvsAaSQ-qlAhIPPEkBBFvWEVycoFQhywUHGaHWH04Q7FUNBdc+x2FXwnDiSss5eJyzwFo2ucQIplBWHrcEVhuoFFirCeEuxsj8mWEOFYmZhZ2JvNOXyc4ICuLXggaxCJwG70AiUHQfIzHBKHGYDMJFhTFwWjlPKhDRKJJIRFGQcIFBsiOIHJw8ZIQ7CUGrY+9g9AnmKbDRaZSaaFRKmVNGpYqo1Uqe+FKYZan1PyElbJYjrB6C5CUJQ9DL5ROvN6Ep8MBlI3WvgkZEzGzyAbnkZK-wjan38UzeEzZtBswdADYupdjm4XoTIOwuwHB5HyGkYyRRdBBLosCIE6ZvpnFsVs24KD77lPgEFf+kzxRH32EyFYlpOzQjAZYV2ehTkfI4W4IAA */
context: {
commands: [],
selectedCommand: undefined,
@ -421,14 +425,6 @@ export const commandBarMachine = setup({
target: 'Selecting command',
},
'Find and select command': {
target: 'Command selected',
actions: [
'Find and select command',
'Initialize arguments to submit',
],
},
'Add commands': {
target: 'Closed',
@ -440,8 +436,6 @@ export const commandBarMachine = setup({
),
}),
],
reenter: false,
},
'Remove commands': {
@ -458,10 +452,13 @@ export const commandBarMachine = setup({
),
}),
],
reenter: false,
},
},
always: {
target: 'Command selected',
guard: 'Has selected command',
},
},
'Selecting command': {
@ -478,7 +475,7 @@ export const commandBarMachine = setup({
{
target: 'Closed',
guard: 'Command has no arguments',
actions: ['Execute command'],
actions: ['Execute command', 'Clear selected command'],
},
{
target: 'Checking Arguments',
@ -548,7 +545,7 @@ export const commandBarMachine = setup({
on: {
'Submit command': {
target: 'Closed',
actions: ['Execute command'],
actions: ['Execute command', 'Clear selected command'],
},
'Add argument': {
@ -580,7 +577,7 @@ export const commandBarMachine = setup({
},
{
target: 'Closed',
actions: 'Execute command',
actions: ['Execute command', 'Clear selected command'],
},
],
onError: [
@ -600,6 +597,7 @@ export const commandBarMachine = setup({
Close: {
target: '.Closed',
actions: 'Clear selected command',
},
Clear: {
@ -607,6 +605,11 @@ export const commandBarMachine = setup({
reenter: false,
actions: ['Clear argument data'],
},
'Find and select command': {
target: '.Command selected',
actions: ['Find and select command', 'Initialize arguments to submit'],
},
},
})