From e4e7782517a9fd0fbbec020694b263a948ed1c0c Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Thu, 6 Aug 2026 03:23:10 +0200 Subject: [PATCH] fix: restore the flow expression editor's property side panel (#10555) * fix: give flow expression editors their property side panel back * fix: keep the picker column tied to an input that can receive the pick --- .../lib/components/InputTransformForm.svelte | 60 +++++----- .../content/BranchPredicateEditor.svelte | 3 +- .../content/FlowEnvironmentVariables.svelte | 6 +- .../components/flows/content/FlowLoop.svelte | 6 +- .../flows/content/FlowModuleEarlyStop.svelte | 6 +- .../flows/content/FlowModuleSkip.svelte | 3 +- .../flows/content/FlowModuleSleep.svelte | 3 +- .../flows/content/FlowModuleSuspend.svelte | 4 +- .../flows/content/FlowModuleTimeout.svelte | 2 +- .../flows/content/FlowRetries.svelte | 3 +- .../flows/propPicker/ExpressionPicker.svelte | 105 ------------------ .../flows/propPicker/PropPickerWrapper.svelte | 104 +++++++++++------ 12 files changed, 116 insertions(+), 189 deletions(-) delete mode 100644 frontend/src/lib/components/flows/propPicker/ExpressionPicker.svelte diff --git a/frontend/src/lib/components/InputTransformForm.svelte b/frontend/src/lib/components/InputTransformForm.svelte index da89d61f12..5df67638b0 100644 --- a/frontend/src/lib/components/InputTransformForm.svelte +++ b/frontend/src/lib/components/InputTransformForm.svelte @@ -37,7 +37,6 @@ import type { PickableProperties } from './flows/previousResults' import { twMerge } from 'tailwind-merge' import FlowPlugConnect from './FlowPlugConnect.svelte' - import ExpressionPicker from './flows/propPicker/ExpressionPicker.svelte' import { deepEqual } from 'fast-equals' import S3ArrayHelperButton from './S3ArrayHelperButton.svelte' import { inputBorderClass } from './text_input/TextInput.svelte' @@ -157,10 +156,6 @@ const propPickerWrapperContext: PropPickerWrapperContext | undefined = getContext('PropPickerWrapper') const pickerMode = $derived(propPickerWrapperContext?.pickerMode?.() ?? 'pane') - // Settings rows hand their properties to the wrapper, not to this form. - const connectableProperties = $derived( - pickableProperties ?? propPickerWrapperContext?.pickableProperties?.() - ) const { inputMatches, connectProp: focusProp, @@ -365,6 +360,18 @@ }) } + /** A predicate is usually half-written when you reach for a property, so insert at the + * cursor and leave the rest of the expression alone. Only a field that isn't an + * expression yet gets replaced outright. */ + function pickIntoArg(path: string) { + if (propertyType === 'javascript' && monaco) { + propPickerWrapperContext?.onPick?.(path) + } else { + connectProperty(path) + } + dispatch('change', { argName }) + } + function connectProperty(rawValue: string) { // Extract path from variable('x') or resource('x') format const varMatch = variableMatch(rawValue) @@ -465,8 +472,21 @@ } } + // The column beside a settings row delivers here rather than through the host's `select` + // handler, which can only reach a mounted expression editor. A collapsed setting has no + // field at all, so it gives the target up and the column closes with it. + $effect(() => { + if (pickerMode !== 'sidePane') return + propPickerWrapperContext?.setPickTarget?.( + collapsed ? undefined : { id: argName, onSelect: pickIntoArg } + ) + }) + onDestroy(() => { updatePropsBeingEdited(false) + if (pickerMode === 'sidePane') { + propPickerWrapperContext?.setPickTarget?.(undefined) + } }) let prevArg: any = undefined @@ -613,27 +633,7 @@ /> {/if} - {#if propPickerWrapperContext && pickerMode === 'popover'} - - { - // A predicate is usually half-written when you reach for a property, so - // insert at the cursor and leave the rest of the expression alone. Only - // a field that isn't an expression yet gets replaced outright. - if (propertyType === 'javascript' && monaco) { - propPickerWrapperContext.onPick?.(path) - } else { - connectProperty(path) - } - dispatch('change', { argName }) - }} - /> - {:else if propPickerWrapperContext} + {#if propPickerWrapperContext} { - connectProperty(path) - dispatch('change', { argName }) + if (pickerMode === 'sidePane') { + pickIntoArg(path) + } else { + connectProperty(path) + dispatch('change', { argName }) + } return true }) } diff --git a/frontend/src/lib/components/flows/content/BranchPredicateEditor.svelte b/frontend/src/lib/components/flows/content/BranchPredicateEditor.svelte index 493f04b86e..949862632a 100644 --- a/frontend/src/lib/components/flows/content/BranchPredicateEditor.svelte +++ b/frontend/src/lib/components/flows/content/BranchPredicateEditor.svelte @@ -46,10 +46,9 @@ { editor?.insertAtCursor(detail) diff --git a/frontend/src/lib/components/flows/content/FlowEnvironmentVariables.svelte b/frontend/src/lib/components/flows/content/FlowEnvironmentVariables.svelte index 44204dc786..cd14ddc23a 100644 --- a/frontend/src/lib/components/flows/content/FlowEnvironmentVariables.svelte +++ b/frontend/src/lib/components/flows/content/FlowEnvironmentVariables.svelte @@ -234,10 +234,8 @@ connectProp: () => {}, propPickerConfig: writable(undefined), clearConnect: () => {}, - pickerMode: () => 'popover' as const, - pickableProperties: () => undefined, - result: () => undefined, - extraResults: () => undefined, + pickerMode: () => 'pane' as const, + setPickTarget: () => {}, onPick: () => {}, exprBeingEdited: writable([]) }) diff --git a/frontend/src/lib/components/flows/content/FlowLoop.svelte b/frontend/src/lib/components/flows/content/FlowLoop.svelte index d78a3199b7..bfb80addd6 100644 --- a/frontend/src/lib/components/flows/content/FlowLoop.svelte +++ b/frontend/src/lib/components/flows/content/FlowLoop.svelte @@ -226,10 +226,9 @@ {#if selectedTab === 'loop'}
{ editor?.insertAtCursor(detail) @@ -323,10 +322,9 @@ {#if mod.value.parallel}
{ parallelismEditor?.insertAtCursor(detail) diff --git a/frontend/src/lib/components/flows/content/FlowModuleEarlyStop.svelte b/frontend/src/lib/components/flows/content/FlowModuleEarlyStop.svelte index 8f0f091320..e9a824c294 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleEarlyStop.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleEarlyStop.svelte @@ -256,12 +256,11 @@ {#if blocks !== 'all-iters' && !isBranchAll}
{ stopAfterEditor?.insertAtCursor(detail) @@ -299,11 +298,10 @@ {#if blocks !== 'stop-after' && (isLoop || isBranchAll)}
{ stopAfterAllItersEditor?.insertAtCursor(detail) diff --git a/frontend/src/lib/components/flows/content/FlowModuleSkip.svelte b/frontend/src/lib/components/flows/content/FlowModuleSkip.svelte index 51d75b0ca6..40fb32b064 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleSkip.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleSkip.svelte @@ -71,11 +71,10 @@
{ editor?.insertAtCursor(detail) diff --git a/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte b/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte index 03dcf2b6f5..f0645c4854 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleSleep.svelte @@ -77,11 +77,10 @@ {#if flowModule.sleep && schema.properties['sleep'] && !sameWorker}
{ editor?.insertAtCursor(detail) diff --git a/frontend/src/lib/components/flows/content/FlowModuleSuspend.svelte b/frontend/src/lib/components/flows/content/FlowModuleSuspend.svelte index a20bb8c671..43c65bc482 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleSuspend.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleSuspend.svelte @@ -219,9 +219,9 @@ for any) { editor?.insertAtCursor(detail) diff --git a/frontend/src/lib/components/flows/content/FlowModuleTimeout.svelte b/frontend/src/lib/components/flows/content/FlowModuleTimeout.svelte index 9f6a1b32b2..ae4c614ee2 100644 --- a/frontend/src/lib/components/flows/content/FlowModuleTimeout.svelte +++ b/frontend/src/lib/components/flows/content/FlowModuleTimeout.svelte @@ -78,7 +78,7 @@ {#if flowModule.timeout && schema.properties['timeout']}
{ retryIfEditor?.insertAtCursor(detail) diff --git a/frontend/src/lib/components/flows/propPicker/ExpressionPicker.svelte b/frontend/src/lib/components/flows/propPicker/ExpressionPicker.svelte deleted file mode 100644 index 6e29efe58d..0000000000 --- a/frontend/src/lib/components/flows/propPicker/ExpressionPicker.svelte +++ /dev/null @@ -1,105 +0,0 @@ - - - - (detail ? connect.arm({ id, onSelect }) : connect.disarm())} -> - {#snippet trigger()} - - {/snippet} - {#snippet content()} -
- {#if pickableProperties} - { - connect.resolve(detail) - open = false - }} - /> - {:else} -
Nothing to pick from yet.
- {/if} -
- {/snippet} - diff --git a/frontend/src/lib/components/flows/propPicker/PropPickerWrapper.svelte b/frontend/src/lib/components/flows/propPicker/PropPickerWrapper.svelte index 7820b1a53b..5fce4a972a 100644 --- a/frontend/src/lib/components/flows/propPicker/PropPickerWrapper.svelte +++ b/frontend/src/lib/components/flows/propPicker/PropPickerWrapper.svelte @@ -13,17 +13,17 @@ inputMatches: Writable<{ word: string; value: string }[] | undefined> connectProp: (propName: string, onSelect: SelectCallback) => void clearConnect: () => void - /** 'popover' hangs the picker off each input's own connect button instead of - * taking a pane — for single-argument settings rows, which are not the step's - * input form. */ - pickerMode: () => 'pane' | 'popover' - /** The wrapper owns these; nested inputs receive none of their own. */ - pickableProperties: () => PickableProperties | undefined - /** The step's own result, and anything extra worth offering beside it (a loop's - * `all_iters`). Only the pane renders them directly — in popover mode the picker - * hangs off each input, so it reads them from here instead. */ - result: () => any - extraResults: () => any + /** Where the properties are offered, and therefore what a pick does: + * - 'pane': the step's input form — a pick replaces the argument outright. + * - 'sidePane': a settings row — a pick lands at the expression's cursor, since a + * half-written predicate must survive it. */ + pickerMode: () => 'pane' | 'sidePane' + /** The single input a `sidePane` column belongs to. It stays a destination for as + * long as its field is mounted, so a pick lands whether or not a connect is armed — + * a static field has no editor for the host's own `select` handler to write into. + * `undefined` (the setting was switched off) leaves the column with nowhere to + * deliver, so it closes. */ + setPickTarget: (target: { id: string; onSelect: (path: string) => void } | undefined) => void /** Deliver a pick the way the pane does — as a `select` event, so each setting's own * handler inserts it at the cursor. Replacing the whole value is right for a step * input but destroys a half-written predicate. */ @@ -37,6 +37,7 @@ import PropPickerResult from '$lib/components/propertyPicker/PropPickerResult.svelte' import { clickOutside } from '$lib/utils' import { createEventDispatcher, getContext, setContext } from 'svelte' + import { fade } from 'svelte/transition' import { Pane, Splitpanes } from 'svelte-splitpanes' import { writable, type Writable } from 'svelte/store' import type { PickableProperties } from '../previousResults' @@ -55,8 +56,9 @@ noPadding?: boolean paneClass?: string /** Settings rows reuse the step-input form for one argument but are not the input - * form; their picker belongs in a popover. */ - popover?: boolean + * form: their picker is a column beside the row, revealed while the expression is + * being written, rather than a permanent split of the panel. */ + sidePane?: boolean children?: import('svelte').Snippet } @@ -70,7 +72,7 @@ notSelectable = false, noPadding = false, paneClass = '', - popover = false, + sidePane = false, children }: Props = $props() @@ -79,6 +81,7 @@ >(undefined) const inputMatches = writable<{ word: string; value: string }[] | undefined>(undefined) + const exprBeingEdited = writable([]) const dispatch = createEventDispatcher() const propPickerContext = getContext('PropPickerContext') @@ -100,13 +103,14 @@ propPickerConfig, inputMatches, connectProp: (propName, onSelect) => connect.arm({ id: propName, onSelect }), - clearConnect: connect.disarm, - pickerMode: () => (popover ? 'popover' : 'pane'), - pickableProperties: () => pickableProperties, - result: () => result, - extraResults: () => extraResults, + clearConnect: closePicker, + pickerMode: () => (sidePane ? 'sidePane' : 'pane'), + setPickTarget: (target) => { + pickTarget = target + if (!target) closePicker() + }, onPick: (path) => dispatch('select', path), - exprBeingEdited: writable([]) + exprBeingEdited }) async function getPropPickerElements(): Promise { @@ -116,9 +120,38 @@ } let rightPaneHeight: number = $state(0) + + let pickTarget: { id: string; onSelect: (path: string) => void } | undefined = $state(undefined) + + // The side column stays put once the row is being worked on: picking a property blurs + // the editor, so closing on blur would take the column away mid-click. It is dismissed + // deliberately instead — by clicking away, by the input's own connect button, or by the + // setting being switched off. + let sidePaneOpen = $state(false) + $effect(() => { + if ($propPickerConfig != undefined || $exprBeingEdited.length > 0) { + sidePaneOpen = true + } + }) + + function closePicker() { + connect.disarm() + // A switched-off setting unmounts its editor rather than blurring it, so the focus + // claim outlives the field — left standing, it reopens the column on the next tick. + exprBeingEdited.set([]) + sidePaneOpen = false + } {#snippet pickerBody()} + + {@const deliver = (path: string) => + connect.armed + ? connect.resolve(path) + : pickTarget + ? pickTarget.onSelect(path) + : dispatch('select', path)}
{ - dispatch('select', detail) - connect.resolve(detail) - }} + on:select={({ detail }) => deliver(detail)} /> {:else if pickableProperties} { - dispatch('select', detail) - connect.resolve(detail) - }} + on:select={({ detail }) => deliver(detail)} /> {/if} @@ -167,13 +194,24 @@ // Through the controller, not the stores: it owns the armed target, and a // target left armed here would make the next click on that same input // read as a toggle-off. - onClickOutside: connect.disarm + onClickOutside: closePicker }} > - {#if popover} - - {@render children?.()} + {#if sidePane} + +
+
{@render children?.()}
+ {#if sidePaneOpen && (pickableProperties != undefined || result != undefined)} + +
+ {@render pickerBody()} +
+ {/if} +
{:else}