This is an automated email from the ASF dual-hosted git repository.
sadpandajoe 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 4bce27f12f4 fix(dashboard): shrink row height on chart resize and
unblock resize handles above columns (#44359)
4bce27f12f4 is described below
commit 4bce27f12f486490fbd300ed89f114a3d6acc901
Author: Joe Li <[email protected]>
AuthorDate: Fri Sep 25 10:51:05 2026 -0700
fix(dashboard): shrink row height on chart resize and unblock resize
handles above columns (#44359)
Co-authored-by: Claude Opus 5 <[email protected]>
Co-authored-by: Evan Rusackas <[email protected]>
Co-authored-by: Evan Rusackas <[email protected]>
---
.../components/gridComponents/Row/Row.test.tsx | 43 ++++++-
.../components/gridComponents/Row/Row.tsx | 124 +++++++++++----------
.../src/dashboard/components/menu/HoverMenu.tsx | 4 +-
.../components/resizable/ResizableContainer.tsx | 3 +-
superset-frontend/src/dashboard/constants.ts | 6 +
5 files changed, 117 insertions(+), 63 deletions(-)
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/Row/Row.test.tsx
b/superset-frontend/src/dashboard/components/gridComponents/Row/Row.test.tsx
index 4c25e0f80e3..aaaee34efba 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Row/Row.test.tsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Row/Row.test.tsx
@@ -65,11 +65,20 @@ jest.mock('src/dashboard/components/dnd/DragDroppable', ()
=> ({
Droppable: ({
children,
depth,
+ style,
+ className,
}: {
children: (args: object) => React.ReactNode;
depth: number;
+ style?: React.CSSProperties;
+ className?: string;
}) => (
- <div data-test="mock-droppable" data-depth={depth}>
+ <div
+ data-test="mock-droppable"
+ data-depth={depth}
+ className={className}
+ style={style}
+ >
{children({})}
</div>
),
@@ -269,6 +278,38 @@ test('should increment the depth of its children', () => {
);
});
+test('row droptargets size via CSS instead of a measured pixel height that can
only grow (regression for #37644)', () => {
+ const { container } = setup({ editMode: true });
+ const getDroptargetHeights = () =>
+ Array.from(
+ container.querySelectorAll<HTMLElement>('.empty-droptarget--vertical'),
+ ).map(el => el.style.height);
+
+ // The leading droptarget (index 0) is absolutely positioned, so a
+ // percentage height resolves fine and stretches it to the row via CSS
+ // rather than a pixel value measured from the row's tallest chart -- it
+ // can never be left pinned to a chart's prior (larger) height after a
+ // resize. The droptarget after the chart is an in-flow flex item under
+ // GridRow's indefinite `height: fit-content`, where a percentage height
+ // resolves to `auto`/is ignored per the flexbox spec (defeating the
+ // `align-self: stretch` CSS already declares for it) -- it gets an
+ // explicit `auto` instead, letting that stretch actually apply.
+ expect(getDroptargetHeights()).toEqual(['100%', 'auto']);
+});
+
+test('trailing droptarget also gets a CSS-driven height when the row is full
(regression for #37644)', () => {
+ const { container } = setup({ editMode: true, occupiedColumnCount: 12 });
+ const getDroptargetHeights = () =>
+ Array.from(
+ container.querySelectorAll<HTMLElement>('.empty-droptarget--vertical'),
+ ).map(el => el.style.height);
+
+ // With no remaining columns, the droptarget after the last chart is
+ // treated as a side target too (isTrailingSideTarget), so it takes the
+ // same '100%' CSS height as the leading one instead of 'auto'.
+ expect(getDroptargetHeights()).toEqual(['100%', '100%']);
+});
+
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
describe('visibility handling for intersection observers', () => {
const mockIntersectionObserver = jest.fn();
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/Row/Row.tsx
b/superset-frontend/src/dashboard/components/gridComponents/Row/Row.tsx
index a641e5481c7..b5f4d3d0280 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Row/Row.tsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Row/Row.tsx
@@ -22,7 +22,6 @@ import {
useCallback,
useRef,
useEffect,
- useLayoutEffect,
useMemo,
memo,
RefObject,
@@ -102,6 +101,7 @@ const GridRow = styled.div<{ editMode: boolean }>`
align-self: center;
&.empty-droptarget--vertical {
min-width: ${theme.sizeUnit * 4}px;
+ align-self: stretch;
&:not(:last-child) {
width: ${theme.sizeUnit * 4}px;
}
@@ -183,7 +183,6 @@ const Row = memo((props: RowProps) => {
const [isFocused, setIsFocused] = useState(false);
const [isInView, setIsInView] = useState(false);
const [hoverMenuHovered, setHoverMenuHovered] = useState(false);
- const [containerHeight, setContainerHeight] = useState<number | null>(null);
const containerRef = useRef<HTMLDivElement | null>(null);
const isComponentVisibleRef = useRef(isComponentVisible);
@@ -284,14 +283,6 @@ const Row = memo((props: RowProps) => {
};
}, []);
- useLayoutEffect(() => {
- if (!editMode) return;
- const updatedHeight = containerRef.current?.clientHeight;
- if (updatedHeight !== undefined && updatedHeight !== containerHeight) {
- setContainerHeight(updatedHeight);
- }
- });
-
const handleChangeFocus = useCallback((nextFocus: boolean) => {
setIsFocused(Boolean(nextFocus));
}, []);
@@ -403,7 +394,7 @@ const Row = memo((props: RowProps) => {
)}
editMode
style={{
- height: rowItems.length > 0 ? containerHeight : '100%',
+ height: '100%',
...(rowItems.length > 0 && { width: 16 }),
}}
>
@@ -416,54 +407,68 @@ const Row = memo((props: RowProps) => {
<div css={emptyRowContentStyles as any}>{t('Empty row')}</div>
)}
{rowItems.length > 0 &&
- rowItems.map((componentId, itemIndex) => (
- <Fragment key={componentId}>
- <DashboardComponent
- key={componentId}
- id={componentId}
- parentId={rowComponent.id as string}
- depth={depth + 1}
- index={itemIndex}
- availableColumnCount={remainColumnCount}
- columnWidth={columnWidth}
- onResizeStart={onResizeStart}
- onResize={onResize}
- onResizeStop={onResizeStop}
- isComponentVisible={isComponentVisible}
- onChangeTab={onChangeTab}
- isInView={isInView}
- />
- {editMode && (
- <Droppable
- component={rowItems}
- parentComponent={rowComponent}
- depth={depth}
- index={itemIndex + 1}
- orientation="row"
- onDrop={handleComponentDrop}
- className={cx(
- 'empty-droptarget',
- 'empty-droptarget--vertical',
- remainColumnCount === 0 &&
- itemIndex === rowItems.length - 1 &&
- 'droptarget-side',
- )}
- editMode
- style={{
- height: containerHeight,
- ...(remainColumnCount === 0 &&
- itemIndex === rowItems.length - 1 && { width: 16 }),
- }}
- >
- {({
- dropIndicatorProps,
- }: {
- dropIndicatorProps: JsonObject;
- }) => dropIndicatorProps && <div {...dropIndicatorProps}
/>}
- </Droppable>
- )}
- </Fragment>
- ))}
+ rowItems.map((componentId, itemIndex) => {
+ const isTrailingSideTarget =
+ remainColumnCount === 0 && itemIndex === rowItems.length - 1;
+ return (
+ <Fragment key={componentId}>
+ <DashboardComponent
+ key={componentId}
+ id={componentId}
+ parentId={rowComponent.id as string}
+ depth={depth + 1}
+ index={itemIndex}
+ availableColumnCount={remainColumnCount}
+ columnWidth={columnWidth}
+ onResizeStart={onResizeStart}
+ onResize={onResize}
+ onResizeStop={onResizeStop}
+ isComponentVisible={isComponentVisible}
+ onChangeTab={onChangeTab}
+ isInView={isInView}
+ />
+ {editMode && (
+ <Droppable
+ component={rowItems}
+ parentComponent={rowComponent}
+ depth={depth}
+ index={itemIndex + 1}
+ orientation="row"
+ onDrop={handleComponentDrop}
+ className={cx(
+ 'empty-droptarget',
+ 'empty-droptarget--vertical',
+ isTrailingSideTarget && 'droptarget-side',
+ )}
+ editMode
+ style={{
+ // Only the last target in a full row (the absolutely
+ // positioned "side" target) actually resolves a
+ // percentage height -- for every other, in-flow
+ // target, GridRow's height is `fit-content`
+ // (indefinite), and a percentage height on a flex
+ // item under an indefinite-height parent resolves to
+ // `auto`/is ignored per the CSS flexbox spec, which
+ // defeats the `align-self: stretch` the CSS above
+ // already declares for it. `height: 'auto'` lets that
+ // stretch actually apply instead of collapsing the
+ // target to its content size.
+ height: isTrailingSideTarget ? '100%' : 'auto',
+ ...(isTrailingSideTarget && { width: 16 }),
+ }}
+ >
+ {({
+ dropIndicatorProps,
+ }: {
+ dropIndicatorProps: JsonObject;
+ }) =>
+ dropIndicatorProps && <div {...dropIndicatorProps} />
+ }
+ </Droppable>
+ )}
+ </Fragment>
+ );
+ })}
</GridRow>
</WithPopoverMenu>
),
@@ -471,7 +476,6 @@ const Row = memo((props: RowProps) => {
backgroundStyle.className,
backgroundStyle.value,
columnWidth,
- containerHeight,
depth,
editMode,
handleChangeBackground,
diff --git a/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx
b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx
index 58549b4bea2..24b78f72db0 100644
--- a/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx
+++ b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx
@@ -21,6 +21,8 @@ import { RefObject, ReactNode, useCallback, memo } from
'react';
import { styled } from '@apache-superset/core/theme';
import cx from 'classnames';
+import { HOVER_MENU_Z_INDEX } from 'src/dashboard/constants';
+
interface HoverMenuProps {
position?: 'left' | 'top';
innerRef?: RefObject<HTMLDivElement> | null;
@@ -32,7 +34,7 @@ const HoverStyleOverrides = styled.div`
.hover-menu {
opacity: 0;
position: absolute;
- z-index: 11; /* one more than DragDroppable */
+ z-index: ${HOVER_MENU_Z_INDEX};
font-size: ${({ theme }) => theme.fontSize};
}
diff --git
a/superset-frontend/src/dashboard/components/resizable/ResizableContainer.tsx
b/superset-frontend/src/dashboard/components/resizable/ResizableContainer.tsx
index a84631b61c5..40eb93c3a58 100644
---
a/superset-frontend/src/dashboard/components/resizable/ResizableContainer.tsx
+++
b/superset-frontend/src/dashboard/components/resizable/ResizableContainer.tsx
@@ -36,6 +36,7 @@ import {
BottomRightResizeHandle,
} from './ResizableHandle';
import { isMobileConsumptionEnabled } from 'src/hooks/useIsMobile';
+import { RESIZE_HANDLE_Z_INDEX } from 'src/dashboard/constants';
import resizableConfig from '../../util/resizableConfig';
import {
GRID_BASE_UNIT,
@@ -130,7 +131,7 @@ const StyledResizable = styled(Resizable)`
.resize-handle {
opacity: 0;
- z-index: 10;
+ z-index: ${RESIZE_HANDLE_Z_INDEX};
&--bottom-right {
position: absolute;
diff --git a/superset-frontend/src/dashboard/constants.ts
b/superset-frontend/src/dashboard/constants.ts
index aa90bd2dfc9..5bd12ffe0d4 100644
--- a/superset-frontend/src/dashboard/constants.ts
+++ b/superset-frontend/src/dashboard/constants.ts
@@ -43,6 +43,12 @@ export const FILTER_BAR_TABS_HEIGHT = 46;
export const BUILDER_SIDEPANEL_WIDTH = 374;
export const OVERWRITE_INSPECT_FIELDS = ['css', 'json_metadata.filter_scopes'];
export const EMPTY_CONTAINER_Z_INDEX = 10;
+// A grid component's hover menu floats outside the component's own box, so it
+// must paint above the drop targets it overlaps.
+export const HOVER_MENU_Z_INDEX = EMPTY_CONTAINER_Z_INDEX + 1;
+// A hover menu rendered above a column overlaps the bottom edge of whatever
+// sits above it, so resize handles have to win hit testing against it.
+export const RESIZE_HANDLE_Z_INDEX = HOVER_MENU_Z_INDEX + 1;
export const DEFAULT_CROSS_FILTER_SCOPING: NativeFilterScope = {
rootPath: [DASHBOARD_ROOT_ID],
