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 }) => ( -
+ const headerContent = useMemo( + () => ( + <> {!hideDashboardHeader && } {showFilterBar && filterBarOrientation === FilterBarOrientation.Horizontal && ( @@ -514,6 +514,14 @@ const DashboardBuilder = () => { hidden={isReport} /> )} + + ), + [hideDashboardHeader, showFilterBar, filterBarOrientation, isReport], + ); + + const renderDraggableContent = useCallback( + ({ dropIndicatorProps }: { dropIndicatorProps: JsonObject }) => ( +
{dropIndicatorProps &&
} {!isReport && topLevelTabs && !uiConfig.hideNav && ( {
), [ - nativeFiltersEnabled, - filterBarOrientation, editMode, handleChangeTab, handleDeleteTopLevelTabs, - hideDashboardHeader, isReport, topLevelTabs, uiConfig.hideNav, @@ -622,6 +627,7 @@ const DashboardBuilder = () => { ref={headerRef} filterBarWidth={headerFilterBarWidth} > + {headerContent} ` } &: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 }));