You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "yousoph (via GitHub)" <gi...@apache.org> on 2026/07/01 19:55:18 UTC
[PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
yousoph opened a new pull request, #41637:
URL: https://github.com/apache/superset/pull/41637
### SUMMARY
Reordering selected pills within an Explore control (metrics, columns, filters, etc.) by dragging is broken: the pill snaps back and the order is unchanged. Dragging a new column/metric *in* from the Datasource panel still works — only reordering is broken.
**Root cause:** the react-dnd → @dnd-kit migration ([#37880](https://github.com/apache/superset/pull/37880), SIP-184) left the Explore `<DndContext>` using @dnd-kit's default `rectIntersection` collision detection. Each control registers a large parent dropzone (`dropzone-<name>`) that geometrically **encloses every pill**, so during an intra-control reorder the parent dropzone always wins the largest-intersection tie-break. `resolveDragEnd` then sees an `over` target with no `dragIndex` and silently no-ops. External drops from the Datasource panel kept working because they legitimately target that parent dropzone (which exposes `onDrop`) — which is why only reordering was reported broken.
**Fix:** a scoped collision-detection strategy (`exploreCollisionDetection`) passed to `DndContext` that resolves collisions by the drag's intent:
- Reordering an existing pill (active data carries a `dragIndex`) → collide **only** with sibling sortables of the same `type`, resolved by `closestCenter`.
- Dragging a new item in from the panel → collide **only** with parent dropzones exposing an `onDrop` handler (`pointerWithin`, with a `rectIntersection` fallback).
This restores reordering and makes external drops onto already-populated controls more robust (pills can no longer intercept them).
### BEFORE/AFTER
Before: dragging a metric/column pill within a control to reorder it does nothing — the pill returns to its original position.
After: pills reorder as expected; dragging items in from the Datasource panel continues to work.
### TESTING INSTRUCTIONS
1. Open any chart in Explore with 2+ metrics or group-by columns (or add them).
2. Drag one pill above/below another within the same control.
3. The order should update and persist.
4. Confirm dragging a new column/metric in from the Datasource panel still works.
Automated: added 3 unit tests to `ExploreDndContext.test.tsx` asserting the collision scoping (which droppables each detector receives). jsdom cannot drive @dnd-kit pointer sensors, so the scoping logic is tested directly rather than via a simulated pointer drag.
### ADDITIONAL INFORMATION
- [ ] Has associated issue: relates to the @dnd-kit migration [#37880](https://github.com/apache/superset/pull/37880); also relevant to [#33951](https://github.com/apache/superset/issues/33951) (index-based sortable IDs remain a separate latent cosmetic issue, not addressed here to keep this fix targeted).
- [x] Required feature flags: n/a
- [ ] Changes UI
- [ ] Migration is atomic
- [ ] Introduces new feature or API
- [x] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4930363043
<!-- Bito Reply -->
The flagged issue is correct. The migration to `@dnd-kit` requires using `arrayMove(items, fromIndex, toIndex)` to handle reordering, as it performs a full array move rather than a simple swap. Using the parameters in the order `(dragIndex, hoverIndex)` is correct for `arrayMove`, but the callback parameters were named `(hoverIndex, dragIndex)` in some components, leading to inverted reordering.
To resolve this, ensure the callback parameters are consistently named `(dragIndex, hoverIndex)` and passed to `arrayMove` in that order. I have verified that this pattern is correctly applied in other components within this PR (e.g., `DndColumnMetricSelect.tsx`, `DndFilterSelect.tsx`, `DndMetricSelect.tsx`).
Would you like me to fetch all remaining review comments to validate and implement fixes for the rest of the PR?
**superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnMetricSelect.tsx**
```
const onShiftOptions = useCallback(
(dragIndex: number, hoverIndex: number) => {
// @dnd-kit fires the reorder once at drag-end with the final indices, so
// this must be a full arrayMove, not an adjacent swap.
const newValues = arrayMove(coercedValue, dragIndex, hoverIndex);
onChange(multi ? newValues : newValues[0]);
},
[onChange, coercedValue, multi],
);
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4962620036
🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/29284430770) for [12064b7](https://github.com/apache/superset/commit/12064b7)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41637:
URL: https://github.com/apache/superset/pull/41637#discussion_r3555370595
##########
superset-frontend/src/explore/components/controls/ContourControl/index.tsx:
##########
@@ -88,12 +89,9 @@ const ContourControl = ({ onChange, ...props }: ContourControlProps) => {
};
const onShiftContour = (hoverIndex: number, dragIndex: number) => {
- const newContours = [...contours];
- [newContours[hoverIndex], newContours[dragIndex]] = [
- newContours[dragIndex],
- newContours[hoverIndex],
- ];
- setContours(newContours);
+ // @dnd-kit reports the final indices at drag-end, so reorder with a full
+ // arrayMove rather than an adjacent swap to support non-adjacent drags.
+ setContours(arrayMove(contours, dragIndex, hoverIndex));
Review Comment:
**Suggestion:** The reorder callback still receives indices in the legacy `(hoverIndex, dragIndex)` semantic order, but `arrayMove` expects `(fromIndex, toIndex)`. Using `arrayMove(contours, dragIndex, hoverIndex)` with the current parameter naming reverses the move direction, so dragging item A onto B can move B to A instead. Swap the arguments passed to `arrayMove` (or rename/reorder the callback parameters) so the source index is first and destination index is second. [incorrect variable usage]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ ContourControl pill reordering moves wrong contour entry.
- ⚠️ DeckGL Contour chart shows unexpected contour order.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open an Explore view using the DeckGL Contour visualization; its control panel wires
the `contours` control to `ContourControl` via
`plugins/preset-chart-deckgl/src/layers/Contour/controlPanel.ts:12-23` and the control
registry in `superset-frontend/src/explore/components/controls/index.ts:29` (mapping
`ContourControl` into `controlMap`).
2. In `ContourControl`
(`superset-frontend/src/explore/components/controls/ContourControl/index.tsx`, new hunk
around lines 79-45), note that `valuesRenderer` maps `contours` to `ContourOption`
components and passes `onShift={onShiftContour}` (lines 34-45 in the read snippet), while
`onShiftContour` is declared as `(hoverIndex: number, dragIndex: number)` and implemented
with `setContours(arrayMove(contours, dragIndex, hoverIndex));` at PR diff line 94.
3. When the user drags an existing contour pill to reorder (e.g., dragging item at index 0
over the pill at index 2), `OptionWrapper`
(`superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx:65-85`)
registers each pill with `useSortable` data `{ type, dragIndex: index, onShiftOptions }`.
On drag end, `resolveDragEnd` in
`superset-frontend/src/explore/components/ExploreContainer/ExploreDndContext.tsx:153-177`
detects a same-list reorder and calls the active pill’s reorder callback as
`reorderCallback(activeData.dragIndex, overData.dragIndex)`, so `onShiftContour` receives
`(fromIndex, toIndex)` even though its parameters are named `(hoverIndex, dragIndex)`.
4. Inside `onShiftContour`, the call `setContours(arrayMove(contours, dragIndex,
hoverIndex));` at line 94 uses `dragIndex` (the destination index) as `arrayMove`’s
`fromIndex` and `hoverIndex` (the source index) as `toIndex`. This reverses the move
direction: dragging the pill at index 0 over the pill at index 2 moves the pill originally
at index 2 to index 0 instead of moving the dragged pill to index 2, producing inverted
contour pill reordering in the Explore ContourControl UI.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c34549fb155b488c84ddb6e4b1ceacac&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c34549fb155b488c84ddb6e4b1ceacac&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/explore/components/controls/ContourControl/index.tsx
**Line:** 94:94
**Comment:**
*Incorrect Variable Usage: The reorder callback still receives indices in the legacy `(hoverIndex, dragIndex)` semantic order, but `arrayMove` expects `(fromIndex, toIndex)`. Using `arrayMove(contours, dragIndex, hoverIndex)` with the current parameter naming reverses the move direction, so dragging item A onto B can move B to A instead. Swap the arguments passed to `arrayMove` (or rename/reorder the callback parameters) so the source index is first and destination index is second.
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.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41637&comment_hash=a38f3fdaf8640a6e26b2e1c271304ec68dda30cbd75138352d62735d818992f6&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41637&comment_hash=a38f3fdaf8640a6e26b2e1c271304ec68dda30cbd75138352d62735d818992f6&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4859625996
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41637?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 64.52%. Comparing base ([`6c57919`](https://app.codecov.io/gh/apache/superset/commit/6c5791964746976f776a20c818527ebe23f88715?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f1bf0b9`](https://app.codecov.io/gh/apache/superset/commit/f1bf0b91e27563173157409a2ac48d40715ba20b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #41637 +/- ##
=======================================
Coverage 64.51% 64.52%
=======================================
Files 2673 2673
Lines 147639 147654 +15
Branches 34093 34097 +4
=======================================
+ Hits 95252 95267 +15
Misses 50655 50655
Partials 1732 1732
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/41637/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/41637/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.28% <100.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41637?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4910489509
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #41637:
URL: https://github.com/apache/superset/pull/41637
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4861249915
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4859553649
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "yousoph (via GitHub)" <gi...@apache.org>.
yousoph commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4930324838
Follow-up commit `3335256c6e`: calm the pill **reorder** animation.
Reordering pills within a control (metrics / dimensions / adhoc filters) was playing @dnd-kit's default FLIP layout animation on drop, which reads as a springy "bounce" that makes it hard to see what actually moved. Two small changes on the pill sortables (`OptionControls` + `OptionWrapper`):
- `animateLayoutChanges: () => false` — drop the FLIP settle so pills settle without the bounce.
- `CSS.Transform.toString` → `CSS.Translate.toString` — translate only (no `scaleX/scaleY`), so variable-width pills don't morph into their neighbor's size mid-reorder.
Verified live in a browser (jsdom can't drive @dnd-kit pointer sensors): reorder still commits/persists, and dragging a new column/metric in from the Datasource panel is unaffected.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4962751322
🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/29284430770) for [12064b7](https://github.com/apache/superset/commit/12064b7)
• **Environment:** http://52.34.128.43:8080 (admin/admin)
• **Lifetime:** 48h auto-cleanup
• **Updates:** New commits create fresh environments automatically
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "yousoph (via GitHub)" <gi...@apache.org>.
yousoph commented on code in PR #41637:
URL: https://github.com/apache/superset/pull/41637#discussion_r3555489741
##########
superset-frontend/src/explore/components/controls/ContourControl/index.tsx:
##########
@@ -88,12 +89,9 @@ const ContourControl = ({ onChange, ...props }: ContourControlProps) => {
};
const onShiftContour = (hoverIndex: number, dragIndex: number) => {
- const newContours = [...contours];
- [newContours[hoverIndex], newContours[dragIndex]] = [
- newContours[dragIndex],
- newContours[hoverIndex],
- ];
- setContours(newContours);
+ // @dnd-kit reports the final indices at drag-end, so reorder with a full
+ // arrayMove rather than an adjacent swap to support non-adjacent drags.
+ setContours(arrayMove(contours, dragIndex, hoverIndex));
Review Comment:
Good catch — confirmed real. `arrayMove` isn't symmetric, so `arrayMove(contours, dragIndex, hoverIndex)` with the reversed param names moved the wrong contour on non-adjacent drags (adjacent ones happened to work). Fixed in `12064b7081` by renaming the params to `(dragIndex, hoverIndex)` so the call is `arrayMove(from, to)`, matching the other DnD controls.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "yousoph (via GitHub)" <gi...@apache.org>.
yousoph commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4909494437
Thanks for the thorough review @rusackas — both blockers were spot on and are fixed in `7491d3c`:
**1. Failing `DndColumnMetricSelect` test** — updated the expectation to the `arrayMove` result `['metric_a', 'column_b', 'column_a']` and rewrote the stale "swap" comment.
**2. `DndFilterSelect` (the standard `adhoc_filters` control)** — you're right, I'd only fixed the legacy `AdhocFilterControl`. Its `onShiftOptions` now does `arrayMove` + `setValues` + `onChange(newValues)` so the reorder actually persists. Verified live in a running Explore:
- Before: `[is_software_dev, school_degree, job_pref, time_start]`
- After dragging filter 0 → position 2: `[school_degree, job_pref, is_software_dev, time_start]`
- The `PUT /api/v1/explore/form_data` payload carried the new `adhoc_filters` order, and it persisted across a full page reload. (As you'd expect, `adhoc_filters` is a query control rather than a `renderTrigger`, so the PUT fires when the query runs — the bug was purely that `onChange` was never called, so the run used the stale order.)
**Non-blocking follow-up (included):** folded in the `arrayMove` fix for `DndColumnSelect` (via `OptionSelector.move` replacing the adjacent `swap`) and `ContourControl.onShiftContour` — they persisted already, but the swap was wrong for non-adjacent drags. All affected suites stay green (`DndColumnMetricSelect`, `DndFilterSelect`, `DndColumnSelect`, `ExploreDndContext` — 52 tests pass).
**Left for a separate issue:** legacy `MetricsControl` reorder (broken before and after this PR), per your note. Happy to file that follow-up.
Ready for re-review 🙏
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4909510303
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 7491d3ccc1612831818d298094299b3948b04e4c |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a4d7913ed6be900080bf91a |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-41637--superset-docs-preview.netlify.app](https://deploy-preview-41637--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpBCoAgEEDRu8w6YaSw7AYdQ5uRglIZWhjR3Z3d5_M-qFLu-sAKMECUkPdDO53NcKtXETaUyQgXIRYlxHrfjRS5MNHs7cjkIntEXDAmbwP8HQ) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-41637--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4931310426
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4883700396
@yousoph the collision-scoping approach is sound... I traced it against `resolveDragEnd` and scoping reorder collisions to same-context sortables is exactly what keeps the dropzone from swallowing them. The new scoping tests are solid too. Two things in the way though:
The CI failure is real and self-inflicted: `DndColumnMetricSelect.test.tsx` ("can drag and reorder items", ~line 228) still asserts the old adjacent-swap order, so it fails against the new `arrayMove` semantics. The expectation should become `['metric_a','column_b','column_a']`.
Bigger one: the standard `adhoc_filters` control is `DndFilterSelect` (via the shared dnd controls), and its `onShiftOptions` still does an adjacent swap with only `setValues`... no `onChange`, so filter reorder still won't persist to form_data on the path nearly every chart uses. The PR only fixes the legacy `AdhocFilterControl`. Mirroring the other handlers there (`arrayMove` + `setValues` + `onChange`) looks like ~5 lines and squarely in scope of the PR's claim.
Related-but-not-blocking: `DndColumnSelect` and `ContourControl` still use swap semantics for non-adjacent drags (they do persist, so fine as a follow-up), and legacy `MetricsControl` reorder was broken before and after... worth a follow-up issue.
Happy to re-review once those two are in. Thanks!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): restore drag-and-drop reordering broken by @dnd-kit migration [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4920299717
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org