Improvements in Record Constrcut View #1002
Conversation
WalkthroughAdds a recursive Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Modal as RecordConfigModal
participant Types as TypeComponents
participant Utils as resetFieldValues
participant UI as ParameterBranch
User->>UI: Click optional header / toggle
UI->>UI: Toggle open/closed (Codicon changes)
User->>Types: Uncheck a field
Types->>Utils: Call resetFieldValues(param) when unchecked
Utils->>Utils: Recursively clear values (nested, unions, inclusions)
User->>Modal: Select a model / change model
Modal->>Modal: set state & recordModelRef
Modal->>Modal: autoSelectFirstRecord(model) -> select required fields
Modal->>Modal: generate source from auto-selected state
User->>Modal: Clear expression input
Modal->>Modal: updateFieldsSelection(deselect all)
Modal->>Modal: clear sourceCode, diagnostics, and force re-render
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/RecordConfigModal.tsx (1)
548-556: ---Cancel pending diagnostics when expression is cleared to prevent stale diagnostics from resurfacing
The empty-expression handling correctly clears diagnostics and prevents a new
fetchDiagnosticscall via early return. However, the issue remains real: any pending debouncedfetchDiagnosticscall scheduled from the previous non-empty value will still execute after the clear and overwrite the cleared state with stale diagnostics.Recommended fix: Add
(fetchDiagnostics as any).cancel?.();before the early return in the empty-expression block (line 585) to cancel the pending debounced invocation. This prevents the race condition where old diagnostics would resurface after clearing the expression.// Inside the `if (updatedValue.trim() === '') { ... }` block (before line 586 return): (fetchDiagnostics as any).cancel?.();
🧹 Nitpick comments (3)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/styles.ts (1)
297-332: Optional polish for hover selector and transitionsThe new
listOptionalWrapper/listOptionalHeader/listOptionalTitlestyles read well and should give a clear interactive affordance.Two small, non-blocking suggestions:
- The
&:hover > div:last-of-typeselector for the title is a bit layout‑dependent; if the header’s children order changes later, the hover styling could accidentally move to a different element. Consider a more explicit selector (e.g., always applying the title class to the hovered element) if the structure evolves.transition: 'all 0.2s ease'on both header and title works but is broader than necessary; constraining it to the specific properties that change (e.g.,color,opacity,background-color) can be a minor perf/readability improvement.These are nice‑to‑have only; the current implementation is fine to ship.
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsx (2)
74-76: Consider adding a comment to document the depth-based logic.The
shouldShowOptionalParamsDirectlycalculation uses specific depth thresholds (depth === 1 and depth < 3) without explanation. Adding a brief comment would improve maintainability by clarifying the UI/UX rationale behind these values.Example:
+ // Show optional params directly at shallow depths to reduce UI clutter: + // - Always show at depth 1 if optional params exist + // - Show at depth < 3 when there are no required params const shouldShowOptionalParamsDirectly = (optionalParams.length > 0 && depth === 1) || (requiredParams.length === 0 && optionalParams.length > 0 && depth < 3);
106-108: Use CSS class instead of inline style for consistency.The margin-left styling uses an inline style, which is inconsistent with the rest of the codebase that uses CSS classes from
useHelperPaneStyles().Consider adding a CSS class to the styles hook:
// In styles.ts listOptionalContent: { marginLeft: '15px', }Then use it here:
- <div style={{ marginLeft: '15px' }}> + <div className={helperStyleClass.listOptionalContent}> {showOptionalParams && optionalParams.length > 0 && optionalParams} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsx(2 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/CustomType/index.tsx(2 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/InclusionType/index.tsx(2 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/RecordType/index.tsx(2 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsx(2 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/styles.ts(2 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx(1 hunks)workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/RecordConfigModal.tsx(8 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 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-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/InclusionType/index.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/RecordType/index.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/CustomType/index.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/styles.ts
📚 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} : Use TypeScript 5.8.3 with strict type checking enabled for all source files
Applied to files:
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/CustomType/index.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-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/CustomType/index.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/RecordConfigModal.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/styles.ts
📚 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-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.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/**/*.test.{ts,tsx} : Create snapshot tests for all node widgets and verify visual consistency across renders
Applied to files:
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/CustomType/index.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-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/styles.ts
🧬 Code graph analysis (6)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/InclusionType/index.tsx (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx (1)
resetFieldValues(67-95)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsx (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx (1)
resetFieldValues(67-95)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/RecordType/index.tsx (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx (1)
resetFieldValues(67-95)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/CustomType/index.tsx (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx (1)
resetFieldValues(67-95)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/RecordConfigModal.tsx (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx (1)
updateFieldsSelection(97-141)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/styles.ts (1)
workspaces/common-libs/ui-toolkit/src/styles/Theme.ts (1)
ThemeColors(79-104)
🔇 Additional comments (9)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/styles.ts (1)
19-19: ThemeColors import usage looks goodImporting
ThemeColorsfrom@wso2/ui-toolkitis consistent with the shared theme utilities and keeps colors aligned with the design system. No issues here.workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsx (2)
21-21: LGTM!The Codicon import is correctly added to support the new icon-based toggle UI.
115-115: Verify that React.memo's default comparison works correctly for the parameters prop.
React.memouses shallow comparison by default, which compares prop references. If theparametersarray is recreated on each parent render (new reference), the memoization won't prevent re-renders. Conversely, if the array reference stays the same while its contents change, necessary re-renders might be skipped.Verify that the parent component manages the
parametersarray in a way compatible with shallow comparison. If the array is frequently recreated, consider providing a custom comparison function:export const MemoizedParameterBranch = React.memo( ParameterBranch, (prevProps, nextProps) => { // Custom comparison logic return prevProps.depth === nextProps.depth && prevProps.parameters.length === nextProps.parameters.length && prevProps.parameters.every((p, i) => p === nextProps.parameters[i]); } );Or ensure the parent component uses
useMemoto stabilize theparametersarray reference.workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/CustomType/index.tsx (1)
25-48: Value reset on uncheck is consistent and correctWiring
resetFieldValues(param)into the uncheck path keeps this type in line with the others and correctly clears values when the checkbox is turned off, without touching required params.workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/utils/index.tsx (1)
67-95: Recursive reset implementation looks sound
resetFieldValuescorrectly walks all relevant structures (nested fields, union members, inclusion-type fields) and only clearsvalue, leaving selection flags to higher-level logic. This matches the PR’s intent and keeps responsibilities nicely separated.workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/RecordType/index.tsx (1)
26-51: Record deselection now fully clears nested stateCalling
resetFieldValues(param)beforeupdateFieldsSelection(param.fields, newSelectedState)ensures both values and selection flags are cleared when the record is unchecked, which aligns with the desired UX for “hide/reset” behavior.workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/UnionType/index.tsx (1)
27-197: Union uncheck path correctly clears values and selectionsUsing
resetFieldValues(param)ahead of the existing loop that clearsfield.selectedand callsupdateFieldsSelection(..., false)ensures both the union node and all member values are reset when the checkbox is turned off, with no change to the existing selection logic.workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/Types/InclusionType/index.tsx (1)
26-54: Inclusion-type deselection reset is consistent with other typesOn uncheck,
resetFieldValues(param)followed byupdateFieldsSelection(param.inclusionType.fields, newSelectedState)gives the expected “clear everything under this inclusion” behavior and matches the pattern used for record and union types.workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Views/RecordConfigModal.tsx (1)
29-30: Auto-selection and record model ref handling are coherentThe introduction of
recordModelRefplusautoSelectFirstRecordand the updatedgetNewRecordModel/handleMemberChangeflows looks solid:
- New models are funneled through a single
newModelarray, which is written to both React state and the ref.autoSelectFirstRecordmarks the first record selected and leveragesupdateFieldsSelectionto recursively select required fields (including union members), aligning with the helper-pane behavior in the type components.- Immediately calling
handleModelChange(newModel)after auto-selection ensures the right-hand expression and diagnostics are in sync with what’s selected in the tree.This gives the “auto-select first record + required fields” behavior described in the PR without changing existing APIs.
Also applies to: 145-147, 253-267, 277-291, 335-344, 385-393
...ina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsx
Outdated
Show resolved
Hide resolved
...ina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsx (1)
86-103: Keyboard accessibility is still missing on the clickable header.The clickable header div lacks keyboard navigation support, preventing keyboard users from toggling optional fields. This was flagged in the previous review but remains unaddressed. Users relying on keyboard navigation cannot interact with this control, violating WCAG accessibility guidelines.
Apply this diff to add keyboard accessibility:
- <div className={helperStyleClass.listOptionalHeader} onClick={toggleOptionalParams}> + <div + className={helperStyleClass.listOptionalHeader} + role="button" + tabIndex={0} + onClick={toggleOptionalParams} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + toggleOptionalParams(); + } + }} + aria-expanded={showOptionalParams} + aria-label="Toggle optional fields" + > <Button data-testid="optional-toggle-button" appearance="icon" sx={{ transition: "all 0.2s ease", "&:hover": { backgroundColor: "transparent !important", }, }} > <Codicon name={showOptionalParams ? "chevron-down" : "chevron-right"} iconSx={{ fontSize: '13px' }} /> </Button> <div className={helperStyleClass.listOptionalTitle}>Optional fields</div> </div>
🧹 Nitpick comments (2)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsx (2)
74-76: Consider adding a comment to explain the visibility logic.The
shouldShowOptionalParamsDirectlycomputation uses depth-based conditions to determine when optional fields should be directly visible vs. collapsible. A brief comment explaining why depth 1 or depth < 3 (with no required params) trigger direct display would improve maintainability.
106-108: Consider moving inline style to the styles file.The
marginLeft: '15px'inline style could be moved to thestyles.tsfile (e.g., aslistOptionalContentclass) for better maintainability and consistency with the rest of the styling approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.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-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.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/resources/icons/**/*.tsx : Create separate SVG icon components in src/resources/icons/ for all diagram icons and import them as React components
Applied to files:
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.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-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsx
🧬 Code graph analysis (1)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsx (2)
workspaces/common-libs/ui-toolkit/src/components/Button/Button.tsx (1)
Button(54-65)workspaces/common-libs/ui-toolkit/src/components/Codicon/Codicon.tsx (1)
Codicon(44-56)
🔇 Additional comments (3)
workspaces/ballerina/ballerina-visualizer/src/views/BI/HelperPaneNew/Components/RecordConstructView/ParameterBranch.tsx (3)
18-21: LGTM: Imports are appropriate for the new collapsible header.The useState hook and UI toolkit components (Button, Codicon) are correctly imported to support the enhanced optional fields toggle functionality.
67-72: Good fix: Event type is now properly typed.The event parameter is now correctly typed as
React.MouseEvent<HTMLDivElement>instead ofany, improving type safety as requested in the previous review.
115-115: Good addition: Component memoization improves performance.Exporting a memoized version of the component is a good practice that prevents unnecessary re-renders when props haven't changed.
sachiniSam
changed the title
2 participants
Purpose
This PR includes the following enhancements:
Screen.Recording.2025-11-25.at.13.54.17.mov
related issue wso2/product-ballerina-integrator#1952
Goals
Approach
UI Component Development
npm run storybookfrom the root directory to view current components.Manage Icons
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
New Features
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.