This is an automated email from the ASF dual-hosted git repository.
enzomartellucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new f0416eff78e fix(dashboard): fix multiple drag-and-drop and edit mode
issues (#37891)
f0416eff78e is described below
commit f0416eff78e738ed4fb725518d7567cad8631af3
Author: Enzo Martellucci <[email protected]>
AuthorDate: Thu Mar 5 16:47:11 2026 +0100
fix(dashboard): fix multiple drag-and-drop and edit mode issues (#37891)
---
.../DashboardBuilder/DashboardBuilder.tsx | 18 ++++++++++------
.../src/dashboard/components/dnd/DragDroppable.tsx | 9 ++++++++
.../components/gridComponents/Column/Column.tsx | 2 ++
.../gridComponents/Markdown/Markdown.test.tsx | 24 +++++++++++++++++++--
.../components/gridComponents/Row/Row.tsx | 25 ++++++++--------------
.../dashboard/components/menu/WithPopoverMenu.tsx | 15 +++++++++++++
6 files changed, 69 insertions(+), 24 deletions(-)
diff --git
a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
index 36dbd29e12f..d8593ff8b30 100644
---
a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
+++
b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
@@ -503,9 +503,9 @@ const DashboardBuilder = () => {
currentTopLevelTabs.current = topLevelTabs;
}, [topLevelTabs]);
- const renderDraggableContent = useCallback(
- ({ dropIndicatorProps }: { dropIndicatorProps: JsonObject }) => (
- <div>
+ const headerContent = useMemo(
+ () => (
+ <>
{!hideDashboardHeader && <DashboardHeader />}
{showFilterBar &&
filterBarOrientation === FilterBarOrientation.Horizontal && (
@@ -514,6 +514,14 @@ const DashboardBuilder = () => {
hidden={isReport}
/>
)}
+ </>
+ ),
+ [hideDashboardHeader, showFilterBar, filterBarOrientation, isReport],
+ );
+
+ const renderDraggableContent = useCallback(
+ ({ dropIndicatorProps }: { dropIndicatorProps: JsonObject }) => (
+ <div>
{dropIndicatorProps && <div {...dropIndicatorProps} />}
{!isReport && topLevelTabs && !uiConfig.hideNav && (
<WithPopoverMenu
@@ -542,12 +550,9 @@ const DashboardBuilder = () => {
</div>
),
[
- nativeFiltersEnabled,
- filterBarOrientation,
editMode,
handleChangeTab,
handleDeleteTopLevelTabs,
- hideDashboardHeader,
isReport,
topLevelTabs,
uiConfig.hideNav,
@@ -622,6 +627,7 @@ const DashboardBuilder = () => {
ref={headerRef}
filterBarWidth={headerFilterBarWidth}
>
+ {headerContent}
<Droppable
data-test="top-level-tabs"
className={cx(!topLevelTabs && editMode && 'empty-droptarget')}
diff --git a/superset-frontend/src/dashboard/components/dnd/DragDroppable.tsx
b/superset-frontend/src/dashboard/components/dnd/DragDroppable.tsx
index 7b1d60e59ff..7cb532cdac8 100644
--- a/superset-frontend/src/dashboard/components/dnd/DragDroppable.tsx
+++ b/superset-frontend/src/dashboard/components/dnd/DragDroppable.tsx
@@ -92,6 +92,15 @@ const DragDroppableStyles = styled.div`
&.dragdroppable-row {
width: 100%;
}
+ /* workaround to avoid a bug in react-dnd where the drag
+ preview expands outside of the bounds of the drag source card, see:
+ https://github.com/react-dnd/react-dnd/issues/832 */
+ &.dragdroppable-column {
+ /* for chrome */
+ transform: translate3d(0, 0, 0);
+ /* for safari */
+ backface-visibility: hidden;
+ }
&.dragdroppable-column .resizable-container span div {
z-index: 10;
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/Column/Column.tsx
b/superset-frontend/src/dashboard/components/gridComponents/Column/Column.tsx
index 6d8d5871abf..ecf9fde33de 100644
---
a/superset-frontend/src/dashboard/components/gridComponents/Column/Column.tsx
+++
b/superset-frontend/src/dashboard/components/gridComponents/Column/Column.tsx
@@ -109,6 +109,8 @@ const ColumnStyles = styled.div<{ editMode: boolean }>`
}
&:first-child:not(.droptarget-edge) {
position: absolute;
+ top: 0;
+ left: 0;
z-index: ${EMPTY_CONTAINER_Z_INDEX};
width: 100%;
height: 100%;
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/Markdown/Markdown.test.tsx
b/superset-frontend/src/dashboard/components/gridComponents/Markdown/Markdown.test.tsx
index 4794cba0fd6..142ab621213 100644
---
a/superset-frontend/src/dashboard/components/gridComponents/Markdown/Markdown.test.tsx
+++
b/superset-frontend/src/dashboard/components/gridComponents/Markdown/Markdown.test.tsx
@@ -403,6 +403,27 @@ test('shouldFocusMarkdown returns false when clicking
outside markdown container
expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
});
+test('should re-enter edit mode on a single click after clicking outside',
async () => {
+ await setup({ editMode: true });
+
+ const markdownContainer = screen.getByTestId(
+ 'dashboard-component-chart-holder',
+ );
+
+ // Click to enter edit mode
+ userEvent.click(markdownContainer);
+ expect(await screen.findByRole('textbox')).toBeInTheDocument();
+
+ // Click outside to exit edit mode
+ userEvent.click(document.body);
+ await new Promise(resolve => setTimeout(resolve, 50));
+ expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
+
+ // Click back inside — editor should appear on a single click
+ userEvent.click(markdownContainer);
+ expect(await screen.findByRole('textbox')).toBeInTheDocument();
+});
+
test('shouldFocusMarkdown keeps focus when clicking on menu items', async ()
=> {
await setup({ editMode: true });
@@ -417,9 +438,8 @@ test('shouldFocusMarkdown keeps focus when clicking on menu
items', async () =>
const editButton = screen.getByText('Edit');
userEvent.click(editButton);
- await new Promise(resolve => setTimeout(resolve, 50));
- expect(screen.queryByRole('textbox')).toBeInTheDocument();
+ expect(await screen.findByRole('textbox')).toBeInTheDocument();
});
test('should exit edit mode when clicking outside in same row', async () => {
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/Row/Row.tsx
b/superset-frontend/src/dashboard/components/gridComponents/Row/Row.tsx
index 5651712e29d..2ed137e9d12 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Row/Row.tsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Row/Row.tsx
@@ -22,6 +22,7 @@ import {
useCallback,
useRef,
useEffect,
+ useLayoutEffect,
useMemo,
memo,
RefObject,
@@ -30,7 +31,7 @@ import cx from 'classnames';
import { t } from '@apache-superset/core';
import { FeatureFlag, isFeatureEnabled, JsonObject } from '@superset-ui/core';
import { css, styled, SupersetTheme } from '@apache-superset/core/ui';
-import { Icons, Constants } from '@superset-ui/core/components';
+import { Icons } from '@superset-ui/core/components';
import {
Draggable,
Droppable,
@@ -47,7 +48,6 @@ import { BACKGROUND_TRANSPARENT } from
'src/dashboard/util/constants';
import { isEmbedded } from 'src/dashboard/util/isEmbedded';
import { EMPTY_CONTAINER_Z_INDEX } from 'src/dashboard/constants';
import { isCurrentUserBot } from 'src/utils/isBot';
-import { useDebouncedEffect } from '../../../../explore/exploreUtils';
export type RowProps = {
id: string;
@@ -215,20 +215,13 @@ const Row = memo((props: RowProps) => {
};
}, []);
- useDebouncedEffect(
- () => {
- const updatedHeight = containerRef.current?.clientHeight;
- if (
- editMode &&
- containerRef.current &&
- updatedHeight !== containerHeight
- ) {
- setContainerHeight(updatedHeight ?? null);
- }
- },
- Constants.FAST_DEBOUNCE,
- [editMode, containerHeight],
- );
+ useLayoutEffect(() => {
+ if (!editMode) return;
+ const updatedHeight = containerRef.current?.clientHeight;
+ if (updatedHeight !== undefined && updatedHeight !== containerHeight) {
+ setContainerHeight(updatedHeight);
+ }
+ });
const handleChangeFocus = useCallback((nextFocus: boolean) => {
setIsFocused(Boolean(nextFocus));
diff --git
a/superset-frontend/src/dashboard/components/menu/WithPopoverMenu.tsx
b/superset-frontend/src/dashboard/components/menu/WithPopoverMenu.tsx
index 4644bc3a7d8..38f96477ae8 100644
--- a/superset-frontend/src/dashboard/components/menu/WithPopoverMenu.tsx
+++ b/superset-frontend/src/dashboard/components/menu/WithPopoverMenu.tsx
@@ -112,6 +112,8 @@ export default class WithPopoverMenu extends PureComponent<
menuRef: HTMLDivElement | null;
+ focusEvent: Event | null;
+
static defaultProps = {
children: null,
disableClick: false,
@@ -136,6 +138,7 @@ export default class WithPopoverMenu extends PureComponent<
isFocused: props.isFocused!,
};
this.menuRef = null;
+ this.focusEvent = null;
this.setRef = this.setRef.bind(this);
this.setMenuRef = this.setMenuRef.bind(this);
this.handleClick = this.handleClick.bind(this);
@@ -181,6 +184,17 @@ export default class WithPopoverMenu extends PureComponent<
return;
}
+ // Skip if this is the same event that just triggered focus via onClick.
+ // The document-level listener registered during focus will see the same
+ // event bubble up; by that time a re-render may have detached the
+ // original event.target, causing shouldFocus to return false and
+ // immediately undoing the focus.
+ const nativeEvent = event.nativeEvent || event;
+ if (this.focusEvent === nativeEvent) {
+ this.focusEvent = null;
+ return;
+ }
+
const {
onChangeFocus,
shouldFocus: shouldFocusFunc,
@@ -194,6 +208,7 @@ export default class WithPopoverMenu extends PureComponent<
if (!disableClick && shouldFocus && !this.state.isFocused) {
document.addEventListener('click', this.handleClick);
document.addEventListener('drag', this.handleClick);
+ this.focusEvent = event.nativeEvent || event;
this.setState(() => ({ isFocused: true }));
