Introduce String Template Support #1012
Conversation
WalkthroughIntroduce a configuration layer for multi-mode expression editors, add mode-aware config classes, refactor Text/Record/Template editors to use configurations, update ChipExpressionEditor to use serialization/deserialization and helper logic from configs, and adjust token/selection utilities and template-interpolation helpers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant ExpressionEditor
participant ExpressionField
participant Config as ConfigClass
participant ChipEditor
participant TokenUtils
User->>ExpressionEditor: edit / change mode
ExpressionEditor->>ExpressionField: render(primaryMode)
ExpressionField->>Config: select / new Config(primaryMode)
ExpressionField->>ChipEditor: render(configuration)
User->>ChipEditor: type / select helper
ChipEditor->>TokenUtils: isSelectionOnToken(from,to,view)
TokenUtils-->>ChipEditor: token | undefined
ChipEditor->>Config: getHelperValue(value, token)
Config-->>ChipEditor: helperText
ChipEditor->>Config: deserializeValue(serialized)
Config-->>ChipEditor: deserializedValue
ChipEditor->>ExpressionEditor: onChange(deserializedValue)
ExpressionEditor->>User: update UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (9)📚 Learning: 2025-11-24T22:16:28.380ZApplied to files:
📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
📚 Learning: 2025-11-24T14:51:49.267ZApplied to files:
📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
📚 Learning: 2025-11-25T06:34:10.812ZApplied to files:
📚 Learning: 2025-11-27T07:59:33.522ZApplied to files:
🧬 Code graph analysis (2)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsx (2)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (4)
🔇 Additional comments (7)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/utils/utils.ts (1)
21-24: Update helper name/comment or confirm behavior change for template interpolation
wrapInTemplateInterpolationnow just returnsvalueand ignores_inputMode, but the name and comment still suggest${}wrapping in template mode. That’s a bit misleading and makes it hard to tell whether call sites still depend on interpolation happening here vs in the new config layer.Either update the comment/name to reflect the no-op behavior or restore interpolation if any consumers still expect it. Also worth double‑checking all callers to ensure the removal of wrapping is fully intentional.
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/DocumentConfig.tsx (1)
47-49: Document wrapping no longer depends on template mode; clean up comments and unused code, and verify syntaxA couple of things to flag together:
wrapInDocumentTypenow always emits<${documentType}>{content: ${content}}with no template interpolation; this is fine if interpolation is now entirely handled at the editor/config layer, but then:
- The comment in
handleItemSelect(“wrap in string interpolation only in template mode”) is no longer accurate.- The template vs non‑template branches for AI document types are effectively identical (
onChange(fullPath, false)either way).- In
URLInputModal.handleCreate,isTemplateModeis computed but never used; you can drop that local.I’d update the comments to reflect that this helper always returns a raw document expression (and doesn’t care about
InputMode), remove the unusedisTemplateModevariable, and double‑check that, for TEMPLATE fields, whatever consumesonChange(wrappedValue, …)still produces valid Ballerina string‑template syntax around the<…>{content: …}snippet.Also applies to: 146-178, 206-215
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpandedEditor/modes/TemplateMode.tsx (1)
29-29: TemplateMode correctly wires inRawTemplateEditorConfigPassing
configuration={new RawTemplateEditorConfig()}intoChipExpressionEditorComponenthere aligns TemplateMode with the new configuration-driven editing model and should give you consistent string-template behavior between minimized and expanded views.If
RawTemplateEditorConfigever gains state or becomes heavier, consider memoizing the instance, but as-is this looks fine.Also applies to: 137-155
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RecordConfigPreviewEditor/RecordConfigPreviewEditor.tsx (1)
41-97: Tighten prop typing and confirm quote-wrapping semantics inRecordConfigPreviewEditorTwo points here:
Props typing
RecordConfigPreviewEditorPropsis defined as:type RecordConfigPreviewEditorProps = Pick<ExpressionField, 'name' | 'value' | ...>;This only type-checks if there is a type/interface
ExpressionFieldexported from../../ExpressionField. IfExpressionFieldis just the React component (which is how it’s used inExpressionEditor.tsx), it’d be clearer and safer to base this on the component props instead, e.g.:type RecordConfigPreviewEditorProps = Pick<React.ComponentProps<typeof ExpressionField>, 'name' | 'value' | ...>;or on a dedicated
ExpressionFieldPropstype if one exists.Quote wrapping on change
handleOnChangealways wraps the edited value in double quotes before callingonChange:const newValue = "\"" + value + "\""; onChange(newValue, updatedCursorPosition);That’s fine if
getValueForTextModeEditoralways strips a single pair of outer quotes and the stored representation is always exactly"foo". It’d be good to confirm:
- There’s no path where
valuealready includes quotes, which would lead to""foo"".- Any escaping rules for embedded quotes are handled consistently elsewhere.
If those invariants hold, the component’s behavior is consistent; otherwise you may want to centralize the quote add/remove logic in a shared helper instead of duplicating it here.
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx (1)
673-719: Verify newExpressionFieldwiring and inlineonChangebehavior in TEMPLATE modeIn the
Controllerrender, two notable changes:
New
primaryModeprop<ExpressionField inputMode={inputMode} primaryMode={getInputModeFromTypes(field.valueTypeConstraint)} ... />This gives
ExpressionFielddirect access to the “natural” mode from the type constraint, which is good. Just make sure its prop type allowsundefinedifgetInputModeFromTypesreturns nothing (e.g., unconstrained or unsupported types).Inline
onChangenow usesupdatedValueeverywhereonChange(updatedValue); ... getExpressionEditorDiagnostics( (required ?? !field.optional) || updatedValue !== '', updatedValue, ... ); ... const triggerCharacter = updatedCursorPosition > 0 ? triggerCharacters.find((char) => updatedValue[updatedCursorPosition - 1] === char) : undefined; ... await retrieveCompletions(updatedValue, ..., updatedCursorPosition, triggerCharacter?);This is a deliberate shift from the previous
rawExpression-based flow, and it lines up with the new configuration-driven string/template handling. However, it does mean that diagnostics and completions in TEMPLATE mode now operate on whatever representationExpressionFieldsurfaces asupdatedValue, while the expanded editor path still derives and uses arawValue.It’d be good to re-validate, in TEMPLATE mode specifically, that:
- LS diagnostics/completions behave as expected when fed
updatedValuefrom the inline editor.- Switching between inline and expanded editors doesn’t cause observable inconsistencies in how the same expression is parsed or tokenized.
If those checks pass, the new wiring looks consistent with the rest of the refactor.
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx (1)
183-183: Consider memoizing configuration instances.New configuration instances are created on every render. While these are lightweight objects, memoizing them would be more efficient and prevent unnecessary re-renders in child components that depend on reference equality.
const stringTemplateConfig = useMemo(() => new StringTemplateEditorConfig(), []); const rawTemplateConfig = useMemo(() => new RawTemplateEditorConfig(), []); // For ChipExpressionEditorConfig, include primaryMode in dependencies if moved outsideAlso applies to: 204-204, 225-225
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx (1)
35-50: Duplicate serialization logic between configurations.
serializeValueanddeserializeValueimplementations are identical betweenStringTemplateEditorConfigandRawTemplateEditorConfig. Since they already callthis.getSerializationPrefix()andthis.getSerializationSuffix(), consider moving these methods to the base classChipExpressionEditorDefaultConfigurationto reduce duplication.Also applies to: 65-80
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpandedEditor/modes/TemplateMode.tsx(2 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx(4 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx(5 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/ChipExpressionDefaultConfig.ts(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts(2 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx(8 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RecordConfigPreviewEditor/RecordConfigPreviewEditor.tsx(1 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsx(2 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/DocumentConfig.tsx(3 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/utils/utils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.{ts,tsx} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts
📚 Learning: 2025-11-24T22:16:28.380Z
Learnt from: dan-niles
Repo: wso2/vscode-extensions PR: 980
File: workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RichTextTemplateEditor/RichTextTemplateEditor.tsx:384-412
Timestamp: 2025-11-24T22:16:28.380Z
Learning: In the RichTextTemplateEditor component (workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RichTextTemplateEditor/RichTextTemplateEditor.tsx), token fetching on external `value` prop changes is intentionally disabled. Users cannot edit both the minimized and expanded editors simultaneously, so tokens only need to be generated based on user typing in the expanded editor view, not on external prop updates.
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.tsworkspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/utils/utils.tsworkspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.tsworkspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RecordConfigPreviewEditor/RecordConfigPreviewEditor.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/ExpandedEditor/modes/TemplateMode.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/**/*.tsx : Use React 18.2.0 features including concurrent rendering and automatic batching; avoid class components in favor of functional components with hooks
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RecordConfigPreviewEditor/RecordConfigPreviewEditor.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/**/*.tsx : Implement lazy expansion of functions: only render function items when they are visible to improve performance for large function lists
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RecordConfigPreviewEditor/RecordConfigPreviewEditor.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsx
📚 Learning: 2025-11-24T14:51:49.267Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 998
File: workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx:113-132
Timestamp: 2025-11-24T14:51:49.267Z
Learning: In workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx, if `textFieldRef.current` is not undefined, `textFieldRef.current.inputElement` is guaranteed to exist. If `inputElement` doesn't exist when `current` exists, it's a fatal error that should reach the error boundary rather than being handled with defensive null checks.
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx
📚 Learning: 2025-11-26T06:34:09.752Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:128-134
Timestamp: 2025-11-26T06:34:09.752Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBreadcrumb() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/DocumentConfig.tsx
📚 Learning: 2025-11-26T07:49:56.428Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:136-141
Timestamp: 2025-11-26T07:49:56.428Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBackButton() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/DocumentConfig.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/**/*.tsx : Use Emotion for CSS-in-JS styling instead of inline styles or external CSS files
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsx
🧬 Code graph analysis (7)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts (3)
getParsedExpressionTokens(111-137)detectTokenPatterns(277-331)ParsedToken(104-109)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts (1)
ParsedToken(104-109)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/ChipExpressionDefaultConfig.ts (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts (1)
ParsedToken(104-109)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx (4)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx (2)
StringTemplateEditorConfig(23-51)RawTemplateEditorConfig(53-81)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts (1)
ParsedToken(104-109)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RecordConfigPreviewEditor/RecordConfigPreviewEditor.tsx (1)
RecordConfigPreviewEditor(43-98)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsx (1)
TextModeEditor(43-72)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpandedEditor/modes/TemplateMode.tsx (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx (1)
RawTemplateEditorConfig(53-81)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsx (2)
workspaces/ballerina/ballerina-side-panel/src/components/editors/utils.ts (1)
getValueForTextModeEditor(98-107)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/FloatingToggleButton.tsx (1)
FloatingToggleButton(30-45)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx (3)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts (1)
getInputModeFromTypes(42-61)workspaces/ballerina/ballerina-side-panel/src/components/editors/index.ts (2)
InputMode(31-31)getPropertyFromFormField(30-30)workspaces/ballerina/ballerina-side-panel/src/components/editors/utils.ts (1)
getPropertyFromFormField(64-81)
🔇 Additional comments (9)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts (1)
22-26: Exporting token index constants looks goodMaking the
TOKEN_*_INDEXconstants public for reuse keeps the encoding of token streams centralized and avoids magic indices in other modules. The numeric values still match the existingTOKEN_TYPE_INDEX_MAP/chunk layout, so behavior stays intact.workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts (2)
223-243: Token parsing now correctly uses the post-transaction documentIn the
tokensChangeEffectbranch oftokenField.update, switching to:const currentValue = tr.newDoc.toString(); tokens = getParsedExpressionTokens(payload.tokens, currentValue); compounds = detectTokenPatterns(tokens, currentValue);keeps parsed token ranges and detected compounds aligned with the actual editor content after the transaction, which is what downstream consumers (chips, selection logic) expect.
This looks like a solid simplification compared to carrying around separate raw/sanitized values.
609-622:isSelectionOnTokenbehavior is well-scoped; confirmtokenFieldis always present when used
isSelectionOnToken:
- Ignores selections that exactly match a compound’s
[start, end)range.- Returns a token only when the selection exactly matches that token’s
[start, end)range.- Otherwise returns
undefined.That aligns nicely with a UX where compound chips are treated as indivisible, and only “bare” tokens can be manipulated via this helper.
One thing to keep in mind:
view.state.field(tokenField)will throw if thetokenFieldextension isn’t present in the state. Make sure this helper is only ever called in editors that have registeredtokenFieldin their extensions (which seems true for the ChipExpressionEditor path), or switch toview.state.field(tokenField, false)with a guard if you anticipate reuse elsewhere.workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionEditor.tsx (1)
436-453: Input mode derivation from valueTypeConstraint looks consistentThe updated effect:
let newInputMode = getInputModeFromTypes(field.valueTypeConstraint); if (isModeSwitcherRestricted()) { setInputMode(InputMode.EXP); return; } if (!newInputMode) { setInputMode(InputMode.EXP); return; } setInputMode(newInputMode);is a nice simplification: it keeps RECORD mode special-cased, respects mode-switcher restrictions, and otherwise defers to the type constraint mapping, with a sensible EXP fallback.
Given
inputModeRefis kept in sync via the separateuseEffect, the rest of the component continues to see the right mode.workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/ChipExpressionDefaultConfig.ts (1)
21-37: I encountered a repository clone failure, which prevents me from directly verifying the code. However, based on TypeScript's typing rules and the web search results, I need to verify specific details about the actual implementation:To complete verification, I need you to provide:
- The actual return type annotation (if any) on
getHelperValueinChipExpressionEditorDefaultConfiguration- The full implementation of
RawTemplateEditorConfig.getHelperValue()fromConfigurations.tsx- Whether TypeScript compilation currently produces errors for this code (and if so, which error codes)
Alternatively, if you have access to the repository, please share:
- The contents of
ChipExpressionDefaultConfig.ts(lines 1-40)- The contents of
Configurations.tsxshowing theRawTemplateEditorConfigclassThis will allow me to verify whether the original review comment's claims about the typing contract mismatch are accurate.
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (2)
346-353: Potential infinite update loop risk.The logic at lines 348-353 calls
props.onChangewhendeserializeValue !== props.value, which triggers a re-render with the new value. If the configuration'sdeserializeValueimplementation doesn't stabilize (i.e.,deserializeValue(deserializeValue(x)) !== deserializeValue(x)), this could cause an infinite loop.Verify that all configuration implementations are idempotent—calling
deserializeValueon an already-deserialized value should return the same value.
99-100: LGTM on configuration defaulting.Using destructuring with a default value for the configuration prop is clean and ensures the component always has a valid configuration instance.
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsx (1)
43-70: LGTM on the component refactor.The simplification to delegate rendering to
ChipExpressionEditorComponentwith configuration-driven behavior is a clean approach. The value transformation viagetValueForTextModeEditoris correctly preserved, and props are properly forwarded.workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx (1)
83-100: I'll help you verify the review comment about the unusedEditorRibboncomponent. Let me search the codebase to confirm whether it's truly unused.
<function_calls>
#!/bin/bashSearch for EditorRibbon usage across the entire codebase
rg "EditorRibbon" -n --type=tsx --type=ts --type=jsx --type=js
</function_calls>Let me also check the file structure to understand the context better:
<function_calls>
#!/bin/bashFirst, find the ExpressionField.tsx file
fd "ExpressionField.tsx" -type f
Also check if there are any barrel exports or re-exports
fd "index.tsx" -o "index.ts" | head -20
</function_calls>
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx
Outdated
Show resolved
Hide resolved
...s/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx
Outdated
Show resolved
Hide resolved
...ina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx
Show resolved
Hide resolved
...ina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx
Show resolved
Hide resolved
...nel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsx
Outdated
Show resolved
Hide resolved
...nel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx (1)
136-144: Move class definition outside the component (duplicate concern).As previously noted, defining
ChipExpressionEditorConfiginside the component creates a new class definition on every render, which is inefficient and unconventional.Move this class to
Configurations.tsxalongside the other configuration classes, or define it at module scope. Since the class needs access toprimaryMode, modify its API to acceptprimaryModeas a constructor parameter:class ChipExpressionEditorConfig extends ChipExpressionEditorDefaultConfiguration { constructor(private primaryMode: InputMode) { super(); } getHelperValue(value: string, token?: ParsedToken): string { const isTextOrTemplateMode = this.primaryMode === InputMode.TEXT || this.primaryMode === InputMode.TEMPLATE; if (isTextOrTemplateMode && (!token || token.type !== TokenType.FUNCTION)) { return `\$\{${value}\}`; } return value; } }Then at line 225, instantiate it with the prop:
configuration={new ChipExpressionEditorConfig(primaryMode)}Based on past review comments and coding best practices.
🧹 Nitpick comments (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx (1)
225-225: Consider memoizing the configuration instance.This instantiation creates a new
ChipExpressionEditorConfiginstance on every render. Once the class is moved outside the component (as noted in lines 136-144), consider memoizing the instantiation:const config = useMemo(() => new ChipExpressionEditorConfig(primaryMode), [primaryMode]);Then use:
configuration={config}This prevents unnecessary re-instantiation when other props change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx(5 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-11-24T22:16:28.380Z
Learnt from: dan-niles
Repo: wso2/vscode-extensions PR: 980
File: workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RichTextTemplateEditor/RichTextTemplateEditor.tsx:384-412
Timestamp: 2025-11-24T22:16:28.380Z
Learning: In the RichTextTemplateEditor component (workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RichTextTemplateEditor/RichTextTemplateEditor.tsx), token fetching on external `value` prop changes is intentionally disabled. Users cannot edit both the minimized and expanded editors simultaneously, so tokens only need to be generated based on user typing in the expanded editor view, not on external prop updates.
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx
📚 Learning: 2025-11-24T14:51:49.267Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 998
File: workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx:113-132
Timestamp: 2025-11-24T14:51:49.267Z
Learning: In workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx, if `textFieldRef.current` is not undefined, `textFieldRef.current.inputElement` is guaranteed to exist. If `inputElement` doesn't exist when `current` exists, it's a fatal error that should reach the error boundary rather than being handled with defensive null checks.
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.{ts,tsx} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/**/*.tsx : Use React 18.2.0 features including concurrent rendering and automatic batching; avoid class components in favor of functional components with hooks
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/**/*.tsx : Use Emotion for CSS-in-JS styling instead of inline styles or external CSS files
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/nodes/EntryNode/components/**/*.tsx : Implement lazy expansion of functions: only render function items when they are visible to improve performance for large function lists
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx
🧬 Code graph analysis (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx (5)
workspaces/ballerina/ballerina-side-panel/src/components/editors/index.ts (1)
InputMode(31-31)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx (3)
ChipExpressionEditorConfig(81-86)StringTemplateEditorConfig(23-50)RawTemplateEditorConfig(52-79)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts (1)
ParsedToken(104-109)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RecordConfigPreviewEditor/RecordConfigPreviewEditor.tsx (1)
RecordConfigPreviewEditor(43-98)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/TextExpressionEditor/TextModeEditor.tsx (1)
TextModeEditor(43-72)
🔇 Additional comments (5)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx (5)
30-30: LGTM!The new imports are clean and necessary for the configuration-driven functionality.
Also applies to: 34-37
41-41: LGTM!The
primaryModeprop enables dynamic configuration of the default editor's behavior based on the primary mode context.
145-166: LGTM!The RECORD mode integration is clean and passes all necessary props to
RecordConfigPreviewEditor.
167-187: LGTM!The TEXT mode rendering correctly uses
StringTemplateEditorConfigto handle string template wrapping and variable insertion.
188-208: LGTM!The TEMPLATE mode rendering correctly uses
RawTemplateEditorConfigto handle raw template wrapping and variable insertion.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (1)
346-374: Avoid mutatingtokenStreamfrom the RPC manager and clarify serialized vs. deserialized usageTwo points here:
- Mutation of the original token stream
prefixCorrectedTokenStreamis currently just an alias oftokenStream, so decrementingTOKEN_START_CHAR_OFFSET_INDEXmutates the array returned byexpressionEditorRpcManager.getExpressionTokens(). If that result is cached or reused, this can cause subtle bugs.You can fix this by copying before adjustment:
- const tokenStream = await expressionEditorRpcManager?.getExpressionTokens( - deserializeValue, + const tokenStream = await expressionEditorRpcManager?.getExpressionTokens( + deserializeValue, props.fileName, startLine !== undefined ? startLine : undefined ); - const prefixCorrectedTokenStream = tokenStream; - if (tokenStream && tokenStream.length >= 5) { - prefixCorrectedTokenStream[TOKEN_START_CHAR_OFFSET_INDEX] -= configuration.getSerializationPrefix().length; - } + let prefixCorrectedTokenStream = tokenStream; + if (Array.isArray(tokenStream) && tokenStream.length >= 5) { + prefixCorrectedTokenStream = [...tokenStream]; + prefixCorrectedTokenStream[TOKEN_START_CHAR_OFFSET_INDEX] -= + configuration.getSerializationPrefix().length; + }This addresses the earlier concern about mutating the original token stream while keeping the prefix offset correction.
- Serialized vs. deserialized value alignment
Tokens are fetched using
deserializeValue(the fully wrapped/raw expression), while the editor document usessanitizedValuederived fromserializedValue. As long as the only structural difference is the configurable prefix/suffix (which you correct viagetSerializationPrefix().length), this is fine. IfsanitizedExpressionperforms additional transformations (beyond stripping prefixes/suffixes), token offsets may drift. In that case, consider usingmapSanitizedToRawfrom../utilsto keep token positions aligned with the sanitized document.
🧹 Nitpick comments (4)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx (1)
89-104: Token parameter ingetHelperValueis currently unused from ChipExpressionEditor
PrimaryModeChipExpressionEditorConfig.getHelperValuebranches ontoken?.type === TokenType.FUNCTION, but inChipExpressionEditor.tsxthe call site currently invokesconfiguration.getHelperValue(value)without passing theParsedTokenfromisSelectionOnToken. That makes the token‑aware logic effectively unreachable from the chip editor path.To fully benefit from this API, consider threading the token through from the editor, e.g.:
- const selectionIsOnToken = isSelectionOnToken(currentSelection.from, currentSelection.to, view); - const newValue = selectionIsOnToken ? value : configuration.getHelperValue(value); + const tokenOnSelection = isSelectionOnToken(currentSelection.from, currentSelection.to, view); + const newValue = tokenOnSelection + ? value + : configuration.getHelperValue(value, tokenOnSelection);This keeps the “replace existing token verbatim” behavior while allowing configs to specialize on token type when inserting new content.
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (3)
96-101: Configuration prop and default instance wiring are fine (minor optimization possible)Allowing a
configuration?: ChipExpressionEditorDefaultConfigurationprop and defaulting tonew ChipExpressionEditorConfig()gives good flexibility. Since the default is instantiated on every render whenconfigurationis undefined, if these configs ever gain internal state or heavier construction, you might wrap the default inuseMemo:- const { configuration = new ChipExpressionEditorConfig() } = props; + const configuration = useMemo( + () => props.configuration ?? new ChipExpressionEditorConfig(), + [props.configuration] + );Not required with the current stateless configs, but worth considering if they evolve.
119-133: onChange now emits deserialized (wrapped) values – verify external callers expect thisUsing
configuration.deserializeValue(newValue)before callingprops.onChangemeans parents always receive the fully wrapped expression (e.g.,string \...`` or backticked templates) rather than the “editor‑internal” representation.This is consistent with the new config abstraction but does subtly change the contract for existing consumers of
ChipExpressionEditorComponent. Please double‑check all call sites to ensure they now expect serialized/typed string templates instead of the raw inner content.
170-180: Leverage the returned token when delegating togetHelperValue
isSelectionOnTokenreturns aParsedToken, but here it’s only used as a truthy flag to decide betweenvalueandconfiguration.getHelperValue(value). As a result, configuration implementations that branch on the optional token argument (e.g., to treat FUNCTION tokens specially) can’t take advantage of that context.A minimal improvement that preserves current behavior while making configs more expressive:
- const selectionIsOnToken = isSelectionOnToken(currentSelection.from, currentSelection.to, view); - const newValue = selectionIsOnToken ? value : configuration.getHelperValue(value); + const tokenOnSelection = isSelectionOnToken(currentSelection.from, currentSelection.to, view); + const newValue = tokenOnSelection + ? value + : configuration.getHelperValue(value, tokenOnSelection);You may also want to confirm the interaction with
options.replaceFullText: when replacing the full expression, using the originalcurrentSelectionto detect “on token” might be surprising if the saved selection happened to be over a token.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx(5 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx(8 hunks)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- workspaces/ballerina/ballerina-side-panel/src/components/editors/ExpressionField.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-24T22:16:28.380Z
Learnt from: dan-niles
Repo: wso2/vscode-extensions PR: 980
File: workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RichTextTemplateEditor/RichTextTemplateEditor.tsx:384-412
Timestamp: 2025-11-24T22:16:28.380Z
Learning: In the RichTextTemplateEditor component (workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/RichTextTemplateEditor/RichTextTemplateEditor.tsx), token fetching on external `value` prop changes is intentionally disabled. Users cannot edit both the minimized and expanded editors simultaneously, so tokens only need to be generated based on user typing in the expanded editor view, not on external prop updates.
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsxworkspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/**/*.tsx : Use React 18.2.0 features including concurrent rendering and automatic batching; avoid class components in favor of functional components with hooks
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/**/*.tsx : Use Emotion for CSS-in-JS styling instead of inline styles or external CSS files
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.{ts,tsx} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values
Applied to files:
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx
🧬 Code graph analysis (2)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (4)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx (3)
ChipExpressionEditorConfig(82-87)deserializeValue(43-50)deserializeValue(72-79)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/CodeUtils.ts (1)
isSelectionOnToken(609-622)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/ChipExpressionDefaultConfig.ts (1)
deserializeValue(34-36)workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts (1)
TOKEN_START_CHAR_OFFSET_INDEX(23-23)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx (2)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/utils.ts (1)
ParsedToken(104-109)workspaces/ballerina/ballerina-side-panel/src/components/editors/index.ts (1)
InputMode(31-31)
🔇 Additional comments (4)
workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/Configurations.tsx (3)
24-50: StringTemplateEditorConfig serialization/deserialization and helper wrapping look consistentThe helper wrapping (
${value}) for non-function tokens and raw passthrough for function tokens, plus the `string ``/\prefix–suffix aware serialize/deserialize, are coherent and match the intended Ballerina string template shape. The logic also safely normalizes legacy unwrapped values on first load. No correctness issues from this file’s side.
53-80: RawTemplateEditorConfig behavior is symmetric and soundUsing bare backticks as serialization prefix/suffix with the same
${value}wrapping rule gives a clean “raw template” variant.serializeValueanddeserializeValuemirror the StringTemplate behavior and are side‑effect free apart from the intentional normalization of unwrapped values. This looks correct.
82-87: ChipExpressionEditorConfig default helper behavior is reasonableProviding a default implementation that only wraps non‑FUNCTION tokens in
${…}and passes through function tokens is a sensible baseline for non‑string‑template modes. Given the simplicity and absence of serialization overrides here, there’s no obvious risk.workspaces/ballerina/ballerina-side-panel/src/components/editors/MultiModeExpressionEditor/ChipExpressionEditor/components/ChipExpressionEditor.tsx (1)
455-459: Layout wrapper for editor height handling looks goodThe additional wrapper
divwith conditional{ height: '100%' }when expanded or when an explicitsx.heightis provided should resolve previous height inconsistencies without regressing the default max‑height behavior on the inner editor container. This change is straightforward and low‑risk.
1 participant
Purpose
This PR introduces string template support for string fields, solving the long-standing issue where users had to manually manage backticks, quotes, and string escaping when inserting variables. This often resulted in syntax errors and a poor editing experience.
The PR also addresses extensibility limitations in the
ChipExpressionEditorcomponent. Previously, editor-specific behavior required direct modifications inside the component. This made it hard to reuse the component across different editors with varying requirements.Resolves: wso2/product-ballerina-integrator#1988
Goals
ChipExpressionEditorby enabling consumers to provide a configuration object to customize behavior.Approach
ChipExpressionEditorto:Summary by CodeRabbit
New Features
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.