codeant-ai-for-open-source[bot] commented on code in PR #38122:
URL: https://github.com/apache/superset/pull/38122#discussion_r2832945815
##########
superset-frontend/src/components/Datasource/FoldersEditor/sensors.ts:
##########
@@ -21,8 +21,69 @@ import {
PointerSensorOptions,
MeasuringConfiguration,
MeasuringStrategy,
+ rectIntersection,
+ pointerWithin,
+ closestCenter,
+ CollisionDetection,
+ UniqueIdentifier,
} from '@dnd-kit/core';
+/**
+ * Custom collision detection that deprioritizes the active (dragged) item.
+ *
+ * When dragging a folder block, the DragPlaceholder at the original position
+ * registers as a droppable. rectIntersection uses a translated RECT (not just
+ * the pointer) for collision checks, so its result can overlap with both the
+ * DragPlaceholder and a neighbouring item. When the DragPlaceholder wins by
+ * area, overId stays as the active item and the "same position" early-return
+ * in handleDragEnd prevents the reposition.
+ *
+ * This strategy falls back to pointerWithin when rectIntersection picks the
+ * active item, which uses the actual pointer position and reliably detects the
+ * item the user is hovering over. When the pointer lands in tiny gaps between
+ * droppable rects (inner element refs are slightly smaller than react-window
+ * slots), closestCenter is used as a final fallback to find the nearest item.
+ */
+export function createCollisionDetection(
+ activeId: UniqueIdentifier | null,
+): CollisionDetection {
+ if (!activeId) return rectIntersection;
Review Comment:
**Suggestion:** The check for the active item ID uses a generic falsy check,
so if a valid identifier like 0 or an empty string is ever used, the custom
collision detection logic will be skipped and the function will fall back to
the default rectIntersection behavior, reintroducing the very drag/drop issues
this helper is meant to solve for those IDs. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ FoldersEditor drag/drop misbehaves for falsy item identifiers.
- ⚠️ Custom rect/pointer/center collision fallback silently disabled.
```
</details>
```suggestion
if (activeId === null) return rectIntersection;
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In
`superset-frontend/src/components/Datasource/FoldersEditor/sensors.ts:47-50`,
the
`createCollisionDetection` function is used to build the collision strategy
for the
folders drag-and-drop behavior, taking `activeId: UniqueIdentifier | null`
from dnd-kit.
2. A caller (e.g., the FoldersEditor configuration for `@dnd-kit/core` in
this feature)
passes a valid `UniqueIdentifier` equal to `0` (number) or `''` (empty
string) as the
`activeId` when a drag starts.
3. When `createCollisionDetection(0)` (or `createCollisionDetection('')`)
executes, the
condition `if (!activeId) return rectIntersection;` at line 50 evaluates to
true, so the
function returns the bare `rectIntersection` strategy instead of the custom
closure that
combines `rectIntersection`, `pointerWithin`, and `closestCenter` (lines
52–83).
4. During a drag where the situation described in the file comment occurs
(rect-based
intersection favors the DragPlaceholder so `overId` stays on the active item
and prevents
reposition), the custom fallback logic is never run, so the "same position"
early-return
in `handleDragEnd` mentioned in the comment is hit and the folder cannot be
repositioned
for items whose ID is `0` or `''`.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/components/Datasource/FoldersEditor/sensors.ts
**Line:** 50:50
**Comment:**
*Logic Error: The check for the active item ID uses a generic falsy
check, so if a valid identifier like 0 or an empty string is ever used, the
custom collision detection logic will be skipped and the function will fall
back to the default rectIntersection behavior, reintroducing the very drag/drop
issues this helper is meant to solve for those IDs.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38122&comment_hash=1cc050b5fef54a94da0c62010b02449117390942a54e96f02a339e0cbd214165&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38122&comment_hash=1cc050b5fef54a94da0c62010b02449117390942a54e96f02a339e0cbd214165&reaction=dislike'>👎</a>
--
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]
