codeant-ai-for-open-source[bot] commented on code in PR #37262:
URL: https://github.com/apache/superset/pull/37262#discussion_r2723940725
##########
superset-frontend/src/dashboard/util/getDropPosition.ts:
##########
@@ -132,13 +157,12 @@ export default function getDropPosition(monitor,
Component) {
return clientOffset.x < refMiddleX ? DROP_LEFT : DROP_RIGHT;
}
const refMiddleY =
- refBoundingRect.top + (refBoundingRect.bottom - refBoundingRect.top) / 2;
+ refBoundingRect.top +
+ (refBoundingRect.bottom - refBoundingRect.top) / 2;
Review Comment:
**Suggestion:** Axis mix-up when deciding drop side for sibling-only drops:
the code treats a "vertical" sibling orientation as an X-axis (left/right)
decision, but it should use the Y-axis (top/bottom) for vertical sibling
layouts; this will return the wrong drop side for vertically stacked siblings.
[logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Dashboard drag-and-drop places items on wrong axis.
- ⚠️ Visual rearrangement UX becomes confusing.
- ⚠️ Affects editor reordering for vertically stacked siblings.
```
</details>
```suggestion
const refMiddleY =
refBoundingRect.top +
(refBoundingRect.bottom - refBoundingRect.top) / 2;
return clientOffset.y < refMiddleY ? DROP_TOP : DROP_BOTTOM;
}
const refMiddleX =
refBoundingRect.left +
(refBoundingRect.right - refBoundingRect.left) / 2;
return clientOffset.x < refMiddleX ? DROP_LEFT : DROP_RIGHT;
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the file superset-frontend/src/dashboard/util/getDropPosition.ts and
locate
function getDropPosition at line 72.
2. Trigger the branch where validSibling is true and validChild is false
(the condition
evaluated at lines 111-113 and the branch starts at line 152). This happens
when
isValidChild returns true for the container's parent context but false for
the container
itself.
3. Ensure the component props produce siblingDropOrientation === 'vertical'
(siblingDropOrientation is computed at lines 118-119; set
Component.props.orientation so
that siblingDropOrientation resolves to 'vertical').
4. With siblingDropOrientation === 'vertical', execution enters the block
starting at line
152 and uses clientOffset.x compared to refMiddleX (lines 154-157) — this
compares X for a
vertically stacked sibling layout and therefore returns LEFT/RIGHT
incorrectly instead of
TOP/BOTTOM.
```
</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/getDropPosition.ts
**Line:** 154:161
**Comment:**
*Logic Error: Axis mix-up when deciding drop side for sibling-only
drops: the code treats a "vertical" sibling orientation as an X-axis
(left/right) decision, but it should use the Y-axis (top/bottom) for vertical
sibling layouts; this will return the wrong drop side for vertically stacked
siblings.
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]
