codeant-ai-for-open-source[bot] commented on PR #36778: URL: https://github.com/apache/superset/pull/36778#issuecomment-3678467850
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/36778/files#diff-c4f2a0f06dacc047cd5b660720a3f757f825bfcd3c86c59e53aa97d813f26161R67-R75'><strong>Drag listeners capture child clicks</strong></a><br>`listeners` and `attributes` from `useSortable` are spread onto the whole `Container`. This makes the entire container a drag target and can intercept clicks on interactive children (delete/undo buttons). Ensure drag activation is constrained or attach listeners only to a drag handle to avoid blocking child button events.<br> - [ ] <a href='https://github.com/apache/superset/pull/36778/files#diff-2b4e1e0468caf93c2e992bc521e86082bba0f33c8a31137f6d9dc80a70c19e92R167-R174'><strong>Drag index resolution</strong></a><br>handleDragEnd reads the start/end indices from active.data.current.sortable.index. That relies on draggable children populating that nested data shape. If the sortable data is missing (or the shape differs) the handler will no-op. A more robust approach is to derive indexes from active.id / over.id (which dnd-kit always sets) and compute positions from the `filters` array.<br> - [ ] <a href='https://github.com/apache/superset/pull/36778/files#diff-c4f2a0f06dacc047cd5b660720a3f757f825bfcd3c86c59e53aa97d813f26161R31-R38'><strong>Prop forwarding</strong></a><br>The `isDragging` prop is provided to the styled `Container` but there is no `shouldForwardProp` defined for `Container`. That can cause the boolean `isDragging` to be forwarded to the DOM as an invalid attribute and trigger React warnings / unexpected behavior. Prefer preventing custom props from reaching DOM elements.<br> - [ ] <a href='https://github.com/apache/superset/pull/36778/files#diff-2b4e1e0468caf93c2e992bc521e86082bba0f33c8a31137f6d9dc80a70c19e92R181-R187'><strong>Sensors recreation & render stability</strong></a><br>`sensors` (and `useSensor`) are created inline on every render. This may be harmless but can cause unnecessary re-subscriptions / re-renders of dnd-kit internals. Consider memoizing sensors (or the handler) so they are stable between renders.<br> </td></tr> </table> -- 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]
