Compare commits

...

5 Commits

Author SHA1 Message Date
Evan
4cef74b422 fix: remove unused recalculating state in DropdownContainer
The shouldShowButton approach superseded the recalculating-based
trigger gating, leaving the recalculating state and its setters
unused (TS6133), which broke lint-frontend and validate-frontend.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-05 21:25:58 -07:00
Evan Rusackas
55d9e13725 address review: use Button tooltip prop so hint shows on disabled trigger
AntD disabled buttons swallow hover/focus, so the previous
<Tooltip><Button disabled /></Tooltip> wrapper left the
"No applied filters" hint unreachable in the exact state this PR
introduces (button visible, no popover content). The superset-ui Button
component's built-in `tooltip` prop already wraps disabled buttons in a
<span> so the tooltip fires — switch to that.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-06-05 20:15:22 -07:00
Evan Rusackas
ec13a2bb7b address review: guard empty popover and disable trigger when no content 2026-06-05 20:15:21 -07:00
Evan Rusackas
e274b45bc6 address review: timeless comment wording 2026-06-05 20:14:37 -07:00
Evan Rusackas
bd1420cfd8 fix(FilterBar): always show 'More filters' button when items exist
This fixes issue #28060 where the "More filters" button would disappear
when filter values were set to their defaults and nothing was overflowing.

The issue was caused by the button only rendering when `popoverContent`
was truthy, which required either `dropdownContent` to be defined OR
`overflowingCount > 0`. When neither condition was met (common when all
filters fit in the container), the button would vanish, causing:
- Inconsistent UI behavior
- Layout shifts as the button appears/disappears
- Poor user experience when resizing the browser window

The fix introduces a `shouldShowButton` flag that ensures the button is
always visible when items exist, regardless of overflow state. The badge
correctly shows 0 when nothing is overflowing, providing clear feedback
to users.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-06-05 20:14:36 -07:00
2 changed files with 74 additions and 79 deletions

View File

@@ -51,8 +51,16 @@ test('renders children with custom horizontal spacing', () => {
expect(screen.getByTestId('container')).toHaveStyle('gap: 20px'); expect(screen.getByTestId('container')).toHaveStyle('gap: 20px');
}); });
test('does not render a dropdown button when not overflowing', () => { test('renders dropdown button when items exist even when not overflowing', () => {
render(<DropdownContainer items={generateItems(3)} />); render(<DropdownContainer items={generateItems(3)} />);
// Button should always be visible when items exist to prevent layout shifts
expect(screen.getByText('More')).toBeInTheDocument();
// Badge should show 0 when nothing is overflowing
expect(screen.getByText('0')).toBeInTheDocument();
});
test('does not render a dropdown button when no items', () => {
render(<DropdownContainer items={[]} />);
expect(screen.queryByText('More')).not.toBeInTheDocument(); expect(screen.queryByText('More')).not.toBeInTheDocument();
}); });

View File

@@ -34,7 +34,7 @@ import { t } from '@apache-superset/core/translation';
import { usePrevious } from '@superset-ui/core'; import { usePrevious } from '@superset-ui/core';
import { css, useTheme } from '@apache-superset/core/theme'; import { css, useTheme } from '@apache-superset/core/theme';
import { useResizeDetector } from 'react-resize-detector'; import { useResizeDetector } from 'react-resize-detector';
import { Badge, Icons, Button, Tooltip, Popover } from '..'; import { Badge, Icons, Button, Popover } from '..';
import { DropdownContainerProps, DropdownItem, DropdownRef } from './types'; import { DropdownContainerProps, DropdownItem, DropdownRef } from './types';
const MAX_HEIGHT = 500; const MAX_HEIGHT = 500;
@@ -72,15 +72,6 @@ export const DropdownContainer = forwardRef(
const [showOverflow, setShowOverflow] = useState(false); const [showOverflow, setShowOverflow] = useState(false);
// When the item set changes, the overflow index is briefly reset while the
// new widths are measured (see the layout effect below). During that window
// the dropdown content momentarily becomes empty, which would hide and then
// re-show the trigger, causing a flicker. We track whether a recalculation
// is pending so the trigger can stay mounted across the transient (when it
// was showing content just before) without lingering in the steady state
// when nothing actually overflows.
const [recalculating, setRecalculating] = useState(false);
// callback to update item widths so that the useLayoutEffect runs whenever // callback to update item widths so that the useLayoutEffect runs whenever
// width of any of the child changes // width of any of the child changes
const recalculateItemWidths = useCallback(() => { const recalculateItemWidths = useCallback(() => {
@@ -180,7 +171,6 @@ export const DropdownContainer = forwardRef(
); );
} else { } else {
setOverflowingIndex(-1); setOverflowingIndex(-1);
setRecalculating(true);
return; return;
} }
} }
@@ -221,7 +211,6 @@ export const DropdownContainer = forwardRef(
} }
setOverflowingIndex(newOverflowingIndex); setOverflowingIndex(newOverflowingIndex);
setRecalculating(false);
} }
}, [ }, [
current, current,
@@ -245,6 +234,10 @@ export const DropdownContainer = forwardRef(
const overflowingCount = const overflowingCount =
overflowingIndex !== -1 ? items.length - overflowingIndex : 0; overflowingIndex !== -1 ? items.length - overflowingIndex : 0;
// Always show button when items exist to prevent layout shifts
// and ensure consistent UI even when no items are overflowing
const shouldShowButton = items.length > 0 || !!dropdownContent;
const popoverContent = useMemo( const popoverContent = useMemo(
() => () =>
dropdownContent || overflowingCount ? ( dropdownContent || overflowingCount ? (
@@ -272,15 +265,6 @@ export const DropdownContainer = forwardRef(
], ],
); );
// The trigger had content in the previous render if popoverContent was
// truthy then. During the brief mid-recalculation render where
// popoverContent flips to null, this still reflects the prior (non-empty)
// value, letting us keep the trigger mounted across the transient.
const hadPopoverContent = usePrevious(!!popoverContent, false);
const showDropdownButton =
!!popoverContent || (recalculating && hadPopoverContent);
useLayoutEffect(() => { useLayoutEffect(() => {
if (popoverVisible) { if (popoverVisible) {
// Measures scroll height after rendering the elements // Measures scroll height after rendering the elements
@@ -313,6 +297,44 @@ export const DropdownContainer = forwardRef(
}; };
}, [popoverVisible]); }, [popoverVisible]);
const triggerButton = (
<Button
buttonStyle="secondary"
data-test="dropdown-container-btn"
icon={dropdownTriggerIcon}
disabled={!popoverContent}
tooltip={dropdownTriggerTooltip}
css={css`
padding-left: ${theme.paddingXS}px;
padding-right: ${theme.paddingXXS}px;
gap: ${theme.sizeXXS}px;
`}
>
{dropdownTriggerText}
<Badge
count={dropdownTriggerCount ?? overflowingCount}
color={
(dropdownTriggerCount ?? overflowingCount) > 0
? theme.colorPrimary
: theme.colorTextSecondary
}
showZero
css={css`
margin-left: ${theme.sizeUnit * 2}px;
`}
/>
<Icons.DownOutlined
iconSize="m"
iconColor={theme.colorIcon}
css={css`
.anticon {
display: flex;
}
`}
/>
</Button>
);
return ( return (
<div <div
ref={ref} ref={ref}
@@ -334,7 +356,7 @@ export const DropdownContainer = forwardRef(
> >
{notOverflowedItems.map(item => item.element)} {notOverflowedItems.map(item => item.element)}
</div> </div>
{showDropdownButton && ( {shouldShowButton && (
<> <>
<Global <Global
styles={css` styles={css`
@@ -359,62 +381,27 @@ export const DropdownContainer = forwardRef(
`} `}
/> />
<Popover {popoverContent ? (
styles={{ <Popover
body: { styles={{
maxHeight: `${MAX_HEIGHT}px`, body: {
overflow: showOverflow ? 'auto' : 'visible', maxHeight: `${MAX_HEIGHT}px`,
}, overflow: showOverflow ? 'auto' : 'visible',
}} },
content={popoverContent} }}
trigger="click" content={popoverContent}
open={popoverVisible && !!popoverContent} trigger="click"
onOpenChange={visible => { open={popoverVisible}
// While a recalculation keeps the trigger mounted but there is onOpenChange={visible => setPopoverVisible(visible)}
// no content yet, ignore open attempts so it stays visible placement="bottom"
// without opening an empty popover. forceRender={forceRender}
if (popoverContent) setPopoverVisible(visible); fresh // This prop prevents caching and stale data for filter scoping.
}} >
placement="bottom" {triggerButton}
forceRender={forceRender} </Popover>
fresh // This prop prevents caching and stale data for filter scoping. ) : (
> triggerButton
<Tooltip title={dropdownTriggerTooltip}> )}
<Button
buttonStyle="secondary"
data-test="dropdown-container-btn"
icon={dropdownTriggerIcon}
css={css`
padding-left: ${theme.paddingXS}px;
padding-right: ${theme.paddingXXS}px;
gap: ${theme.sizeXXS}px;
`}
>
{dropdownTriggerText}
<Badge
count={dropdownTriggerCount ?? overflowingCount}
color={
(dropdownTriggerCount ?? overflowingCount) > 0
? theme.colorPrimary
: theme.colorTextSecondary
}
showZero
css={css`
margin-left: ${theme.sizeUnit * 2}px;
`}
/>
<Icons.DownOutlined
iconSize="m"
iconColor={theme.colorIcon}
css={css`
.anticon {
display: flex;
}
`}
/>
</Button>
</Tooltip>
</Popover>
</> </>
)} )}
</div> </div>