ui: add scenario reorder buttons for condition agent#5720
ui: add scenario reorder buttons for condition agent#5720TianHougege wants to merge 1 commit intoFlowiseAI:mainfrom
Conversation
Summary of ChangesHello @TianHougege, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the usability of the 'Condition Agent' component by adding intuitive reordering capabilities for its scenarios. By providing simple Up/Down controls, users can now easily adjust the sequence of their conditional logic. The implementation is robust, focusing on stability during UI updates and ensuring the feature is applied precisely where needed without unintended side effects on other array-based inputs. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces reordering functionality for Condition Agent scenarios using simple up/down buttons. The implementation correctly uses stable keys to prevent rendering issues during reordering, which is a great approach. My review includes a couple of suggestions to improve the code's design and maintainability. Specifically, I recommend refactoring some duplicated code into a helper function and suggest a more flexible, prop-based approach to enable the reorder feature, which would make the ArrayRenderer component more reusable.
| const isConditionAgentNode = data?.name === 'conditionAgentV2' || data?.name === 'conditionAgentAgentflow' | ||
|
|
||
| // Whitelist scenario field names across variants (keep this minimal) | ||
| // Include both legacy and UI field name for compatibility | ||
| const scenarioFieldNames = new Set(['conditionAgentScenarios', 'ConditionAgentScenarios']) | ||
|
|
||
| const isScenarioField = scenarioFieldNames.has(inputParam?.name) | ||
| const enableReorder = !isDocStore && isConditionAgentNode && isScenarioField |
There was a problem hiding this comment.
Hardcoding node names (conditionAgentV2, conditionAgentAgentflow) and input parameter names (conditionAgentScenarios, ConditionAgentScenarios) makes the ArrayRenderer component less reusable and tightly coupled to specific nodes. A more flexible approach would be to control this behavior via a prop on the inputParam object, for example reorderable: true. This would decouple ArrayRenderer and make it a more generic component that can be used for any array input that needs reordering capabilities.
There was a problem hiding this comment.
@TianHougege can we make this change? Adding reorderable to ConditionAgents.ts so in the inputParam will have it and just have enableReorder to be
const enableReorder = !isDocStore && !disabled && !!inputParam?.reorderable
This allows us to be more dynamic and not have specific hardcoded nodes to be reorderable
we can also remove
const isConditionAgentNode = data?.name === 'conditionAgentV2' || data?.name === 'conditionAgentAgentflow'
const scenarioFieldNames = new Set(['conditionAgentScenarios', 'ConditionAgentScenarios'])
const isScenarioField = scenarioFieldNames.has(inputParam?.name)
as those won't be necessary anymore.
| const swapInArray = (arr, i, j) => { | ||
| const next = [...arr] | ||
| ;[next[i], next[j]] = [next[j], next[i]] | ||
| return next | ||
| } |
There was a problem hiding this comment.
To avoid duplicating the key generation logic found in useEffect (lines 130-141) and handleAddItem (lines 221-229), you can extract it into a single createKey helper function here. This function can then be reused in both places, improving code reuse and maintainability.
const swapInArray = (arr, i, j) => {
const next = [...arr]
;[next[i], next[j]] = [next[j], next[i]]
return next
}
const createKey = () => {
try {
return crypto.randomUUID()
} catch (e) {
return 'key-' + Date.now() + '-' + Math.random()
}
}There was a problem hiding this comment.
Can we make this change as well?
There was a problem hiding this comment.
Hello @TianHougege, thanks for making this change, I have a few suggestions I have listed out.
There's also a small visual bug if you can have time to take a look. Pressing 'up' button is also affecting the 'up' button above it.
VisualBug.mp4
@HenryHengZJ any thoughts on this? Long-term, drag and drop might be the more standard UI pattern, but this approach works well and is also more accessible for keyboard users.
| const isConditionAgentNode = data?.name === 'conditionAgentV2' || data?.name === 'conditionAgentAgentflow' | ||
|
|
||
| // Whitelist scenario field names across variants (keep this minimal) | ||
| // Include both legacy and UI field name for compatibility | ||
| const scenarioFieldNames = new Set(['conditionAgentScenarios', 'ConditionAgentScenarios']) | ||
|
|
||
| const isScenarioField = scenarioFieldNames.has(inputParam?.name) | ||
| const enableReorder = !isDocStore && isConditionAgentNode && isScenarioField |
There was a problem hiding this comment.
@TianHougege can we make this change? Adding reorderable to ConditionAgents.ts so in the inputParam will have it and just have enableReorder to be
const enableReorder = !isDocStore && !disabled && !!inputParam?.reorderable
This allows us to be more dynamic and not have specific hardcoded nodes to be reorderable
we can also remove
const isConditionAgentNode = data?.name === 'conditionAgentV2' || data?.name === 'conditionAgentAgentflow'
const scenarioFieldNames = new Set(['conditionAgentScenarios', 'ConditionAgentScenarios'])
const isScenarioField = scenarioFieldNames.has(inputParam?.name)
as those won't be necessary anymore.
| if (isDocStore || !reactFlowInstance) return | ||
|
|
||
| if (data.name !== 'conditionAgentflow' && data.name !== 'conditionAgentAgentflow') return | ||
| if (!isConditionAgentNode) return |
There was a problem hiding this comment.
Can probably revert this back once reorderable is implemented.
Also I think conditionAgentV2 isn't used anywhere else in the project so probably a typo.
| const swapInArray = (arr, i, j) => { | ||
| const next = [...arr] | ||
| ;[next[i], next[j]] = [next[j], next[i]] | ||
| return next | ||
| } |
There was a problem hiding this comment.
Can we make this change as well?
|
|
||
| //scenario array swap function | ||
| const moveItem = (fromIndex, toIndex) => { | ||
| if (!enableReorder) return |
There was a problem hiding this comment.
I think there are a lot of redundant guards here, below is enough.
const moveItem = (fromIndex, toIndex) => {
const nextItems = swapInArray(arrayItems, fromIndex, toIndex)
const nextKeys = swapInArray(itemKeys, fromIndex, toIndex)
const canSwapParams = itemParameters.length === arrayItems.length
const nextParams = canSwapParams ? swapInArray(itemParameters, fromIndex, toIndex) : itemParameters
setArrayItems(nextItems)
if (canSwapParams) setItemParameters(nextParams)
setItemKeys(nextKeys)
data.inputs[inputParam.name] = nextItems
updateOutputAnchors(nextItems, 'REORDER')
}
moveItem is only called by handleMoveUp and handleMoveDown, which already enforce bounds. The buttons only render when enableReorder is true and are disabled when disabled is true. All 6 guards duplicate checks that are already enforced by the callers and JSX.
What
Add lightweight Up/Down controls to reorder Condition Agent scenarios.
Why
Up/Down buttons keep the behavior simple and predictable (no extra drag-and-drop deps).
Notes
inputParam.name), so other array inputs are unaffected.itemKeys) during reorder to avoid input/focus mismatch caused bykey={index}.Demo
Kapture.2026-02-08.at.13.07.51.mp4
Closes #5699