codeant-ai-for-open-source[bot] commented on code in PR #37262:
URL: https://github.com/apache/superset/pull/37262#discussion_r2706883231
##########
superset-frontend/src/dashboard/util/getDetailedComponentWidth.ts:
##########
@@ -77,9 +95,9 @@ export default function getDetailedComponentWidth({
result.occupiedWidth = 0;
(component.children || []).forEach(childId => {
// rows don't have widths, so find the width of its children
- if (components[childId].type === ROW_TYPE) {
+ if (components[childId]?.type === ROW_TYPE) {
result.minimumWidth = Math.max(
- result.minimumWidth,
+ result.minimumWidth ?? GRID_MIN_COLUMN_COUNT,
getTotalChildWidth({ id: childId, components }),
Review Comment:
**Suggestion:** In the `COLUMN_TYPE` branch `result.occupiedWidth` is
initialized to 0 but never updated inside the loop; the comment says "find the
width of the largest child" so `occupiedWidth` should reflect the largest child
width (or other intended aggregate). Update the loop to compute and store the
maximum child width into `result.occupiedWidth`. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Column occupiedWidth stays zero causing layout miscalculations.
- ❌ Affects column sizing in dashboard render and DnD calculations.
- ⚠️ Impacts drag-and-drop resize logic relying on occupiedWidth.
```
</details>
```suggestion
const childWidth = getTotalChildWidth({ id: childId, components });
result.minimumWidth = Math.max(
result.minimumWidth ?? GRID_MIN_COLUMN_COUNT,
childWidth,
);
result.occupiedWidth = Math.max(result.occupiedWidth ?? 0,
childWidth);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Prepare a layout in a unit test that includes:
- a COLUMN_TYPE component 'col-1' with children ['row-1'],
- a ROW_TYPE component 'row-1' with children ['chart-1'],
- a leaf component 'chart-1' with meta.width = 4.
2. Call getDetailedComponentWidth({ id: 'col-1', components }) from
superset-frontend/src/dashboard/util/getDetailedComponentWidth.ts (function
lines 56-114).
3. Execution enters the COLUMN_TYPE branch and runs the loop at lines
~95-103. The current
code updates result.minimumWidth but never updates result.occupiedWidth for
child widths.
4. Observe output: result.occupiedWidth === 0 while result.minimumWidth
reflects child
widths (e.g., 4). This mismatch demonstrates occupiedWidth is not computed
as intended.
Explanation: The comment says "find the width of the largest child" but
occupiedWidth is
left at 0; the improved code computes the maximum child width into
occupiedWidth.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/util/getDetailedComponentWidth.ts
**Line:** 100:101
**Comment:**
*Logic Error: In the `COLUMN_TYPE` branch `result.occupiedWidth` is
initialized to 0 but never updated inside the loop; the comment says "find the
width of the largest child" so `occupiedWidth` should reflect the largest child
width (or other intended aggregate). Update the loop to compute and store the
maximum child width into `result.occupiedWidth`.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
