Show the collection name/type in the Foreach visualization#1558
Show the collection name/type in the Foreach visualization#1558dan-niles wants to merge 1 commit intowso2:release/bi-1.8.xfrom
Conversation
📝 WalkthroughWalkthroughThis change restructures the WhileNode widget component to extract and display collection information from FOREACH statements. It adds description rendering logic that formats collection type and variable names, implements hover-based menu button visibility, and reorganizes the header layout to accommodate the new description display alongside the title. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
dan-niles
force-pushed
the
add-description-to-foreach-node
branch
from
6aefe52 to
4b64015
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx (1)
45-46:⚠️ Potential issue | 🟡 MinorDuplicate CSS property:
coloris declared twice.Line 45 and 46 both set
color: ${ThemeColors.ON_SURFACE}. The second declaration shadows the first. Remove the duplicate.Proposed fix
color: ${ThemeColors.ON_SURFACE}; - color: ${ThemeColors.ON_SURFACE};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx` around lines 45 - 46, In WhileNodeWidget (component WhileNodeWidget), remove the duplicate CSS declaration by deleting the redundant "color: ${ThemeColors.ON_SURFACE};" so the style block only declares color once; locate the duplicate inside the styled block where ThemeColors.ON_SURFACE is applied and remove the extra line to avoid shadowing.
🧹 Nitpick comments (3)
.vscode/settings.json (1)
2-2: Unrelated change: enablingformatOnSavecan produce noisy diffs.This is orthogonal to the PR objective (Foreach description). Enabling it repo-wide may cause large formatting-only diffs from contributors whose local formatter config differs. Consider splitting this into a separate PR or ensuring all contributors share identical Prettier/formatter settings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vscode/settings.json at line 2, Remove the unrelated change enabling "editor.formatOnSave" from this PR (it causes noisy formatting diffs); either revert that line from the current diff or move the change into a separate, focused PR that also adds/updates a shared formatter configuration (e.g., .prettierrc or .editorconfig) and contributor guidance so everyone shares identical formatter settings before enabling format-on-save.workspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx (2)
284-292: Description extraction logic looks correct; minor note on the regex.The regex
/^foreach\s+(.+?)\s*\{/won't match ifsourceCodehas a newline between the iterable expression and the opening brace. This is likely fine for typical Ballerina source representations, but if multiline source is possible, adding thesflag (dotAll) would make it resilient.Suggested tweak
- const match = model.node.codedata.sourceCode.match(/^foreach\s+(.+?)\s*\{/); + const match = model.node.codedata.sourceCode.match(/^foreach\s+(.+?)\s*\{/s);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx` around lines 284 - 292, The description extraction in WhileNodeWidget (const description in WhileNodeWidget.tsx) uses the regex /^foreach\s+(.+?)\s*\{/ which fails if model.node.codedata.sourceCode contains a newline before the opening brace; update the pattern to allow newlines (either add the dotAll flag, e.g. use /pattern/s, or replace .+? with [\s\S]+? ) when matching the iterable expression so match?.[1] correctly captures multiline expressions from model.node.codedata.sourceCode.
354-357: Emptystyle={{}}wrapper and missing optional chaining onmetadata.Two small items:
Line 355:
<div style={{}}>— the empty style object is a no-op. If this div is just a grouping wrapper, a<>fragment (or removing it) would be cleaner. If styling is planned, a TODO would help.Line 357:
model.node.metadata.label— considermodel.node.metadata?.labelfor defensive access in casemetadatais everundefined.Proposed fix
- <div style={{ display: "flex", flexDirection: "row", alignItems: "center", justifyContent: "space-between", width: "100%" }}> - <div style={{}}> - <div style={{ display: "flex", flexDirection: "row", alignItems: "center", gap: "4px" }}> - <NodeStyles.Title>{model.node.metadata.label || model.node.codedata.node}</NodeStyles.Title> + <div style={{ display: "flex", flexDirection: "row", alignItems: "center", justifyContent: "space-between", width: "100%" }}> + <div> + <div style={{ display: "flex", flexDirection: "row", alignItems: "center", gap: "4px" }}> + <NodeStyles.Title>{model.node.metadata?.label || model.node.codedata.node}</NodeStyles.Title>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx` around lines 354 - 357, Remove the no-op wrapper div (the <div style={{}}> around the title) — replace it with a React fragment or remove it entirely so you don’t keep an empty style prop, and if you intend to keep a wrapper add a TODO comment for future styles; also make the metadata access defensive by using optional chaining when rendering the title (change references like model.node.metadata.label to model.node.metadata?.label || model.node.codedata.node) so NodeStyles.Title (and any other uses of model.node.metadata) won’t throw if metadata is undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@workspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx`:
- Around line 45-46: In WhileNodeWidget (component WhileNodeWidget), remove the
duplicate CSS declaration by deleting the redundant "color:
${ThemeColors.ON_SURFACE};" so the style block only declares color once; locate
the duplicate inside the styled block where ThemeColors.ON_SURFACE is applied
and remove the extra line to avoid shadowing.
---
Nitpick comments:
In @.vscode/settings.json:
- Line 2: Remove the unrelated change enabling "editor.formatOnSave" from this
PR (it causes noisy formatting diffs); either revert that line from the current
diff or move the change into a separate, focused PR that also adds/updates a
shared formatter configuration (e.g., .prettierrc or .editorconfig) and
contributor guidance so everyone shares identical formatter settings before
enabling format-on-save.
In
`@workspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx`:
- Around line 284-292: The description extraction in WhileNodeWidget (const
description in WhileNodeWidget.tsx) uses the regex /^foreach\s+(.+?)\s*\{/ which
fails if model.node.codedata.sourceCode contains a newline before the opening
brace; update the pattern to allow newlines (either add the dotAll flag, e.g.
use /pattern/s, or replace .+? with [\s\S]+? ) when matching the iterable
expression so match?.[1] correctly captures multiline expressions from
model.node.codedata.sourceCode.
- Around line 354-357: Remove the no-op wrapper div (the <div style={{}}> around
the title) — replace it with a React fragment or remove it entirely so you don’t
keep an empty style prop, and if you intend to keep a wrapper add a TODO comment
for future styles; also make the metadata access defensive by using optional
chaining when rendering the title (change references like
model.node.metadata.label to model.node.metadata?.label ||
model.node.codedata.node) so NodeStyles.Title (and any other uses of
model.node.metadata) won’t throw if metadata is undefined.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.vscode/settings.jsonworkspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
workspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx (1)
284-315: Extract FOREACH description parsing to a utility function for maintainability and testability.The regex parsing at lines 288–299 is coupled to the widget render path, making it difficult to test and maintain. While the line 288 regex is robust enough, line 299's pattern
/^(\S+)\s+(\S+)\s+(in)\s+(.+)$/cannot handle Ballerina's destructuring binding patterns (e.g.,foreach [string, int] [x, y] in pairs), which limits the widget's ability to display complex foreach statements correctly. Moving this extraction logic to a dedicated utility function insrc/utils/with comprehensive test coverage would improve both maintainability and handle edge cases more gracefully.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx` around lines 284 - 315, Extract the FOREACH parsing logic from WhileNodeWidget: move the regex/parse block currently inside the description IIFE and renderDescription() into a new utility function (e.g., parseForeachDescription(sourceOrCondition: string): { type?: string; varName?: string; iterable?: string; full?: string } | null) placed in src/utils/, then call that function from WhileNodeWidget's description computation and renderDescription; update renderDescription to use the returned structured object (falling back to raw description) rather than the inline regex, and add unit tests for parseForeachDescription to cover simple and destructuring patterns like "foreach [string,int] [x,y] in pairs" so the component can render complex foreach statements correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@workspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx`:
- Around line 284-315: Extract the FOREACH parsing logic from WhileNodeWidget:
move the regex/parse block currently inside the description IIFE and
renderDescription() into a new utility function (e.g.,
parseForeachDescription(sourceOrCondition: string): { type?: string; varName?:
string; iterable?: string; full?: string } | null) placed in src/utils/, then
call that function from WhileNodeWidget's description computation and
renderDescription; update renderDescription to use the returned structured
object (falling back to raw description) rather than the inline regex, and add
unit tests for parseForeachDescription to cover simple and destructuring
patterns like "foreach [string,int] [x,y] in pairs" so the component can render
complex foreach statements correctly.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/ballerina/bi-diagram/src/components/nodes/WhileNode/WhileNodeWidget.tsx
2 participants
Purpose
Fixes wso2/product-ballerina-integrator#902
This PR updates the
FOREACHnode widget to show the collection name and type in the flow diagram.Summary by CodeRabbit