feat(folders): drag and drop entire folders in explore view - #42483
feat(folders): drag and drop entire folders in explore view#42483greggailly wants to merge 13 commits into
Conversation
Code Review Agent Run #20b69eActionable 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 |
dosubot
Bot
added
change:frontend
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
|
The flagged issue is correct. The current implementation of To resolve this, the logic should be updated to evaluate the folder's child items when the active item type is superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #42483 +/- ##
==========================================
+ Coverage 66.55% 66.58% +0.03%
==========================================
Files 2864 2866 +2
Lines 161894 162010 +116
Branches 37305 37349 +44
==========================================
+ Hits 107748 107881 +133
+ Misses 52100 52083 -17
Partials 2046 2046
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review Agent Run #c563d8Actionable Suggestions - 0Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
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 |
|
@greggailly nice feature, and the regression tests for the single-value folder-drop bug are a good touch. Codeant's thread on |
65d7635 to
9dd953d
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 #e7ae6f
Actionable Suggestions - 1
-
superset-frontend/src/explore/components/DatasourcePanel/types.ts - 1
- Dead code: unused exported interface · Line 36-40
Additional Suggestions - 4
-
superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx - 1
-
Missing component integration test · Line 129-136While resolveCanDrop has unit tests for the pure logic (lines 106-216 of test file), the component-level integration that connects canDrop state to UI feedback is not covered. Tests should verify the ghost button state during folder drags.
-
-
superset-frontend/src/explore/components/DndItemType.ts - 1
-
Missing unit test for new enum · Line 41-42Per adaptive rule [11730], new enum values introduced to support feature functionality should have explicit unit test coverage. While `DndItemType.Folder` is referenced in 19+ locations and tested indirectly via `DndSelectLabel.test.tsx`, there is no dedicated test verifying the `Folder` enum member itself.
-
-
superset-frontend/src/explore/components/DatasourcePanel/types.ts - 1
-
Misleading JSDoc on unused interface · Line 30-35The JSDoc comment on `FolderDndItem` is misleading. It claims drop targets can "reuse their per-item `canDrop`/`onDrop` logic" via the `items` array, but `DndSelectLabel.tsx:81` shows the actual API is `onDropFolder?: (items: DatasourcePanelDndItem[]) => void` — items are passed directly, not wrapped in a `FolderDndItem`. The comment describes functionality that does not exist in the codebase.
-
-
superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx - 1
-
Missing folder drop tests · Line 100-121The `onDropFolder` handler (lines 100-121) is consistent with the `resolveDragEnd` pattern in ExploreDndContext (lines 207-228) and mirrors `DndMetricSelect.tsx` (line 421). No bugs found. However, `DndColumnSelect.test.tsx` lacks coverage for folder drops — add tests similar to the existing `ExploreDndContext.test.tsx` folder-drop cases to verify: (1) folder drop adds accepted columns to multi-select, (2) single-select replaces existing value, (3) empty columnNames is a no-op.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset-frontend/src/explore/components/DatasourcePanel/folderDrag.test.ts - 1
- TypeScript: remove any casts · Line 70-73
Review Details
-
Files reviewed - 17 · Commit Range:
5b39872..96054b6- superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanelItem.tsx
- superset-frontend/src/explore/components/DatasourcePanel/folderDrag.test.ts
- superset-frontend/src/explore/components/DatasourcePanel/folderDrag.ts
- superset-frontend/src/explore/components/DatasourcePanel/types.ts
- superset-frontend/src/explore/components/DndItemType.ts
- superset-frontend/src/explore/components/ExploreContainer/ExploreDndContext.test.tsx
- superset-frontend/src/explore/components/ExploreContainer/ExploreDndContext.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnMetricSelect.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnMetricSelect.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/createAdhocMetricFromColumn.test.ts
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/dndTestUtils.ts
-
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
Code Review Agent Run #e802eeActionable 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 |
| export function collectFolderDragItems( | ||
| folder: Folder, | ||
| ): DatasourcePanelDndItem[] { | ||
| const items = folder.items.map(toDndItem); |
There was a problem hiding this comment.
When datasource search is active, this Folder was built from the filtered column/metric arrays, so dragging its header adds only the visible matches rather than the entire folder promised by this feature. Should the drag payload be built from the unfiltered folder contents while the rendered rows remain filtered?
There was a problem hiding this comment.
Not sure what would be best here. I guess personally I prefer only dragging "in" the visible fields. If that is ok I can change the PR title/description. If you prefer that all columns be dropped no matter what no problem I'll make it happen !
|
@greggailly this is showing conflicts with master now, needs a rebase. Also, @sadpandajoe's comments above (the folder-drag accessibility gap and the semantic-layer/search-filter gating questions) are still open, worth a look before this is ready to merge. Holler if you ant a hand with any of it. |
3d50ec7 to
0b0b44e
Compare
@rusackas something seems off with master (might be on my side but can't see dataset columns anymore in the explore view) so hard to rebase for the moment. I will try to nuke my setup when possible just in case. 
|
There was a problem hiding this comment.
Code Review Agent Run #aaaf4c
Actionable Suggestions - 1
-
superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx - 1
- Missing duplicate guard in multi add · Line 116-116
Additional Suggestions - 2
-
superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx - 1
-
Missing useCallback dependency · Line 267-267The `canDrop` callback reads `disallowAdhocMetrics` (line 247) but omits it from its dependency array, creating a React stale-closure risk where the metricrestriction state may be stale when the datasource prop changes mid-session. This could allow invalid drops to pass validation.
-
-
superset-frontend/src/explore/components/ExploreContainer/ExploreDndContext.test.tsx - 1
-
Use DndItemType enum instead of string literals · Line 47-49The new test section uses plain string literals (`'column'`, `'metric'`, `'folder'`) for drag types while the rest of the codebase consistently uses the `DndItemType` enum (e.g. `DndItemType.Column`). Using the enum here aligns with established conventions and prevents subtle type mismatches that casting-based tests could mask.
-
Review Details
-
Files reviewed - 16 · Commit Range:
c6e45a0..0b0b44e- superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanelItem.tsx
- superset-frontend/src/explore/components/DatasourcePanel/folderDrag.test.ts
- superset-frontend/src/explore/components/DatasourcePanel/folderDrag.ts
- superset-frontend/src/explore/components/DndItemType.ts
- superset-frontend/src/explore/components/ExploreContainer/ExploreDndContext.test.tsx
- superset-frontend/src/explore/components/ExploreContainer/ExploreDndContext.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnMetricSelect.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnMetricSelect.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/createAdhocMetricFromColumn.test.ts
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/dndTestUtils.ts
-
Files skipped - 0
-
Tools
- Eslint (Linter) - ✔︎ Successful
- 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
…text to avoiding collapsing to 0
…flict with expand/collapse keyboard behaviour
0b0b44e to
d097af9
Compare
|
@rusackas cherry-picked from #43056 to be able to rebase and test it successfully. Let me know if you wish me to revert this. |
There was a problem hiding this comment.
Code Review Agent Run #8caafe
Actionable Suggestions - 1
-
superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx - 1
- Type mismatch inconsistency · Line 255-255
Additional Suggestions - 5
-
superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx - 1
-
Missing dependency in useCallback · Line 438-438The `onDropFolder` callback calls `setValue(newValue)` on line 435 but does not include `setValue` in its dependency array. This could cause a stale closure in edge cases where the component re-renders with an outdated `setValue` reference.
-
-
superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanelItem.tsx - 1
-
WCAG Enhancement: aria-roledescription · Line 291-294The FolderDragHandle lacks `aria-roledescription` which other sortable/draggable elements in the codebase use (FoldersEditor.test.tsx line 336: 'dnd-kit adds aria-roledescription="sortable" to sortable elements'). This attribute helps screen readers describe the interaction pattern.
-
-
superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx - 1
-
Typo in variable name · Line 607-607Typo in variable name: `unknowTypeColumn` should be `unknownTypeColumn`. The test comments at lines 644 and 645 correctly say "Other columns default to COUNT_DISTINCT as well", but the misspelled identifier makes it look like a wrong-type constant rather than an unknown-type one.
-
-
superset-frontend/src/explore/components/DatasourcePanel/DatasourceItems.tsx - 1
-
Dead code: redundant maxHeight · Line 151-151The `maxHeight: height` property is semantically redundant since `height` is already specified with the same value. Setting `maxHeight` equal to `height` provides no additional constraint — it's functionally equivalent to not setting it at all. This adds unnecessary styling code that creates maintenance confusion with no benefit.
-
-
superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx - 1
-
Misleading comment about filter scope · Line 102-102The comment at line 102 reads "Items already passed `canDrop` (in options, not already selected)." However, the upstream filter in `ExploreDndContext.tsx` (lines 229–234) gates items by both `accept` type inclusion AND `canDrop` validation. The current phrasing misrepresents the upstream contract and could mislead future maintainers who expect `canDrop` alone to be the gating mechanism.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanelItem.tsx - 1
- CWE-269: Keyboard Inaccessible Drag Handle · Line 289-297
Review Details
-
Files reviewed - 23 · Commit Range:
dc2497a..d097af9- superset-frontend/src/dashboard/components/SliceAdder.tsx
- superset-frontend/src/explore/components/DatasourcePanel/DatasourceItems.tsx
- superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanelDragOption/DatasourcePanelDragOption.test.tsx
- superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanelDragOption/index.tsx
- superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanelItem.test.tsx
- superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanelItem.tsx
- superset-frontend/src/explore/components/DatasourcePanel/compatibility.test.ts
- superset-frontend/src/explore/components/DatasourcePanel/compatibility.ts
- superset-frontend/src/explore/components/DatasourcePanel/folderDrag.test.ts
- superset-frontend/src/explore/components/DatasourcePanel/folderDrag.ts
- superset-frontend/src/explore/components/DndItemType.ts
- superset-frontend/src/explore/components/ExploreContainer/ExploreDndContext.test.tsx
- superset-frontend/src/explore/components/ExploreContainer/ExploreDndContext.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnMetricSelect.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnMetricSelect.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.test.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/createAdhocMetricFromColumn.test.ts
- superset-frontend/src/explore/components/controls/DndColumnSelectControl/dndTestUtils.ts
-
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
Code Review Agent Run #b51e8dActionable 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 |
3 participants
feat(folders): drag and drop entire folders in explore view
SUMMARY
Allow to drag and drop all columns of a single folder (including subfolders) onto dnd fields (group by, metrics etc...) in explore view.
Folder can be grabbed by the header.
A small overlay showing the number of columns dragged shows up when dragging.
Already added columns are not added.
If dropped on metrics the "SUM" aggregation is applied by default for numeric values and count distinct for text values .
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Columns with a subfolder:
Metrics just a single subfolder:
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION