refactor(Filter components): migrate from react-dnd to dnd-kit#37445
refactor(Filter components): migrate from react-dnd to dnd-kit#37445rusackas merged 13 commits intoapache:masterfrom
Conversation
LevisNgigi
requested review from
betodealmeida,
eschutho,
geido,
mistercrunch,
rusackas and
sadpandajoe
as code owners
Code Review Agent Run #82a83fActionable Suggestions - 0Additional Suggestions - 8
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
dosubot
bot
added
change:frontend
Code Review Agent Run #799355Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
I tested manually (videos are not playing) and cross list divider d&d does not work. Should it? |
In the Filters and Config Modal? Below is the video. 2026-01-29.12-59-26.mp4 |
|
I think it might work but the dropping takes a while: 2026-01-30_14-40-43.mp4 |
LevisNgigi
requested review from
kgabryje,
michael-s-molina and
villebro
as code owners
github-actions
bot
added
i18n
LevisNgigi
force-pushed
the
use-dnd-kit
branch
from
3f92d30 to
aba104b
Compare
github-actions
bot
removed
i18n
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Code Review Agent Run #c5da1dActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Code Review Agent Run #261891Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
alexandrusoare
left a comment
alexandrusoare
left a comment
There was a problem hiding this comment.
LGTM - one curiosity though. Are we sure we covering all the cases where dnd-kit should be used? For example I know we also do drag and drop in SQL lab and on filters on explore.
This PR is just the first step of the migration to dnd-kit. For now I focused specifically on drag-and-drop inside the config modals, mainly filters and chart customizations, since they share similar behavior. Other areas will be handled in follow up PRs like this one: #37880 |
github-actions
bot
removed
🎪 be83d7b 🚦 running
LevisNgigi
force-pushed
the
use-dnd-kit
branch
from
b7509f8 to
bad1cf6
Compare
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #8113c0
Actionable Suggestions - 2
-
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ConfigModalSidebar/ConfigModalSidebar.tsx - 1
- Unsafe property access in drag handler · Line 158-158
-
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx - 1
- Debounced state update causes UI lag · Line 463-476
Additional Suggestions - 2
-
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx - 1
-
Test coverage reduced for drag behavior · Line 69-79The 'drag and drop' test now only checks for presence of UI elements but does not verify that drag and drop actually triggers the onRearrange callback. According to BITO.md rule [6262], tests should assert the behavior they claim to test, not just rendering. Since @dnd-kit drag interactions are difficult to test with React Testing Library (as confirmed by library docs), consider either properly testing the behavior by mocking events or renaming the test to reflect its actual scope.
-
-
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx - 1
-
Test name mismatch · Line 579-579The test name 'rearranges three filters and deletes one of them' does not match the implementation, which only rearranges filters using keyboard navigation and asserts deleted: []. Since deletion is tested separately in other tests, update the name for clarity.
Code suggestion
@@ -579,1 +579,1 @@ - test('rearranges three filters and deletes one of them', async () => { + test('rearranges three filters using keyboard navigation', async () => {
-
Review Details
-
Files reviewed - 9 · Commit Range:
0f6e867..c4d78d6- superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.nativeFilters.test.ts
- superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ConfigModalSidebar/ConfigModalSidebar.tsx
- superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.test.tsx
- superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx
- superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx
- superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx
- superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx
- superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
- superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ItemTitleContainer.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
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 documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| const overCustomizationIndex = customizationOrderedIds.findIndex( | ||
| id => id === over.id, | ||
| ); | ||
|
|
There was a problem hiding this comment.
Accessing active.data.current without checking if active.data exists could cause a runtime error if drag data is not set on the draggable item. Use optional chaining to safely access the nested property.
Code suggestion
Check the AI-generated fix before applying
- );
- const activeData = active.data.current;
- if (
- activeFilterIndex === -1 &&
- activeCustomizationIndex === -1 &&
+ );
+ const activeData = active.data?.current;
+ if (
+ activeFilterIndex === -1 &&
- activeCustomizationIndex === -1 &&
+ activeCustomizationIndex === -1 &&
Code Review Run #8113c0
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
| const debouncedHandleErroredItems = useMemo( | ||
| () => | ||
| debounce(() => { | ||
| setSaveAlertVisible(false); | ||
| modalSaveLogic.handleErroredItems(); | ||
| setFormValuesVersion(prev => prev + 1); | ||
| }, Constants.SLOW_DEBOUNCE), | ||
| [modalSaveLogic], | ||
| [modalSaveLogic, setSaveAlertVisible], | ||
| ); | ||
|
|
||
| const handleValuesChange = useCallback(() => { | ||
| setFormValuesVersion(prev => prev + 1); | ||
| debouncedErrorHandling(); | ||
| }, [debouncedErrorHandling]); | ||
| const handleValuesChange = useMemo( | ||
| () => debouncedHandleErroredItems, | ||
| [debouncedHandleErroredItems], | ||
| ); |
There was a problem hiding this comment.
This refactoring delays the formValuesVersion increment by 500ms, causing itemTitles (used in sidebar) and the collapse component reset to lag behind form changes, leading to stale UI display.
Code suggestion
Check the AI-generated fix before applying
| const debouncedHandleErroredItems = useMemo( | |
| () => | |
| debounce(() => { | |
| setSaveAlertVisible(false); | |
| modalSaveLogic.handleErroredItems(); | |
| setFormValuesVersion(prev => prev + 1); | |
| }, Constants.SLOW_DEBOUNCE), | |
| [modalSaveLogic], | |
| [modalSaveLogic, setSaveAlertVisible], | |
| ); | |
| const handleValuesChange = useCallback(() => { | |
| setFormValuesVersion(prev => prev + 1); | |
| debouncedErrorHandling(); | |
| }, [debouncedErrorHandling]); | |
| const handleValuesChange = useMemo( | |
| () => debouncedHandleErroredItems, | |
| [debouncedHandleErroredItems], | |
| ); | |
| const debouncedHandleErroredItems = useMemo( | |
| () => | |
| debounce(() => { | |
| setSaveAlertVisible(false); | |
| modalSaveLogic.handleErroredItems(); | |
| }, Constants.SLOW_DEBOUNCE), | |
| [modalSaveLogic, setSaveAlertVisible], | |
| [debouncedHandleErroredItems], | |
| ); | |
| const handleValuesChange = useCallback( | |
| () => { | |
| setFormValuesVersion(prev => prev + 1); | |
| debouncedHandleErroredItems(); | |
| }, [debouncedHandleErroredItems], | |
| ); |
Code Review Run #8113c0
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
There was a problem hiding this comment.
Code Review Agent Run #8dcabb
Actionable Suggestions - 1
-
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx - 1
- Test selector breakage from i18n · Line 99-99
Review Details
-
Files reviewed - 1 · Commit Range:
c4d78d6..809b070- superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
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 documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx
Show resolved
Hide resolved
5 participants
SUMMARY
This PR migrates DraggableFilter and related components from react-dnd to dnd-kit, replacing an unmaintained dependency with a modern, hooks-based drag-and-drop solution while preserving existing behavior.
The migration maintains container-level dragging, supports same-list reordering, and enables cross-list drag-and-drop via a shared DndContext.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
BEFORE:
2026-01-25.10-23-59.mp4
AFTER:
2026-01-26.09-13-46.mp4
TESTING INSTRUCTIONS
1)Open a dashboard .
2)Open the Filter Configuration / Customization sidebar.
3)Verify that filters can be:
4)Dragged to reorder within the same list
5)Dragged without interfering with Delete and Undo actions
6)Verify cross-list drag-and-drop behavior for dividers works as before.
7)Confirm that drag interactions work seamlessly.
ADDITIONAL INFORMATION