fix(dashboard): restore top-level tab drop target for dashboards with content#39423
fix(dashboard): restore top-level tab drop target for dashboards with content#39423sadpandajoe wants to merge 9 commits intomasterfrom
Conversation
|
🎪 Showtime deployed environment on GHA for 862b004 • Environment: http://34.221.68.241:8080 (admin/admin) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #39423 +/- ##
==========================================
- Coverage 64.37% 64.36% -0.01%
==========================================
Files 2569 2569
Lines 134745 134745
Branches 31278 31278
==========================================
- Hits 86739 86728 -11
- Misses 46508 46519 +11
Partials 1498 1498
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Restores a usable top-level tab drop target in dashboards that already have content by ensuring the header-area droppable has non-zero height, preventing react-dnd hover/drop failures after the header/droppable layout change in #37891.
Changes:
- Add a
min-heightrule for.empty-droptargetwithin the dashboard header wrapper styling. - Add a Jest test intended to verify the presence of the new styling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx | Adds min-height to the header-area empty droptarget so the top-level droppable remains hoverable. |
| superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx | Adds a regression test aiming to validate the new header droptarget min-height styling. |
|
Yes, the suggestion is valid — it improves test reliability by directly checking the droppable element's class and computed min-height, focusing on actual behavior rather than CSSOM internals. superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx |
…ds with content PR #37891 moved the DashboardHeader out of the root Droppable, leaving it with zero height when no top-level tabs exist. This made it impossible to drag a Tabs component onto dashboards that already have content. Add min-height to .empty-droptarget in StyledHeader, matching the pattern used by DashboardGrid for its drop targets. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🎪 Showtime deployed environment on GHA for 3fe5902 • Environment: http://54.71.224.38:8080 (admin/admin) |
…on-jest types - Assert min-height is '16px' (theme.sizeUnit * 4) instead of /\S+/ to catch zero-height regressions - Move @emotion/jest type reference from per-file directive to src/types/emotion-jest.d.ts for global availability Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🎪 Showtime deployed environment on GHA for 93066ad • Environment: http://34.217.75.247:8080 (admin/admin) |
|
🎪 Showtime deployed environment on GHA for d25c075 • Environment: http://52.24.63.201:8080 (admin/admin) |
|
🎪 Showtime deployed environment on GHA for 7d6513f • Environment: http://52.41.2.251:8080 (admin/admin) |
aminghadersohi
left a comment
There was a problem hiding this comment.
Approve — minimal, correct fix with a proper regression test. Two minor nits inline.
| * under the License. | ||
| */ | ||
|
|
||
| /// <reference types="@emotion/jest" /> |
There was a problem hiding this comment.
Nit: this triple-slash directive augments expect globally across all src/ code, not just tests. Harmless in practice since it only adds a type, but the augmentation could live more precisely in spec/helpers/setup.ts (which already does import { matchers } from '@emotion/jest') or via a test-only tsconfig types entry. Not a blocker — putting it in src/types/ is consistent with the existing project convention for ambient declarations.
Code Review Agent Run #fc1989Actionable 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 |
Use supersetTheme.sizeUnit * 4 instead of hard-coded '16px' so the test stays in sync with the source rule and resilient to theme token changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
🎪 Showtime deployed environment on GHA for b71c556 • Environment: http://35.165.228.132:8080 (admin/admin) |
|
🎪 Showtime deployed environment on GHA for 78fada7 • Environment: http://34.219.244.53:8080 (admin/admin) |
Code Review Agent Run #3bfab6Actionable 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 |
|
🎪 Showtime deployed environment on GHA for ff323ba • Environment: http://35.161.127.154:8080 (admin/admin) |
SUMMARY
PR #37891 moved the
DashboardHeaderout of the rootDroppableinDashboardBuilder.tsx, which left the drop target with zero height when no top-level tabs exist. This made it impossible to drag a "Tabs" component onto dashboards that already have content — the only way to create top-level tabs on an existing dashboard.The fix adds
min-heightto.empty-droptargetinStyledHeader, matching the pattern used byDashboardGridfor its empty drop targets.Root cause: The
Droppablerenders an empty<div>when no top-level tabs exist. Before #37891, theDashboardHeaderwas inside it (giving it physical height). After #37891, the header was extracted as a sibling, collapsing theDroppableto zero height — react-dnd cannot detect hover events on a zero-height element.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: Dragging a Tabs component from the builder pane onto a dashboard with existing charts shows no drop indicator and the drop does not register.
Screen.Recording.2026-05-01.at.2.48.34.PM.mov
After: The drop target in the header area has 16px min-height, the drop indicator appears, and top-level tabs can be created.
Screen.Recording.2026-05-01.at.2.46.42.PM.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION