[PRO-93] feat: apply select box to condition node enum parameters#79
[PRO-93] feat: apply select box to condition node enum parameters#79fray-cloud merged 1 commit intodevfrom
Conversation
- Add `options?: string[]` to ParamDefinition type - Add options arrays to and-or (AND/OR), crossover (above/below), threshold operator (</>/<=/>==/==), market-order side (buy/sell), and rsi source (close/open/high/low) params - ParamInput renders <select> dropdown when options are present, showing localized labels from PARAM_VALUE_LABELS Co-Authored-By: Paperclip <noreply@paperclip.ing>
Reviewer's GuideAdds enum-like parameter options to flow node definitions and updates the node inspector to render those parameters as localized select dropdowns instead of free-form inputs where applicable. Sequence diagram for enum parameter selection in node inspectorsequenceDiagram
actor User
participant NodeInspector
participant ParamInput
participant BrowserDOM
participant FlowStore
User->>NodeInspector: Open_node_configuration
NodeInspector->>NodeInspector: Read_NodeTypeInfo_and_ParamDefinition
NodeInspector->>ParamInput: render(paramKey, value, onChange, options)
ParamInput->>BrowserDOM: Render_select_when_options_present
User->>BrowserDOM: Change_selected_option
BrowserDOM->>ParamInput: change_event_with_selected_value
ParamInput->>NodeInspector: onChange(selectedValue)
NodeInspector->>FlowStore: updateNodeConfig(nodeId, partialConfig)
FlowStore-->>NodeInspector: Updated_node_config_state
NodeInspector-->>User: Display_updated_parameter_value
Class diagram for updated flow parameter definitions and node inspectorclassDiagram
class ParamDefinition {
string key
boolean required
string[] options
}
class PortDefinition {
string name
string type
string direction
}
class NodeTypeInfo {
string subtype
PortDefinition[] inputs
PortDefinition[] outputs
any defaultConfig
ParamDefinition[] params
}
class NodeTypeRegistry {
NodeTypeInfo rsi
NodeTypeInfo threshold
NodeTypeInfo crossover
NodeTypeInfo and_or
NodeTypeInfo market_order
}
class NodeInspector {
+render()
+updateNodeConfig(nodeId, partialConfig)
}
class ParamInput {
+render(paramKey, value, onChange, options)
}
NodeTypeRegistry "1" --> "many" NodeTypeInfo : contains
NodeTypeInfo "1" --> "many" ParamDefinition : has
NodeTypeInfo "1" --> "many" PortDefinition : defines
NodeInspector "1" --> "many" ParamInput : renders
ParamInput --> ParamDefinition : uses_options
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- When rendering the
<select>, consider handling cases wherevalueis not in theoptionsarray (e.g., legacy or malformed configs) by providing a fallback option or default to avoid an empty/undefined visual state. - If you expect enum-like options to grow more complex over time (labels, descriptions, disabled states), it may be worth introducing a structured
ParamOptiontype instead of a barestring[]forParamDefinition.optionsto keep the API extensible.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When rendering the `<select>`, consider handling cases where `value` is not in the `options` array (e.g., legacy or malformed configs) by providing a fallback option or default to avoid an empty/undefined visual state.
- If you expect enum-like options to grow more complex over time (labels, descriptions, disabled states), it may be worth introducing a structured `ParamOption` type instead of a bare `string[]` for `ParamDefinition.options` to keep the API extensible.
## Individual Comments
### Comment 1
<location path="apps/web/src/components/flows/node-inspector.tsx" line_range="50-64" />
<code_context>
+ if (options && options.length > 0) {
+ return (
+ <select
+ value={String(value)}
+ onChange={(e) => onChange(e.target.value)}
+ className="rounded border border-border bg-background px-2 py-1 text-xs text-foreground outline-none focus:border-primary"
</code_context>
<issue_to_address>
**suggestion:** Stringifying `undefined` can yield `"undefined"` in the select value.
For optional params, `value` may briefly be `undefined` (e.g., during config updates), so `String(value)` becomes `'undefined'`, which won't match any option and can put the select in an odd state. Consider normalizing nullish values, e.g. `value={value == null ? '' : String(value)}`, and providing a corresponding placeholder option if appropriate.
```suggestion
if (options && options.length > 0) {
const hasEmptyOption = options.includes('');
const stringValue = value == null ? '' : String(value);
return (
<select
value={stringValue}
onChange={(e) => onChange(e.target.value)}
className="rounded border border-border bg-background px-2 py-1 text-xs text-foreground outline-none focus:border-primary"
>
{!hasEmptyOption && stringValue === '' && (
<option value="" disabled>
Select…
</option>
)}
{options.map((opt) => (
<option key={opt} value={opt}>
{PARAM_VALUE_LABELS[opt] ?? opt}
</option>
))}
</select>
);
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if (options && options.length > 0) { | ||
| return ( | ||
| <select | ||
| value={String(value)} | ||
| onChange={(e) => onChange(e.target.value)} | ||
| className="rounded border border-border bg-background px-2 py-1 text-xs text-foreground outline-none focus:border-primary" | ||
| > | ||
| {options.map((opt) => ( | ||
| <option key={opt} value={opt}> | ||
| {PARAM_VALUE_LABELS[opt] ?? opt} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| ); | ||
| } |
There was a problem hiding this comment.
suggestion: Stringifying undefined can yield "undefined" in the select value.
For optional params, value may briefly be undefined (e.g., during config updates), so String(value) becomes 'undefined', which won't match any option and can put the select in an odd state. Consider normalizing nullish values, e.g. value={value == null ? '' : String(value)}, and providing a corresponding placeholder option if appropriate.
| if (options && options.length > 0) { | |
| return ( | |
| <select | |
| value={String(value)} | |
| onChange={(e) => onChange(e.target.value)} | |
| className="rounded border border-border bg-background px-2 py-1 text-xs text-foreground outline-none focus:border-primary" | |
| > | |
| {options.map((opt) => ( | |
| <option key={opt} value={opt}> | |
| {PARAM_VALUE_LABELS[opt] ?? opt} | |
| </option> | |
| ))} | |
| </select> | |
| ); | |
| } | |
| if (options && options.length > 0) { | |
| const hasEmptyOption = options.includes(''); | |
| const stringValue = value == null ? '' : String(value); | |
| return ( | |
| <select | |
| value={stringValue} | |
| onChange={(e) => onChange(e.target.value)} | |
| className="rounded border border-border bg-background px-2 py-1 text-xs text-foreground outline-none focus:border-primary" | |
| > | |
| {!hasEmptyOption && stringValue === '' && ( | |
| <option value="" disabled> | |
| Select… | |
| </option> | |
| )} | |
| {options.map((opt) => ( | |
| <option key={opt} value={opt}> | |
| {PARAM_VALUE_LABELS[opt] ?? opt} | |
| </option> | |
| ))} | |
| </select> | |
| ); | |
| } |
|
코드 리뷰 완료. LGTM ✅ 검토 결과:
머지 진행합니다. |
Summary
options?: string[]toParamDefinitiontype so node parameters can declare fixed enum values<select>dropdown inParamInputwhenoptionsare present (localized viaPARAM_VALUE_LABELS)Changes
packages/types/src/flow.ts— addoptionsfield toParamDefinitionapps/web/src/components/flows/node-inspector.tsx— conditional<select>render + Korean labelsRelated
🤖 Generated with Claude Code
Summary by Sourcery
Introduce enum-style parameter options to flow nodes and render them as select dropdowns in the node inspector UI.
New Features:
Enhancements: