-
Notifications
You must be signed in to change notification settings - Fork 85
fix(calm-widgets): render primitive arrays inline with comma separator list for for MDX compatibility (#2080) #2082
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…r for MDX compatibility (finos#2080)
|
|
||
| {{else}} | ||
| {{this}} | ||
| {{this}}{{#unless @last}},{{/unless}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to treat this a list separated by new line, I would recommend then we build an inner list and leverage the list widget then.
<br/> has no semantic reasoning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this renders comprehensibly with comma, I'd suggest we go with it and raise a feature request is we want to add a list widget.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an MDX compilation error that occurs when CALM architecture metadata contains arrays of primitive values. The fix ensures primitive arrays are rendered inline with comma separators instead of with newlines, preventing MDX from interpreting the content as paragraph breaks within <td> elements.
Changes:
- Modified
table-vertical.htmlto render primitive arrays inline with comma-space separators - Modified
row-template.htmlto add commas after primitive array elements - Added test fixture
metadata-primitive-arrayand E2E test for the new behavior - Updated expected outputs for existing tests to reflect comma separators
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| calm-widgets/src/widgets/table/table-vertical.html | Added inline rendering for primitive arrays with comma-space separators; contains a critical bug (using cellValue instead of this on line 20) |
| calm-widgets/src/widgets/table/row-template.html | Added comma separators for primitive arrays (preserves newlines) |
| calm-widgets/test-fixtures/table-widget/metadata-primitive-array/* | New test fixture demonstrating MDX-safe inline rendering |
| calm-widgets/src/widgets.e2e.spec.ts | New E2E test for primitive array rendering |
| cli/test_fixtures/template/expected-output/widget-tests/* | Updated expected outputs to include comma separators |
| calm-widgets/test-fixtures/combined-widgets/comprehensive-documentation/expected.md | Updated expected output to include comma separators |
| calm-plugins/vscode/package.json | Version bump (appears unrelated to this PR) |
| package-lock.json | Lock file update for VSCode plugin version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| {{else}} | ||
| {{this}} | ||
| {{this}}{{#unless @last}},{{/unless}} |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an inconsistency in how primitive arrays are rendered between table-vertical.html and row-template.html:
table-vertical.html(line 21): Renders arrays inline with commas and spaces:add, subtract, multiply, dividerow-template.html(line 21): Adds commas but preserves newlines between elements
While adding commas may be sufficient to prevent MDX from treating newlines as paragraph breaks, this inconsistency could lead to confusion. Consider whether both templates should render primitive arrays identically (fully inline), or if there's a deliberate reason for the different formatting (e.g., readability for deeply nested structures).
Additionally, note that row-template.html uses commas without spaces (,{{/unless}}), which is inconsistent with the space-after-comma style in table-vertical.html (, {{/unless}}).
| {{this}}{{#unless @last}},{{/unless}} | |
| {{this}}{{#unless @last}}, {{/unless}} |
| "displayName": "CALM", | ||
| "description": "Live-visualize CALM architecture models, validate, and generate docs.", | ||
| "version": "0.4.1", | ||
| "version": "0.4.2", |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This version bump from 0.4.1 to 0.4.2 for the VSCode plugin appears unrelated to the calm-widgets fix described in this PR. The PR title indicates this is a fix for calm-widgets, and the VSCode plugin doesn't appear to have a direct dependency on calm-widgets based on its package.json.
Consider whether this version bump should be in a separate PR, or if there's a connection that should be documented in the PR description.
| "version": "0.4.2", | |
| "version": "0.4.1", |
| {{> table-template.html rows=(objectEntries this) headers=false flatTable=false hasRows=true isNested=true}} | ||
| {{else}}{{this}} | ||
| {{/if}}{{/each}}{{else}}{{cellValue}}{{/if}}</td> | ||
| {{> table-template.html rows=(objectEntries cellValue) headers=false flatTable=false hasRows=true isNested=true}} |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line incorrectly uses cellValue instead of this when processing objects within an array. At this point in the code, we're inside a {{#each cellValue}} loop (line 19), where this refers to the current element being iterated, not cellValue which refers to the entire array.
The original code correctly used this:
{{> table-template.html rows=(objectEntries this) headers=false ...This should be reverted to use this instead of cellValue. The pattern in row-template.html (line 18) shows the correct implementation:
{{#each data}}
{{#if (isObject this)}}
{{> table-template.html rows=(objectEntries this) headers=false ...| {{> table-template.html rows=(objectEntries cellValue) headers=false flatTable=false hasRows=true isNested=true}} | |
| {{> table-template.html rows=(objectEntries this) headers=false flatTable=false hasRows=true isNested=true}} |

Description
Fix MDX compilation error when metadata contains arrays of primitive values.
When relationships have metadata with array values (e.g.,
"operations": ["add", "subtract", "multiply", "divide"]), the generated documentation site fails to compile with:This was caused by array values being rendered with newlines inside
<td>elements, which MDX interprets as paragraph breaks.The fix renders primitive arrays inline with comma separators:
add, subtract, multiply, divideExample in #2080 now renders appropriately
Type of Change
Affected Components
calm-widgets/)Testing
Added new test fixture
metadata-primitive-arrayto verify MDX-safe array rendering.Checklist