Fix missing TEXT_SET editors in service creation forms#1533
Merged
kanushka merged 2 commits intowso2:release/bi-1.8.xfrom Feb 20, 2026
Merged
Fix missing TEXT_SET editors in service creation forms#1533kanushka merged 2 commits intowso2:release/bi-1.8.xfrom
kanushka merged 2 commits intowso2:release/bi-1.8.xfrom
Conversation
Contributor
📝 WalkthroughWalkthroughThese changes introduce a new DynamicArrayBuilder component to handle array input modes (ARRAY and TEXT_ARRAY) in the expression editor, integrate it into ExpressionField, refine input type fallback logic in FieldFactory, and fix a trailing semicolon in a utility file. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExpressionField
participant DynamicArrayBuilder
participant ChipExpressionEditorComponent
participant FormContext
User->>ExpressionField: Interact with array input
ExpressionField->>ExpressionField: Detect InputMode.ARRAY or TEXT_ARRAY
ExpressionField->>DynamicArrayBuilder: Render with label, value, onChange
DynamicArrayBuilder->>DynamicArrayBuilder: Initialize array from various value shapes
DynamicArrayBuilder->>DynamicArrayBuilder: Pad array to meet minItems/defaultItems
DynamicArrayBuilder->>FormContext: Set initial array value
User->>DynamicArrayBuilder: Edit array item
DynamicArrayBuilder->>ChipExpressionEditorComponent: Render per-item editor
ChipExpressionEditorComponent->>User: Display editor UI
User->>ChipExpressionEditorComponent: Submit item value
ChipExpressionEditorComponent->>DynamicArrayBuilder: Return edited value
DynamicArrayBuilder->>FormContext: Update form state with new array
FormContext->>DynamicArrayBuilder: Validate and update dirty flag
User->>DynamicArrayBuilder: Add/Remove items
DynamicArrayBuilder->>DynamicArrayBuilder: Enforce minimum item constraints
DynamicArrayBuilder->>FormContext: Sync updated array
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kanushka
requested review from
LakshanWeerasinghe,
axewilledge and
senithkay
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/DynamicArrayBuilder/DynamicArrayBuilder.tsx (2)
168-199: Editor configuration objects are instantiated per-item on every render.
new StringTemplateEditorConfig()andnew ChipExpressionEditorDefaultConfiguration()are constructed inside.map()on every render cycle. IfChipExpressionEditorComponentperforms shallow-equality checks on itsconfigurationprop (e.g. viaReact.memo), every render forces a full re-render of all list items regardless of value changes.♻️ Proposed fix — hoist to a memo
+ const editorConfig = useMemo(() => + expressionSetType?.fieldType === "TEXT_SET" + ? new StringTemplateEditorConfig() + : new ChipExpressionEditorDefaultConfiguration(), + [expressionSetType?.fieldType] + ); return ( <S.Container> {arrayValues.map((itemValue, index) => ( <S.ItemContainer key={`${expressionFieldProps.field.key}-${index}`}> <ChipExpressionEditorComponent ... - configuration={expressionSetType?.fieldType === "TEXT_SET" ? new StringTemplateEditorConfig() : new ChipExpressionEditorDefaultConfiguration()} + configuration={editorConfig} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/DynamicArrayBuilder/DynamicArrayBuilder.tsx` around lines 168 - 199, The configuration objects are being re-created inside arrayValues.map which breaks shallow equality checks for ChipExpressionEditorComponent; hoist the construction out of the loop by creating a memoized config (using React.useMemo) before rendering the map and compute a single instance based on expressionSetType?.fieldType (e.g., memoize either new StringTemplateEditorConfig() or new ChipExpressionEditorDefaultConfiguration() with expressionSetType?.fieldType as the dependency), then pass that memoized configuration into ChipExpressionEditorComponent instead of instantiating it inline.
29-34:labelis declared but never used;onChangeis accepted but intentionally bypassed.Both create a misleading public interface:
labelis passed from the call site (label={field.label}) but never referenced in the component body — remove it from both the interface and the call site, or wire it to anaria-label/ visible heading.onChangeis silently ignored in favour ofsetValue(the comment at line 55 explains why). Consider dropping it from the interface altogether to make the contract explicit, since callers (e.g. ExpressionField.tsx line 161) construct a wrapper that is never invoked.♻️ Suggested cleanup
interface DynamicArrayBuilderProps { - label: string; value: string | any[]; - onChange?: (value: string) => void; expressionFieldProps: ExpressionFieldProps; }And at the call site in ExpressionField.tsx:
<DynamicArrayBuilder - label={field.label} value={value} - onChange={(val) => onChange(val, val.length)} expressionFieldProps={props} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/DynamicArrayBuilder/DynamicArrayBuilder.tsx` around lines 29 - 34, The DynamicArrayBuilderProps interface exposes unused props: remove the unused label and the misleading onChange from the public contract and callers, or wire label into the component (e.g., as aria-label/heading) if a visible/accessibility label is required; similarly, either invoke the provided onChange wrapper instead of bypassing it or delete onChange from DynamicArrayBuilderProps and update call sites (notably where ExpressionField constructs a wrapper at ExpressionField.tsx line ~161) so callers no longer pass it; ensure refs to setValue remain internal to DynamicArrayBuilder and update the prop types and all usages consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/DynamicArrayBuilder/DynamicArrayBuilder.tsx`:
- Around line 186-198: The delete-button disables threshold is inconsistent with
the auto-padding logic: compute a single derived minimum (e.g., const minVisible
= useMemo(() => Math.max(minItems, defaultItems), [minItems, defaultItems]))
inside the DynamicArrayBuilder component and use this minVisible in both the
padding effect that auto-adds items and in the S.DeleteButton disabled prop
(replace arrayValues.length <= minItems with arrayValues.length <= minVisible)
so deletion and re-padding thresholds match; keep handleDelete and existing
padding logic but reference minVisible everywhere instead of
minItems/defaultItems separately.
---
Nitpick comments:
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/DynamicArrayBuilder/DynamicArrayBuilder.tsx`:
- Around line 168-199: The configuration objects are being re-created inside
arrayValues.map which breaks shallow equality checks for
ChipExpressionEditorComponent; hoist the construction out of the loop by
creating a memoized config (using React.useMemo) before rendering the map and
compute a single instance based on expressionSetType?.fieldType (e.g., memoize
either new StringTemplateEditorConfig() or new
ChipExpressionEditorDefaultConfiguration() with expressionSetType?.fieldType as
the dependency), then pass that memoized configuration into
ChipExpressionEditorComponent instead of instantiating it inline.
- Around line 29-34: The DynamicArrayBuilderProps interface exposes unused
props: remove the unused label and the misleading onChange from the public
contract and callers, or wire label into the component (e.g., as
aria-label/heading) if a visible/accessibility label is required; similarly,
either invoke the provided onChange wrapper instead of bypassing it or delete
onChange from DynamicArrayBuilderProps and update call sites (notably where
ExpressionField constructs a wrapper at ExpressionField.tsx line ~161) so
callers no longer pass it; ensure refs to setValue remain internal to
DynamicArrayBuilder and update the prop types and all usages consistently.
Comment on lines
+186
to
+198
| //have a switch to show the array editor mode and the expression mode. | ||
| //Exception: TEXT_SET uses StringTemplateEditorConfig for TEXT mode | ||
| configuration={expressionSetType?.fieldType === "TEXT_SET" ? new StringTemplateEditorConfig() : new ChipExpressionEditorDefaultConfiguration()} | ||
| placeholder={expressionFieldProps.field.placeholder} | ||
| /> | ||
| <S.DeleteButton | ||
| appearance="icon" | ||
| onClick={() => handleDelete(index)} | ||
| disabled={arrayValues.length <= minItems} | ||
| > | ||
| <Codicon name="trash" /> | ||
| </S.DeleteButton> | ||
| </S.ItemContainer> |
Contributor
There was a problem hiding this comment.
Delete-button minimum (minItems) is inconsistent with the padding minimum (Math.max(minItems, defaultItems)).
When defaultItems > minItems the delete button stays enabled until the array hits minItems, but the padding effect at line 136 immediately re-adds items up to Math.max(minItems, defaultItems). The net result is that clicking Delete appears to succeed (the item disappears momentarily) and then the item re-appears — a confusing experience.
The disable threshold should match the padding threshold:
🐛 Proposed fix
-const requiredCount = Math.max(minItems, defaultItems);
+const minVisible = Math.max(minItems, defaultItems);
...
- disabled={arrayValues.length <= minItems}
+ disabled={arrayValues.length <= minVisible}(Declare minVisible with useMemo alongside minItems/defaultItems, then use it in both the padding effect and the disabled prop.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/DynamicArrayBuilder/DynamicArrayBuilder.tsx`
around lines 186 - 198, The delete-button disables threshold is inconsistent
with the auto-padding logic: compute a single derived minimum (e.g., const
minVisible = useMemo(() => Math.max(minItems, defaultItems), [minItems,
defaultItems])) inside the DynamicArrayBuilder component and use this minVisible
in both the padding effect that auto-adds items and in the S.DeleteButton
disabled prop (replace arrayValues.length <= minItems with arrayValues.length <=
minVisible) so deletion and re-padding thresholds match; keep handleDelete and
existing padding logic but reference minVisible everywhere instead of
minItems/defaultItems separately.
LakshanWeerasinghe
approved these changes
Feb 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This fixes a regression where
TEXT_SETfields (for example,DatabasesandSchemasin Microsoft SQL Server CDC Integration) showed only label/description without input editors in the new BI form flow.Screen.Recording.2026-02-20.at.20.54.34.mov
Root cause
During side-panel editor cleanup, the dynamic array editor path for
TEXT_SET/EXPRESSION_SETwas removed fromExpressionField.As a result, array-backed values resolved to no rendered editor.
Also, type mode selection depended on
selectedflags, which can be allfalsein some refactored models.Fix
DynamicArrayBuilderusage forInputMode.ARRAYandInputMode.TEXT_ARRAYin:workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/DynamicArrayBuilder/DynamicArrayBuilder.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/FieldFactory.tsxtypes.length === 1, use that type regardless ofselectedselected === false, fallback to last type (typically expression mode)Notes
This aligns with the previous editor behavior before the side-panel cleanup refactor where the same
DynamicArrayBuilderpath was used for set-like array fields.Related Task
Summary by CodeRabbit
Bug Fixes
New Features