This is an automated email from the ASF dual-hosted git repository. msyavuz pushed a commit to branch msyavuz/fix/select-conflicts-bug in repository https://gitbox.apache.org/repos/asf/superset.git
commit 68375c656bfa10fc8c49bf2e5ad48aa6a1937c2a Merge: 51f719f8d4 a53907a646 Author: Mehmet Salih Yavuz <[email protected]> AuthorDate: Thu Apr 17 20:53:34 2025 +0300 Merge branch 'master' into msyavuz/fix/select-conflicts-bug .../DatabaseSelector/DatabaseSelector.test.tsx | 16 +- .../src/components/DropdownContainer/index.tsx | 36 +++ .../src/components/Select/CustomTag.tsx | 32 +- .../src/components/Select/Select.test.tsx | 255 +++++---------- superset-frontend/src/components/Select/Select.tsx | 349 ++++++++++++--------- superset-frontend/src/components/Select/styles.tsx | 21 +- superset-frontend/src/components/Select/types.ts | 4 + superset-frontend/src/components/Select/utils.tsx | 2 + .../FilterBar/FilterControls/FilterControl.tsx | 49 ++- .../components/controls/SelectControl.test.jsx | 9 +- .../components/Select/SelectFilterPlugin.test.tsx | 122 ++++++- .../components/Select/SelectFilterPlugin.tsx | 148 ++++++--- superset-frontend/src/filters/utils.ts | 4 +- superset-frontend/src/types/TagType.ts | 1 + 14 files changed, 624 insertions(+), 424 deletions(-) diff --cc superset-frontend/src/components/Select/CustomTag.tsx index eee84ed835,ab5d135ec9..750774fd11 --- a/superset-frontend/src/components/Select/CustomTag.tsx +++ b/superset-frontend/src/components/Select/CustomTag.tsx @@@ -17,12 -17,44 +17,9 @@@ * under the License. */ import { MouseEvent } from 'react'; - import { css } from '@superset-ui/core'; -// eslint-disable-next-line no-restricted-imports -import { Tag as AntdTag } from 'antd'; // TODO: Remove antd -import { styled, useCSSTextTruncation } from '@superset-ui/core'; -import { Tooltip } from '../Tooltip'; -import { CustomCloseIcon } from '../Tags/Tag'; +import { Tag } from '../Tag'; import { CustomTagProps } from './types'; - import { SELECT_ALL_VALUE } from './constants'; - import { NoElement } from './styles'; -const StyledTag = styled(AntdTag)` - & .ant-tag-close-icon { - display: inline-flex; - align-items: center; - margin-left: ${({ theme }) => theme.gridUnit}px; - } - - & .tag-content { - overflow: hidden; - text-overflow: ellipsis; - } -`; - -// TODO: use antd Tag props instead of any. Currently it's causing a typescript error -const Tag = (props: any) => { - const [tagRef, tagIsTruncated] = useCSSTextTruncation<HTMLSpanElement>(); - return ( - <Tooltip title={tagIsTruncated ? props.children : null}> - <StyledTag - closeIcon={props?.closable ? CustomCloseIcon : undefined} - {...props} - className="ant-select-selection-item" - > - <span className="tag-content" ref={tagRef}> - {props.children} - </span> - </StyledTag> - </Tooltip> - ); -}; - /** * Custom tag renderer */ @@@ -42,21 -74,9 +39,14 @@@ export const customTagRender = (props: } }; - if (value !== SELECT_ALL_VALUE) { - return ( - <Tag - onClick={onPreventMouseDown} - css={css` - display: flex; - align-items: center; - `} - name={label} - className="antd5-select-selection-item" - {...(props as object)} - > - {label} - </Tag> - ); - } - return <NoElement />; + return ( - <Tag onMouseDown={onPreventMouseDown} {...(props as object)}> ++ <Tag ++ onMouseDown={onPreventMouseDown} ++ name={label} ++ className="antd5-select-selection-item" ++ {...(props as object)} ++ > + {label} + </Tag> + ); }; diff --cc superset-frontend/src/components/Select/Select.test.tsx index d7785b0c29,4ed6a384b3..4928f01eba --- a/superset-frontend/src/components/Select/Select.test.tsx +++ b/superset-frontend/src/components/Select/Select.test.tsx @@@ -25,8 -25,7 +25,7 @@@ import waitFor, within, } from 'spec/helpers/testing-library'; -import Select from 'src/components/Select/Select'; +import { Select } from '.'; - import { SELECT_ALL_VALUE } from './constants'; type Option = { label: string; @@@ -283,23 -264,9 +264,18 @@@ test('should sort selected to the top w // should revert to original order clearAll(); await reopen(); - expect( - await matchOrder([ - selectAllOptionLabel(originalLabels.length), - ...originalLabels, - ]), - ).toBe(true); + expect(await matchOrder(originalLabels)).toBe(true); }); +test('order of selected values is preserved until dropdown is closed', async () => { + render(<Select {...defaultProps} mode="multiple" allowSelectAll={false} />); + const originalLabels = OPTIONS.map(option => option.label); + await open(); + userEvent.click(await findSelectOption(originalLabels[1])); + userEvent.click(await findSelectOption(originalLabels[5])); + expect(await matchOrder(originalLabels)).toBe(true); +}); + test('searches for label or value', async () => { const option = OPTIONS[11]; render(<Select {...defaultProps} />); diff --cc superset-frontend/src/components/Select/Select.tsx index 205dbf9187,efef785616..8f6af815bd --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@@ -25,37 -26,29 +25,35 @@@ import useState, useCallback, ClipboardEvent, + Ref, + ReactElement, } from 'react'; -import { ensureIsArray, t, usePrevious } from '@superset-ui/core'; -// eslint-disable-next-line no-restricted-imports -import { LabeledValue as AntdLabeledValue } from 'antd/lib/select'; // TODO: Remove antd +import { + ensureIsArray, + FAST_DEBOUNCE, - formatNumber, - NumberFormats, + t, + usePrevious, +} from '@superset-ui/core'; +import { + LabeledValue as AntdLabeledValue, + RefSelectProps, +} from 'antd-v5/es/select'; import { debounce, isEqual, uniq } from 'lodash'; -import { FAST_DEBOUNCE } from 'src/constants'; import { ++ dropDownRenderHelper, ++ getOption, ++ getSuffixIcon, getValue, ++ handleFilterOptionHelper, hasOption, isLabeledValue, - mapValues, - mapOptions, - isEqual as utilsIsEqual, - renderSelectOptions, - sortSelectedFirstHelper, - sortComparatorWithSearchHelper, -- handleFilterOptionHelper, -- dropDownRenderHelper, - sortSelectedFirstHelper, - getSuffixIcon, - mapValues, - mapOptions, - hasCustomLabels, -- getOption, isObject, - getSuffixIcon, ++ mapOptions, ++ mapValues, + sortComparatorWithSearchHelper, ++ sortSelectedFirstHelper, + isEqual as utilsIsEqual, } from './utils'; import { RawValue, SelectOptionsType, SelectProps } from './types'; import { @@@ -70,12 -63,10 +69,11 @@@ import EMPTY_OPTIONS, MAX_TAG_COUNT, TOKEN_SEPARATORS, - SELECT_ALL_VALUE, - SELECT_ALL_OPTION, - DEFAULT_SORT_COMPARATOR, + VIRTUAL_THRESHOLD, } from './constants'; import { customTagRender } from './CustomTag'; -import Button from '../Button'; +import { Space } from '../Space'; ++import { Button } from '../Button'; /** * This component is a customized version of the Antdesign 4.X Select component @@@ -176,14 -168,9 +176,10 @@@ const Select = forwardRef () => (Array.isArray(options) ? options.slice() : EMPTY_OPTIONS), [options], ); ++ const initialOptionsSorted = useMemo( - () => - [...initialOptions].sort((a, b) => { - if (a.value === SELECT_ALL_VALUE) return -1; - if (b.value === SELECT_ALL_VALUE) return 1; - return 0; - }), - [initialOptions], + () => initialOptions.slice().sort(sortSelectedFirst), + [initialOptions, sortSelectedFirst], ); const [selectOptions, setSelectOptions] = @@@ -673,25 -713,14 +726,15 @@@ <StyledCheckOutlined iconSize="m" aria-label="check" /> ) } - options={ - [ - selectAllEnabled && { - label: selectAllLabel(), - value: SELECT_ALL_VALUE, - className: 'select-all', - id: 'select-all', - }, - ...fullSelectOptions, - ].filter(Boolean) as SelectOptionsType - } - options={shouldRenderChildrenOptions ? undefined : visibleOptions} ++ options={visibleOptions} + optionRender={option => <Space>{option.label || option.value}</Space>} oneLine={oneLine} tagRender={customTagRender} + css={props.css} {...props} + showSearch={shouldShowSearch} ref={ref} - > - {shouldRenderChildrenOptions && renderSelectOptions(visibleOptions)} - </StyledSelect> + /> </StyledContainer> ); }, diff --cc superset-frontend/src/components/Select/styles.tsx index 57e6b3cb46,966d9ade42..9c63aaf42f --- a/superset-frontend/src/components/Select/styles.tsx +++ b/superset-frontend/src/components/Select/styles.tsx @@@ -17,9 -17,12 +17,10 @@@ * under the License. */ import { styled } from '@superset-ui/core'; +import { Select } from 'antd-v5'; import { Icons } from 'src/components/Icons'; -// eslint-disable-next-line no-restricted-imports -import { Spin, Tag } from 'antd'; // TODO: Remove antd -// eslint-disable-next-line no-restricted-imports -import AntdSelect from 'antd/lib/select'; // TODO: Remove antd +import { Spin } from '../Spin'; + import { Space } from '../Space'; export const StyledHeader = styled.span<{ headerPosition: string }>` ${({ theme, headerPosition }) => ` @@@ -44,23 -47,14 +45,19 @@@ export const StyledSelect = styled(Sele })<{ headerPosition?: string; oneLine?: boolean }>` ${({ theme, headerPosition, oneLine }) => ` flex: ${headerPosition === 'left' ? 1 : 0}; - && .ant-select-selector { - border-radius: ${theme.gridUnit}px; + line-height: ${theme.sizeXL}px; + + && .antd5-select-selection-search { + left: 0px; + } + + && .antd5-select-selection-item, .antd5-select-selection-placeholder { + max-height: ${theme.sizeXL}px; } - // Open the dropdown when clicking on the suffix - // This is fixed in version 4.16 - .ant-select-arrow .anticon:not(.ant-select-suffix) { - pointer-events: none; + .antd5-select-selection-item::after { + height: 0; + display: block !important; } - .select-all { - border-radius: 0; - border-bottom: 1px solid ${theme.colors.grayscale.light3}; - } ${ oneLine && ` @@@ -134,3 -136,19 +131,19 @@@ export const StyledErrorMessage = style overflow: hidden; text-overflow: ellipsis; `; + + export const StyledBulkActionsContainer = styled(Space)` + ${({ theme }) => ` - padding: ${theme.gridUnit}px; ++ padding: ${theme.sizeUnit}px; + display: flex; + justify-content: center; + border-top: 1px solid ${theme.colors.grayscale.light3}; + .superset-button { - color: ${theme.colors.primary.dark1}; - font-weight: ${theme.typography.weights.normal}; ++ color: ${theme.colorPrimaryText}; ++ font-weight: ${theme.fontWeightNormal}; + } + .superset-button:disabled { + background-color: transparent; + } + `} + `; diff --cc superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx index 096b7fde4b,76a162e7c5..ed555b9dde --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx @@@ -107,12 -109,12 +107,14 @@@ const HorizontalOverflowFilterControlCo } `; - const VerticalFormItem = styled(StyledFormItem)` - .antd5-form-item-label { + const VerticalFormItem = styled(StyledFormItem)<{ + inverseSelection: boolean; + }>` + .ant-form-item-label { overflow: visible; - label.ant-form-item-required:not(.ant-form-item-required-mark-optional) { + label.antd5-form-item-required:not( + .antd5-form-item-required-mark-optional + ) { &::after { display: none; } @@@ -143,8 -153,16 +155,16 @@@ const HorizontalFormItem = styled(Style } } - .ant-form-item-control { + .antd5-form-item-control { - width: ${({ theme }) => theme.sizeUnit * 41}px; + width: ${({ inverseSelection }) => (inverseSelection ? 252 : 164)}px; + } + + .select-container { + ${({ inverseSelection, theme }) => + inverseSelection && + ` + width: 164px; + `} } `; @@@ -177,10 -205,10 +207,10 @@@ const useFilterControlDisplay = FilterControlTitleBox: VerticalFilterControlTitleBox, FilterControlTitle: VerticalFilterControlTitle, }; - }, [orientation, overflow]); + }, [orientation, overflow, inverseSelection]); const ToolTipContainer = styled.div` - font-size: ${({ theme }) => theme.typography.sizes.m}px; + font-size: ${({ theme }) => theme.fontSize}px; display: flex; `; diff --cc superset-frontend/src/explore/components/controls/SelectControl.test.jsx index 10d39d08ba,4cf235a98e..fde1cfcae6 --- a/superset-frontend/src/explore/components/controls/SelectControl.test.jsx +++ b/superset-frontend/src/explore/components/controls/SelectControl.test.jsx @@@ -165,10 -163,13 +165,13 @@@ describe('SelectControl', () => }, }); fireEvent(selectorInput, paste); - const yearOption = screen.getByLabelText('1 year ago'); + const yearOption = screen.getAllByText('1 year ago')[1]; expect(yearOption).toBeInTheDocument(); - const weekOption = screen.getAllByText('1 week ago')[1]; - expect(weekOption).toBeInTheDocument(); + expect(yearOption).toHaveAttribute('aria-selected', 'true'); + const weekOption = screen.getByText(/1 week ago/, { + selector: 'div [role="option"]', + }); + expect(weekOption?.getAttribute('aria-selected')).toEqual('true'); }); describe('empty placeholder', () => { diff --cc superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx index 877832bc0d,cb1f2de7cc..17d779736d --- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx +++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx @@@ -253,16 -299,70 +299,72 @@@ describe('SelectFilterPlugin', () => coltypeMap={{ bval: 1 }} data={[{ bval: bigValue }]} setDataMask={jest.fn()} + showOverflow={false} />, + { + useRedux: true, + initialState: { + nativeFilters: { + filters: { + 'test-filter': { + name: 'Test Filter', + }, + }, + }, + dataMask: { + 'test-filter': { + extraFormData: {}, + filterState: { + value: [], + label: '', + excludeFilterValues: true, + }, + }, + }, + }, + }, ); - userEvent.click(screen.getByRole('combobox')); + const filterSelect = screen.getAllByRole('combobox')[0]; + userEvent.click(filterSelect); expect(await screen.findByRole('combobox')).toBeInTheDocument(); await userEvent.type(screen.getByRole('combobox'), '1'); - expect(screen.queryByLabelText(String(bigValue))).toBeInTheDocument(); + expect( + await screen.findByRole('option', { name: String(bigValue) }), + ).toBeInTheDocument(); }); + test('Is/Is Not select is visible when inverseSelection is true', () => { + getWrapper({ inverseSelection: true }); + expect(screen.getByText('is not')).toBeInTheDocument(); + }); + + test('Is/Is Not select is not visible when inverseSelection is false', () => { + getWrapper({ inverseSelection: false }); + expect(screen.queryByText('is not')).not.toBeInTheDocument(); + }); + + test('Is/Is Not select toggles correctly', async () => { + getWrapper({ inverseSelection: true }); + + const isNotSelect = screen.getByText('is not'); + expect(isNotSelect).toBeInTheDocument(); + + // Click to open dropdown + userEvent.click(isNotSelect); + + // Click "is" option + userEvent.click(screen.getByText('is')); + + // Should update excludeFilterValues to false + expect(setDataMask).toHaveBeenCalledWith( + expect.objectContaining({ + filterState: expect.objectContaining({ + excludeFilterValues: false, + }), + }), + ); + }); + test('Should not allow for new values when creatable is false', () => { getWrapper({ creatable: false }); userEvent.type(screen.getByRole('combobox'), 'new value'); diff --cc superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx index 594f51a35e,0efb93a89b..89b76658d2 --- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx @@@ -29,16 -29,21 +29,18 @@@ import finestTemporalGrainFormatter, t, tn, + styled, } from '@superset-ui/core'; - import { debounce } from 'lodash'; -// eslint-disable-next-line no-restricted-imports -import { LabeledValue as AntdLabeledValue } from 'antd/lib/select'; // TODO: Remove antd + import { debounce, isUndefined } from 'lodash'; import { useImmerReducer } from 'use-immer'; - import { Select, FormItem, type LabeledValue } from 'src/components'; -import { Select } from 'src/components'; -// eslint-disable-next-line no-restricted-imports -import { Space } from 'antd'; // Import Space directly from antd ++import { FormItem, LabeledValue, Select } from 'src/components'; ++import { Space } from 'src/components/Space'; import { SLOW_DEBOUNCE } from 'src/constants'; import { hasOption, propertyComparator } from 'src/components/Select/utils'; import { FilterBarOrientation } from 'src/dashboard/types'; - import { PluginFilterSelectProps, SelectValue } from './types'; - import { FilterPluginStyle, StatusMessage } from '../common'; import { getDataRecordFormatter, getSelectExtraFormData } from '../../utils'; -import { FilterPluginStyle, StatusMessage, StyledFormItem } from '../common'; ++import { FilterPluginStyle, StatusMessage } from '../common'; + import { PluginFilterSelectProps, SelectValue } from './types'; type DataMaskAction = | { type: 'ownState'; ownState: JsonObject } @@@ -286,46 -322,83 +319,83 @@@ export default function PluginFilterSel setDataMask(dataMask); }, [JSON.stringify(dataMask)]); + useEffect(() => { + dispatchDataMask({ + type: 'filterState', + extraFormData: getSelectExtraFormData( + col, + filterState.value, + !filterState.value?.length, + excludeFilterValues && inverseSelection, + ), + filterState: { + ...(filterState as { + value: SelectValue; + label?: string; + excludeFilterValues?: boolean; + }), + excludeFilterValues, + }, + }); + }, [excludeFilterValues]); + + const handleExclusionToggle = (value: string) => { + setExcludeFilterValues(value === 'true'); + }; + return ( <FilterPluginStyle height={height} width={width}> - <StyledFormItem + <FormItem validateStatus={filterState.validateStatus} extra={formItemExtra} > - <Select - name={formData.nativeFilterId} - allowClear - allowNewOptions={!searchAllOptions && creatable !== false} - allowSelectAll={!searchAllOptions} - // @ts-ignore - value={filterState.value || []} - disabled={isDisabled} - getPopupContainer={ - showOverflow - ? () => (parentRef?.current as HTMLElement) || document.body - : (trigger: HTMLElement) => - (trigger?.parentNode as HTMLElement) || document.body - } - showSearch={showSearch} - mode={multiSelect ? 'multiple' : 'single'} - placeholder={placeholderText} - onClear={() => onSearch('')} - onSearch={onSearch} - onBlur={handleBlur} - onFocus={setFocusedFilter} - onMouseEnter={setHoveredFilter} - onMouseLeave={unsetHoveredFilter} - // @ts-ignore - onChange={handleChange} - ref={inputRef} - loading={isRefreshing} - oneLine={filterBarOrientation === FilterBarOrientation.Horizontal} - invertSelection={inverseSelection} - options={options} - sortComparator={sortComparator} - onDropdownVisibleChange={setFilterActive} - /> + <StyledSpace $inverseSelection={inverseSelection}> + {inverseSelection && ( + <Select + className="exclude-select" + value={`${excludeFilterValues}`} + options={[ + { value: 'true', label: t('is not') }, + { value: 'false', label: t('is') }, + ]} + onChange={handleExclusionToggle} + /> + )} + <Select + name={formData.nativeFilterId} + allowClear + allowNewOptions={!searchAllOptions && creatable !== false} + allowSelectAll={!searchAllOptions} + value={filterState.value || []} + disabled={isDisabled} + getPopupContainer={ + showOverflow + ? () => (parentRef?.current as HTMLElement) || document.body + : (trigger: HTMLElement) => + (trigger?.parentNode as HTMLElement) || document.body + } + showSearch={showSearch} + mode={multiSelect ? 'multiple' : 'single'} + placeholder={placeholderText} + onClear={() => onSearch('')} + onSearch={onSearch} + onBlur={handleBlur} + onFocus={setFocusedFilter} + onMouseEnter={setHoveredFilter} + onMouseLeave={unsetHoveredFilter} + // @ts-ignore + onChange={handleChange} + ref={inputRef} + loading={isRefreshing} + oneLine={filterBarOrientation === FilterBarOrientation.Horizontal} + invertSelection={inverseSelection && excludeFilterValues} + options={options} + sortComparator={sortComparator} + onDropdownVisibleChange={setFilterActive} + className="select-container" + /> + </StyledSpace> - </StyledFormItem> + </FormItem> </FilterPluginStyle> ); } diff --cc superset-frontend/src/types/TagType.ts index 62e78a6383,ef68b1c1ac..04db0c18e6 --- a/superset-frontend/src/types/TagType.ts +++ b/superset-frontend/src/types/TagType.ts @@@ -26,17 -25,10 +26,18 @@@ export interface TagType editable?: boolean; onDelete?: (index: number) => void; onClick?: MouseEventHandler<HTMLSpanElement>; - name: string; - index?: number | undefined; ++ onMouseDown?: MouseEventHandler<HTMLSpanElement>; + onClose?: () => void; + color?: string; + name?: string; + index?: number; toolTipTitle?: string; - children?: React.ReactNode; + children?: ReactNode; + role?: string; + style?: CSSProperties; + icon?: ReactNode; + css?: SerializedStyles; + closable?: boolean; } export default TagType;
