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
This commit is contained in:
Ruben Fiszel
2026-08-06 03:23:10 +02:00
committed by GitHub
parent b2d38e0391
commit e4e7782517
12 changed files with 116 additions and 189 deletions
@@ -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<PropPickerWrapperContext>('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'}
<!-- Settings rows have no picker pane, so the properties hang off the
button itself, exactly as the other expression inputs do. -->
<ExpressionPicker
id={argName}
pickableProperties={connectableProperties}
result={propPickerWrapperContext.result?.()}
extraResults={propPickerWrapperContext.extraResults?.()}
onSelect={(path) => {
// 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}
<FlowPlugConnect
wrapperClasses={twMerge(
'group-hover:opacity-100 transition-opacity',
@@ -646,8 +646,12 @@
clearFocus()
} else {
focusProp?.(argName, (path) => {
connectProperty(path)
dispatch('change', { argName })
if (pickerMode === 'sidePane') {
pickIntoArg(path)
} else {
connectProperty(path)
dispatch('change', { argName })
}
return true
})
}
@@ -46,10 +46,9 @@
</script>
<PropPickerWrapper
popover={true}
sidePane
flow_input={stepPropPicker.pickableProperties.flow_input}
notSelectable
displayContext={false}
pickableProperties={stepPropPicker.pickableProperties}
on:select={({ detail }) => {
editor?.insertAtCursor(detail)
@@ -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([])
})
@@ -226,10 +226,9 @@
{#if selectedTab === 'loop'}
<section>
<PropPickerWrapper
popover={true}
sidePane
flow_input={stepPropPicker.pickableProperties.flow_input}
notSelectable
displayContext={false}
pickableProperties={stepPropPicker.pickableProperties}
on:select={({ detail }) => {
editor?.insertAtCursor(detail)
@@ -323,10 +322,9 @@
{#if mod.value.parallel}
<div class="pl-9" transition:slideDynamic>
<PropPickerWrapper
popover={true}
sidePane
flow_input={stepPropPicker.pickableProperties.flow_input}
notSelectable
displayContext={false}
pickableProperties={stepPropPicker.pickableProperties}
on:select={({ detail }) => {
parallelismEditor?.insertAtCursor(detail)
@@ -256,12 +256,11 @@
{#if blocks !== 'all-iters' && !isBranchAll}
<div class="w-full flex flex-col gap-2">
<PropPickerWrapper
popover={true}
sidePane
flow_input={stepPropPicker.pickableProperties.flow_input}
notSelectable
result={earlyStopResult}
extraResults={isLoop ? { all_iters: result } : undefined}
displayContext={false}
pickableProperties={stepPropPicker.pickableProperties}
on:select={({ detail }) => {
stopAfterEditor?.insertAtCursor(detail)
@@ -299,11 +298,10 @@
{#if blocks !== 'stop-after' && (isLoop || isBranchAll)}
<div class="w-full flex flex-col gap-2">
<PropPickerWrapper
popover={true}
sidePane
flow_input={stepPropPicker.pickableProperties.flow_input}
notSelectable
{result}
displayContext={false}
pickableProperties={stepPropPicker.pickableProperties}
on:select={({ detail }) => {
stopAfterAllItersEditor?.insertAtCursor(detail)
@@ -71,11 +71,10 @@
<div class="w-full">
<PropPickerWrapper
popover={true}
sidePane
flow_input={stepPropPicker.pickableProperties.flow_input}
notSelectable
{result}
displayContext={false}
pickableProperties={stepPropPicker.pickableProperties}
on:select={({ detail }) => {
editor?.insertAtCursor(detail)
@@ -77,11 +77,10 @@
{#if flowModule.sleep && schema.properties['sleep'] && !sameWorker}
<div class="pl-9" transition:slideDynamic>
<PropPickerWrapper
popover={true}
sidePane
flow_input={stepPropPicker.pickableProperties.flow_input}
notSelectable
{result}
displayContext={false}
pickableProperties={stepPropPicker.pickableProperties}
on:select={({ detail }) => {
editor?.insertAtCursor(detail)
@@ -219,9 +219,9 @@
for any)
</span>
<PropPickerWrapper
popover={true}
sidePane
notSelectable
{result}
displayContext={false}
pickableProperties={undefined}
on:select={({ detail }) => {
editor?.insertAtCursor(detail)
@@ -78,7 +78,7 @@
{#if flowModule.timeout && schema.properties['timeout']}
<div class="pl-9" transition:slideDynamic>
<PropPickerWrapper
popover={true}
sidePane
flow_input={stepPropPicker.pickableProperties.flow_input}
notSelectable
pickableProperties={stepPropPicker.pickableProperties}
@@ -265,11 +265,10 @@
{#if stepPropPicker}
<PropPickerWrapper
popover={true}
sidePane
flow_input={stepPropPicker.pickableProperties.flow_input}
notSelectable
{result}
displayContext={false}
pickableProperties={stepPropPicker.pickableProperties}
on:select={({ detail }) => {
retryIfEditor?.insertAtCursor(detail)
@@ -1,105 +0,0 @@
<script lang="ts">
import { getContext, setContext } from 'svelte'
import { writable } from 'svelte/store'
import Popover from '$lib/components/meltComponents/Popover.svelte'
import PropPicker from '$lib/components/propertyPicker/PropPicker.svelte'
import FlowPlugConnect from '$lib/components/FlowPlugConnect.svelte'
import type { PropPickerContext } from '$lib/components/prop_picker'
import type { PickableProperties } from '../previousResults'
import type { PropPickerWrapperContext } from './PropPickerWrapper.svelte'
import { useConnect } from './useConnect.svelte'
import { twMerge } from 'tailwind-merge'
interface Props {
/** Identifies this input within the panel; arming another one disarms it. */
id: string
pickableProperties: PickableProperties | undefined
result?: any
extraResults?: any
/** Receives the picked property, whether it came from the popover or the graph. */
onSelect: (path: string) => void
disabled?: boolean
}
let { id, pickableProperties, result, extraResults, onSelect, disabled = false }: Props = $props()
const propPickerContext = getContext<PropPickerContext | undefined>('PropPickerContext')
const inModalPanel = $derived(propPickerContext?.inModalPanel?.() ?? false)
const connect = useConnect({
inModalPanel: () => inModalPanel,
hasPickableProperties: () => pickableProperties != undefined,
flowPropPickerConfig: propPickerContext?.flowPropPickerConfig ?? writable(undefined)
})
// PropPicker reads these to filter and highlight against what is being typed. Only the
// step input form produces that signal, so here they stay empty.
setContext<PropPickerWrapperContext>('PropPickerWrapper', {
propPickerConfig: writable(undefined),
inputMatches: writable(undefined),
exprBeingEdited: writable([]),
connectProp: () => {},
clearConnect: () => connect.disarm(),
pickerMode: () => 'popover' as const,
pickableProperties: () => undefined,
result: () => undefined,
extraResults: () => undefined,
onPick: () => {}
})
let open = $state(false)
// A pick from the graph resolves without touching the popover, so follow the armed
// slot rather than tracking open/closed separately.
$effect(() => {
if (!connect.isArmed(id)) {
open = false
}
})
</script>
<!-- excludeSelectors: a step's output picker in the graph is the other half of connecting, not
somewhere else to click. Reaching for it must not close this popover, which would disarm
the connect and leave the pick that follows with nowhere to land. -->
<Popover
bind:isOpen={open}
class="flex"
placement="bottom-start"
closeOnOutsideClick
excludeSelectors="[data-prop-picker]"
contentClasses="rounded-md border bg-surface shadow-lg overflow-hidden"
on:openChange={({ detail }) => (detail ? connect.arm({ id, onSelect }) : connect.disarm())}
>
{#snippet trigger()}
<FlowPlugConnect
connecting={connect.isArmed(id)}
{disabled}
title="Connect a property from a previous step"
wrapperClasses={twMerge(
// Revealed by the row it sits in, like the pane-mode plug. Armed or open, it stays
// put: the pointer has to be able to leave the row without the control vanishing.
'group-hover:opacity-100 transition-opacity',
connect.isArmed(id) || open ? '' : 'opacity-0'
)}
/>
{/snippet}
{#snippet content()}
<div class="max-h-80 w-72 overflow-auto p-2">
{#if pickableProperties}
<PropPicker
{pickableProperties}
previousId={pickableProperties?.previousId}
{result}
{extraResults}
on:select={({ detail }) => {
connect.resolve(detail)
open = false
}}
/>
{:else}
<div class="p-2 text-xs text-tertiary">Nothing to pick from yet.</div>
{/if}
</div>
{/snippet}
</Popover>
@@ -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<string[]>([])
const dispatch = createEventDispatcher()
const propPickerContext = getContext<PropPickerContext>('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<string[]>([])
exprBeingEdited
})
async function getPropPickerElements(): Promise<HTMLElement[]> {
@@ -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
}
</script>
{#snippet pickerBody()}
<!-- Exactly one destination per pick, or a cursor insertion lands twice: the armed input
if there is one, else the column's own input, else the form's `select` handler. -->
{@const deliver = (path: string) =>
connect.armed
? connect.resolve(path)
: pickTarget
? pickTarget.onSelect(path)
: dispatch('select', path)}
<div bind:clientHeight={rightPaneHeight} class="min-h-40 h-full !bg-surface">
<AnimatedButton
animate={$propPickerConfig != undefined}
@@ -134,10 +167,7 @@
{extraResults}
{flow_input}
allowCopy={!notSelectable && !$propPickerConfig}
on:select={({ detail }) => {
dispatch('select', detail)
connect.resolve(detail)
}}
on:select={({ detail }) => deliver(detail)}
/>
{:else if pickableProperties}
<PropPicker
@@ -148,10 +178,7 @@
previousId={pickableProperties?.previousId}
{pickableProperties}
allowCopy={!notSelectable && !$propPickerConfig}
on:select={({ detail }) => {
dispatch('select', detail)
connect.resolve(detail)
}}
on:select={({ detail }) => deliver(detail)}
/>
{/if}
</AnimatedButton>
@@ -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}
<!-- The picker lives on each input's connect button here, so the row keeps its
full width — and its own spacing, since the setting around it lays that out. -->
{@render children?.()}
{#if sidePane}
<!-- The row keeps its own spacing, since the setting around it lays that out. -->
<div class="flex w-full items-start gap-3">
<div class="min-w-0 grow">{@render children?.()}</div>
{#if sidePaneOpen && (pickableProperties != undefined || result != undefined)}
<!-- A set height, not the row's: the picker scrolls its own categories, and an
expression editor is one line tall next to them. -->
<div
class="shrink-0 w-[38%] min-w-52 max-w-xs h-72 overflow-auto border-l pl-2 {paneClass}"
transition:fade={{ duration: 100 }}
>
{@render pickerBody()}
</div>
{/if}
</div>
{:else}
<Splitpanes class={$propPickerConfig ? 'splitpanes-remove-splitter' : ''}>
<Pane minSize={20} size={60} class={'relative !transition-none'}>