fix(dashboard): fix multiple drag-and-drop and edit mode issues (#37891)

This commit is contained in:
Enzo Martellucci
2026-03-05 16:47:11 +01:00
committed by GitHub
parent a513406239
commit f0416eff78
6 changed files with 69 additions and 24 deletions

View File

@@ -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')}

View File

@@ -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;

View File

@@ -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%;

View File

@@ -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 () => {

View File

@@ -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));

View File

@@ -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 }));