refactor(listview): CompactSelectPanel reuses Select/AsyncSelect

Replace custom option rendering, debounced search, race-condition
guards, and loading states with Select/AsyncSelect from
@superset-ui/core/components. The trigger is hidden via a
zero-height wrapper; the dropdown renders inside a container div
via getPopupContainer, giving the same visual as Explore's select
dropdowns. Also exposes the 'open' prop in AntdExposedProps so
Select/AsyncSelect can be controlled externally.

Removes ~130 lines of reimplemented debounce/race-condition/
filter logic and eliminates the outline hover hack.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
kasiazjc
2026-05-20 14:41:18 +00:00
committed by Amin Ghadersohi
parent eb945f8289
commit 6a16e7dca4
3 changed files with 229 additions and 405 deletions

View File

@@ -72,6 +72,7 @@ export type AntdExposedProps = Pick<
| 'getPopupContainer'
| 'menuItemSelectedIcon'
| 'dropdownAlign'
| 'open'
>;
export type SelectOptionsType = Exclude<AntdProps['options'], undefined>;

View File

@@ -17,67 +17,87 @@
* under the License.
*/
import { createRef, act } from 'react';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { render, screen } from 'spec/helpers/testing-library';
import CompactSelectPanel from './CompactSelectPanel';
import type { FilterHandler } from './types';
const SMALL_SELECTS = [
const SELECTS = [
{ label: 'Alice', value: 1 },
{ label: 'Bob', value: 2 },
{ label: 'Charlie', value: 3 },
];
const LARGE_SELECTS = [
{ label: 'Alice', value: 1 },
{ label: 'Bob', value: 2 },
{ label: 'Charlie', value: 3 },
{ label: 'David', value: 4 },
{ label: 'Eve', value: 5 },
{ label: 'Frank', value: 6 },
{ label: 'Grace', value: 7 },
];
jest.mock('@superset-ui/core/components', () => ({
Select: jest.fn(({ onChange, onOpenChange, value, open, ariaLabel }: any) => (
<div
data-test="mock-select"
data-open={String(open)}
aria-label={ariaLabel}
>
<button
type="button"
data-test="select-option-alice"
onClick={() => onChange({ label: 'Alice', value: 1 })}
>
Alice
</button>
<button
type="button"
data-test="select-option-bob"
onClick={() => onChange({ label: 'Bob', value: 2 })}
>
Bob
</button>
<button
type="button"
data-test="trigger-close"
onClick={() => onOpenChange?.(false)}
>
close
</button>
{value && <span data-test="selected-value">{String(value.value)}</span>}
</div>
)),
AsyncSelect: jest.fn(({ onChange, onOpenChange, open, ariaLabel }: any) => (
<div
data-test="mock-async-select"
data-open={String(open)}
aria-label={ariaLabel}
>
<button
type="button"
data-test="async-option-remote"
onClick={() => onChange({ label: 'Remote User', value: 99 })}
>
Remote User
</button>
<button
type="button"
data-test="async-trigger-close"
onClick={() => onOpenChange?.(false)}
>
close
</button>
</div>
)),
}));
beforeEach(() => {
jest.clearAllMocks();
});
test('renders options from selects prop', () => {
test('renders Select when selects prop is provided', () => {
render(
<CompactSelectPanel
selects={SMALL_SELECTS}
selects={SELECTS}
value={undefined}
onSelect={jest.fn()}
/>,
);
expect(screen.getByText('Alice')).toBeInTheDocument();
expect(screen.getByText('Bob')).toBeInTheDocument();
expect(screen.getByText('Charlie')).toBeInTheDocument();
expect(screen.getByTestId('mock-select')).toBeInTheDocument();
});
test('hides search input when selects.length is 6 or fewer', () => {
render(
<CompactSelectPanel
selects={SMALL_SELECTS}
value={undefined}
onSelect={jest.fn()}
/>,
);
expect(screen.queryByPlaceholderText('Search')).not.toBeInTheDocument();
});
test('shows search input when selects.length exceeds 6', () => {
render(
<CompactSelectPanel
selects={LARGE_SELECTS}
value={undefined}
onSelect={jest.fn()}
/>,
);
expect(screen.getByPlaceholderText('Search')).toBeInTheDocument();
});
test('shows search input when fetchSelects is provided', () => {
test('renders AsyncSelect when fetchSelects prop is provided', () => {
const fetchSelects = jest.fn().mockResolvedValue({ data: [], totalCount: 0 });
render(
<CompactSelectPanel
@@ -86,87 +106,87 @@ test('shows search input when fetchSelects is provided', () => {
onSelect={jest.fn()}
/>,
);
expect(screen.getByPlaceholderText('Search')).toBeInTheDocument();
expect(screen.getByTestId('mock-async-select')).toBeInTheDocument();
});
test('filters static options by search term', async () => {
test('passes isOpen as open prop to Select', () => {
render(
<CompactSelectPanel
selects={LARGE_SELECTS}
selects={SELECTS}
value={undefined}
onSelect={jest.fn()}
isOpen
/>,
);
await userEvent.type(screen.getByPlaceholderText('Search'), 'ali');
expect(screen.getByText('Alice')).toBeInTheDocument();
expect(screen.queryByText('Bob')).not.toBeInTheDocument();
expect(screen.getByTestId('mock-select')).toHaveAttribute(
'data-open',
'true',
);
});
test('calls onSelect with normalized option when an option is clicked', async () => {
test('calls onSelect with normalized option when onChange fires', () => {
const onSelect = jest.fn();
render(
<CompactSelectPanel
selects={SMALL_SELECTS}
selects={SELECTS}
value={undefined}
onSelect={onSelect}
/>,
);
await userEvent.click(screen.getByText('Alice'));
screen.getByTestId('select-option-alice').click();
expect(onSelect).toHaveBeenCalledWith({ label: 'Alice', value: 1 }, false);
});
test('calls onSelect with undefined when same option is clicked twice (deselect)', async () => {
test('calls onSelect(undefined, true) when onChange fires with undefined', () => {
const onSelect = jest.fn();
const { Select: MockSelect } = jest.requireMock(
'@superset-ui/core/components',
) as any;
MockSelect.mockImplementationOnce(({ onChange }: any) => (
<button
type="button"
data-test="trigger-clear"
onClick={() => onChange(undefined)}
>
clear
</button>
));
render(
<CompactSelectPanel
selects={SMALL_SELECTS}
selects={SELECTS}
value={{ label: 'Alice', value: 1 }}
onSelect={onSelect}
/>,
);
await userEvent.click(screen.getByText('Alice'));
screen.getByTestId('trigger-clear').click();
expect(onSelect).toHaveBeenCalledWith(undefined, true);
});
test('shows checkmark icon on selected option', () => {
render(
<CompactSelectPanel
selects={SMALL_SELECTS}
value={{ label: 'Alice', value: 1 }}
onSelect={jest.fn()}
/>,
);
const aliceOption = screen
.getByText('Alice')
.closest('[role="option"]') as HTMLElement;
expect(aliceOption).toHaveAttribute('aria-selected', 'true');
});
test('unselected options have aria-selected false', () => {
render(
<CompactSelectPanel
selects={SMALL_SELECTS}
value={{ label: 'Alice', value: 1 }}
onSelect={jest.fn()}
/>,
);
const bobOption = screen
.getByText('Bob')
.closest('[role="option"]') as HTMLElement;
expect(bobOption).toHaveAttribute('aria-selected', 'false');
});
test('calls onClose after a selection is made', async () => {
test('calls onClose after onChange fires', () => {
const onClose = jest.fn();
render(
<CompactSelectPanel
selects={SMALL_SELECTS}
selects={SELECTS}
value={undefined}
onSelect={jest.fn()}
onClose={onClose}
/>,
);
await userEvent.click(screen.getByText('Alice'));
screen.getByTestId('select-option-alice').click();
expect(onClose).toHaveBeenCalledTimes(1);
});
test('calls onClose when onOpenChange fires false', () => {
const onClose = jest.fn();
render(
<CompactSelectPanel
selects={SELECTS}
value={undefined}
onSelect={jest.fn()}
onClose={onClose}
/>,
);
screen.getByTestId('trigger-close').click();
expect(onClose).toHaveBeenCalledTimes(1);
});
@@ -176,154 +196,81 @@ test('clearFilter via ref resets selection and calls onSelect(undefined, true)',
render(
<CompactSelectPanel
ref={ref}
selects={SMALL_SELECTS}
selects={SELECTS}
value={{ label: 'Alice', value: 1 }}
onSelect={onSelect}
/>,
);
expect(screen.getByText('Alice').closest('[role="option"]')).toHaveAttribute(
'aria-selected',
'true',
);
act(() => {
ref.current?.clearFilter();
});
expect(onSelect).toHaveBeenCalledWith(undefined, true);
expect(screen.getByText('Alice').closest('[role="option"]')).toHaveAttribute(
'aria-selected',
'false',
);
});
test('shows Loading text when loading prop is true', () => {
render(
test('syncs selected state when external value prop changes', () => {
const { rerender } = render(
<CompactSelectPanel
selects={SMALL_SELECTS}
value={undefined}
selects={SELECTS}
value={{ label: 'Alice', value: 1 }}
onSelect={jest.fn()}
loading
/>,
);
expect(screen.getByText('Loading...')).toBeInTheDocument();
});
expect(screen.getByTestId('selected-value')).toHaveTextContent('1');
test('shows No results when displayOptions is empty', () => {
render(
<CompactSelectPanel selects={[]} value={undefined} onSelect={jest.fn()} />,
);
expect(screen.getByText('No results')).toBeInTheDocument();
});
test('renders options list with listbox role and accessible label', () => {
render(
rerender(
<CompactSelectPanel
selects={SMALL_SELECTS}
selects={SELECTS}
value={undefined}
onSelect={jest.fn()}
/>,
);
const listbox = screen.getByRole('listbox');
expect(listbox).toBeInTheDocument();
expect(listbox).toHaveAttribute('aria-label', 'Filter options');
expect(screen.queryByTestId('selected-value')).not.toBeInTheDocument();
});
test('option items have option role', () => {
render(
<CompactSelectPanel
selects={SMALL_SELECTS}
value={undefined}
onSelect={jest.fn()}
/>,
);
const options = screen.getAllByRole('option');
expect(options).toHaveLength(3);
});
test('fetches and displays remote options via fetchSelects on mount', async () => {
const fetchSelects = jest.fn().mockResolvedValue({
data: [{ label: 'Remote User', value: 99 }],
totalCount: 1,
});
render(
<CompactSelectPanel
fetchSelects={fetchSelects}
value={undefined}
onSelect={jest.fn()}
/>,
);
expect(screen.getByText('Loading...')).toBeInTheDocument();
await waitFor(() => {
expect(screen.getByText('Remote User')).toBeInTheDocument();
});
expect(fetchSelects).toHaveBeenCalledWith('', 0, 50);
});
test('shows No results when fetchSelects returns empty data', async () => {
test('passes isOpen as open prop to AsyncSelect', () => {
const fetchSelects = jest.fn().mockResolvedValue({ data: [], totalCount: 0 });
render(
<CompactSelectPanel
fetchSelects={fetchSelects}
value={undefined}
onSelect={jest.fn()}
isOpen
/>,
);
await waitFor(() => {
expect(screen.getByText('No results')).toBeInTheDocument();
});
expect(screen.getByTestId('mock-async-select')).toHaveAttribute(
'data-open',
'true',
);
});
test('shows No results when fetchSelects rejects', async () => {
const fetchSelects = jest.fn().mockRejectedValue(new Error('network error'));
test('calls onClose when AsyncSelect onOpenChange fires false', () => {
const onClose = jest.fn();
const fetchSelects = jest.fn().mockResolvedValue({ data: [], totalCount: 0 });
render(
<CompactSelectPanel
fetchSelects={fetchSelects}
value={undefined}
onSelect={jest.fn()}
onClose={onClose}
/>,
);
await waitFor(() => {
expect(screen.getByText('No results')).toBeInTheDocument();
});
screen.getByTestId('async-trigger-close').click();
expect(onClose).toHaveBeenCalledTimes(1);
});
test('selects option via keyboard Enter key', async () => {
test('calls onSelect on AsyncSelect onChange', () => {
const onSelect = jest.fn();
const fetchSelects = jest.fn().mockResolvedValue({ data: [], totalCount: 0 });
render(
<CompactSelectPanel
selects={SMALL_SELECTS}
fetchSelects={fetchSelects}
value={undefined}
onSelect={onSelect}
/>,
);
const aliceOption = screen.getByText('Alice').closest('[role="option"]')!;
await userEvent.type(aliceOption, '{Enter}');
expect(onSelect).toHaveBeenCalledWith({ label: 'Alice', value: 1 }, false);
});
test('syncs selected state when external value prop changes', () => {
const { rerender } = render(
<CompactSelectPanel
selects={SMALL_SELECTS}
value={{ label: 'Alice', value: 1 }}
onSelect={jest.fn()}
/>,
);
expect(screen.getByText('Alice').closest('[role="option"]')).toHaveAttribute(
'aria-selected',
'true',
);
rerender(
<CompactSelectPanel
selects={SMALL_SELECTS}
value={undefined}
onSelect={jest.fn()}
/>,
);
expect(screen.getByText('Alice').closest('[role="option"]')).toHaveAttribute(
'aria-selected',
'false',
screen.getByTestId('async-option-remote').click();
expect(onSelect).toHaveBeenCalledWith(
{ label: 'Remote User', value: 99 },
false,
);
});

View File

@@ -18,6 +18,7 @@
*/
import {
forwardRef,
useCallback,
useImperativeHandle,
useMemo,
useRef,
@@ -25,15 +26,9 @@ import {
useEffect,
type RefObject,
} from 'react';
import { debounce } from 'lodash';
import { t } from '@apache-superset/core/translation';
import { useTheme, styled, css } from '@apache-superset/core/theme';
import {
Icons,
Input,
Constants,
type InputRef,
} from '@superset-ui/core/components';
import { styled } from '@apache-superset/core/theme';
import { Select, AsyncSelect } from '@superset-ui/core/components';
import type { SelectOption, ListViewFilter as Filter } from '../types';
import type { FilterHandler } from './types';
@@ -43,80 +38,27 @@ interface CompactSelectPanelProps {
value?: SelectOption;
onSelect: (option: SelectOption | undefined, isClear?: boolean) => void;
onClose?: () => void;
/** Injected by CompactFilterTrigger via cloneElement — true when dropdown is open */
/** Injected by CompactFilterTrigger via cloneElement — true when the outer dropdown is open */
isOpen?: boolean;
/** External loading state from filter config */
loading?: boolean;
}
const PanelContainer = styled.div`
${({ theme }) => css`
min-width: 220px;
max-width: 320px;
max-height: 320px;
display: flex;
flex-direction: column;
border-radius: ${theme.borderRadiusLG}px;
overflow: hidden;
background: ${theme.colorBgElevated};
box-shadow: ${theme.boxShadowSecondary};
padding: ${theme.paddingXXS}px 0;
`}
/**
* Shows only the dropdown portion of Select/AsyncSelect by hiding the trigger
* via a zero-height wrapper and rendering the dropdown inside a container div
* via getPopupContainer. This gives the same visual as Explore's select dropdowns.
*/
const Container = styled.div`
position: relative;
`;
const SearchRow = styled.div`
${({ theme }) => css`
padding: 0 ${theme.sizeUnit * 2}px ${theme.paddingXXS}px;
`}
`;
const OptionList = styled.ul`
${({ theme }) => css`
margin: 0;
padding: 0;
overflow-y: auto;
flex: 1;
list-style: none;
`}
`;
const OptionItem = styled.li<{ $active: boolean }>`
${({ theme, $active }) => css`
display: flex;
align-items: center;
justify-content: space-between;
padding: 5px ${theme.sizeUnit * 3}px;
cursor: pointer;
font-size: ${theme.fontSize}px;
color: ${theme.colorText};
border-radius: ${theme.borderRadiusSM}px;
background: ${$active ? theme.colorPrimaryBg : 'transparent'};
transition: background 0.15s;
&:hover {
background: ${$active
? theme.colorPrimaryBgHover
: theme.colorFillTertiary};
outline: 2px solid ${theme.colorPrimary};
outline-offset: -2px;
}
`}
`;
const OptionLabel = styled.span`
const HiddenWrapper = styled.div`
position: absolute;
height: 0;
width: 100%;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
max-width: 240px;
`;
const StatusText = styled.div`
${({ theme }) => css`
padding: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 3}px;
text-align: center;
color: ${theme.colorTextDisabled};
font-size: ${theme.fontSizeSM}px;
`}
pointer-events: none;
`;
function CompactSelectPanel(
@@ -127,179 +69,113 @@ function CompactSelectPanel(
onSelect,
onClose,
isOpen,
loading: externalLoading,
loading,
}: CompactSelectPanelProps,
ref: RefObject<FilterHandler>,
) {
const theme = useTheme();
const inputRef = useRef<InputRef>(null);
const [search, setSearch] = useState('');
const [debouncedSearch, setDebouncedSearch] = useState('');
const [remoteOptions, setRemoteOptions] = useState<SelectOption[]>([]);
const [internalLoading, setInternalLoading] = useState(false);
const containerRef = useRef<HTMLDivElement>(null);
const [selectedOption, setSelectedOption] = useState<
SelectOption | undefined
>(value);
const isLoading = externalLoading || internalLoading;
const debouncedSetSearch = useMemo(
() => debounce(setDebouncedSearch, Constants.FAST_DEBOUNCE),
[],
);
useEffect(
() => () => {
debouncedSetSearch.cancel();
},
[debouncedSetSearch],
);
// Sync selected state when external value changes (e.g. clearFilters called from parent)
// Sync when external value changes (e.g. clearFilters from parent)
useEffect(() => {
setSelectedOption(value);
}, [value]);
// Focus search input when dropdown opens; reset search when it closes
useEffect(() => {
let timeoutId: ReturnType<typeof setTimeout>;
if (isOpen) {
timeoutId = setTimeout(() => {
inputRef.current?.input?.focus({ preventScroll: true });
}, 100);
} else {
setSearch('');
setDebouncedSearch('');
}
return () => {
if (timeoutId) clearTimeout(timeoutId);
};
}, [isOpen]);
// Fetch remote options when debounced search changes
useEffect(() => {
if (!fetchSelects) return;
let cancelled = false;
setInternalLoading(true);
fetchSelects(debouncedSearch, 0, 50)
.then(result => {
if (!cancelled) setRemoteOptions(result?.data ?? []);
})
.catch(() => {
if (!cancelled) setRemoteOptions([]);
})
.finally(() => {
if (!cancelled) setInternalLoading(false);
});
return () => {
cancelled = true;
};
}, [debouncedSearch, fetchSelects]);
useImperativeHandle(ref, () => ({
clearFilter: () => {
setSelectedOption(undefined);
setSearch('');
setDebouncedSearch('');
onSelect(undefined, true);
},
}));
const displayOptions = (
fetchSelects
? remoteOptions
: selects.filter(o => {
const label = typeof o.label === 'string' ? o.label : String(o.value);
return label.toLowerCase().includes(search.toLowerCase());
})
).filter(o => o != null);
const handleChange = useCallback(
(selected: SelectOption | undefined) => {
// Normalize to plain {label: string, value} to avoid circular-ref errors
// when emotion-styled ReactNode labels are URL-serialized.
const next = selected
? {
label:
typeof selected.label === 'string'
? selected.label
: String(selected.value ?? ''),
value: selected.value,
}
: undefined;
setSelectedOption(next);
onSelect(next, !selected);
onClose?.();
},
[onSelect, onClose],
);
// Show search for async selects or large static lists
const showSearch = !!fetchSelects || selects.length > 6;
const handleOpenChange = useCallback(
(visible: boolean) => {
if (!visible) onClose?.();
},
[onClose],
);
// displayText is the actual rendered text of the clicked list item, captured
// from the DOM via e.currentTarget.textContent. This is more reliable than
// reading opt.label, which may be a styled ReactNode (e.g. for owner options)
// rather than a plain string — causing tooltip to show the raw value instead.
const handleSelect = (opt: SelectOption, displayText?: string) => {
const isDeselect = selectedOption?.value === opt.value;
// Normalize to a plain object so the value can be safely serialized to
// URL query params without circular-reference errors from emotion metadata
// on styled ReactNode labels.
const next = isDeselect
? undefined
: {
label:
displayText ||
(typeof opt.label === 'string' ? opt.label : String(opt.value ?? '')),
value: opt.value,
};
setSelectedOption(next);
onSelect(next, isDeselect);
onClose?.();
};
const getPopupContainer = useCallback(
() => containerRef.current ?? document.body,
[],
);
const fetchAndFormat = useMemo(
() =>
fetchSelects
? async (inputValue: string, page: number, pageSize: number) => {
const result = await fetchSelects(inputValue, page, pageSize);
return {
data: result?.data ?? [],
totalCount: result?.totalCount ?? 0,
};
}
: undefined,
[fetchSelects],
);
const placeholder = t('Search');
return (
<PanelContainer>
{showSearch && (
<SearchRow>
<Input
ref={inputRef}
prefix={
<Icons.SearchOutlined iconSize="l" iconColor={theme.colorIcon} />
}
placeholder={t('Search')}
value={search}
onChange={e => {
setSearch(e.target.value);
debouncedSetSearch(e.target.value);
}}
<Container ref={containerRef}>
<HiddenWrapper>
{fetchSelects ? (
<AsyncSelect
open={isOpen}
value={selectedOption}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
onChange={handleChange as any}
options={fetchAndFormat!}
getPopupContainer={getPopupContainer}
onOpenChange={handleOpenChange}
showSearch
allowClear
css={css`
width: 100%;
box-shadow: none;
`}
loading={loading}
placeholder={placeholder}
ariaLabel={placeholder}
labelInValue
/>
</SearchRow>
)}
<OptionList role="listbox" aria-label={t('Filter options')}>
{isLoading ? (
<StatusText>{t('Loading...')}</StatusText>
) : displayOptions.length === 0 ? (
<StatusText>{t('No results')}</StatusText>
) : (
displayOptions.map(opt => {
const isActive = selectedOption?.value === opt.value;
const getDisplayText = (el: HTMLElement) =>
el.textContent?.trim() || undefined;
return (
<OptionItem
key={opt.value}
$active={isActive}
role="option"
aria-selected={isActive}
tabIndex={0}
onClick={e => handleSelect(opt, getDisplayText(e.currentTarget))}
onKeyDown={e => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
handleSelect(opt, getDisplayText(e.currentTarget));
}
}}
>
<OptionLabel>{opt.label}</OptionLabel>
{isActive && (
<Icons.CheckOutlined
iconSize="s"
iconColor={theme.colorPrimary}
/>
)}
</OptionItem>
);
})
<Select
open={isOpen}
value={selectedOption}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
onChange={handleChange as any}
options={selects}
getPopupContainer={getPopupContainer}
onOpenChange={handleOpenChange}
showSearch={selects.length > 6}
allowClear
loading={loading}
placeholder={placeholder}
ariaLabel={placeholder}
labelInValue
/>
)}
</OptionList>
</PanelContainer>
</HiddenWrapper>
</Container>
);
}