diff --git a/superset-frontend/src/components/DropdownContainer/index.tsx b/superset-frontend/src/components/DropdownContainer/index.tsx index 0735ba289eab..908e79ed8dee 100644 --- a/superset-frontend/src/components/DropdownContainer/index.tsx +++ b/superset-frontend/src/components/DropdownContainer/index.tsx @@ -179,6 +179,42 @@ const DropdownContainer = forwardRef( [items, overflowingIndex], ); + useEffect(() => { + const container = current?.children.item(0); + if (!container) return; + + const childrenArray = Array.from(container.children); + + const resizeObserver = new ResizeObserver(() => { + recalculateItemWidths(); + }); + + childrenArray.map(child => resizeObserver.observe(child)); + + // eslint-disable-next-line consistent-return + return () => { + childrenArray.map(child => resizeObserver.unobserve(child)); + resizeObserver.disconnect(); + }; + }, [items.length]); + + // callback to update item widths so that the useLayoutEffect runs whenever + // width of any of the child changes + const recalculateItemWidths = () => { + const container = current?.children.item(0); + if (container) { + const { children } = container; + const childrenArray = Array.from(children); + + const currentWidths = childrenArray.map( + child => child.getBoundingClientRect().width, + ); + + // Update state with new widths + setItemsWidth(currentWidths); + } + }; + useLayoutEffect(() => { if (popoverVisible) { return; diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index 0a3e04f88d3c..273eb2fa3a81 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -89,6 +89,7 @@ import { customTagRender } from './CustomTag'; const Select = forwardRef( ( { + className, allowClear, allowNewOptions = false, allowSelectAll = true, @@ -121,6 +122,7 @@ const Select = forwardRef( getPopupContainer, oneLine, maxTagCount: propsMaxTagCount, + ...props }: SelectProps, ref: RefObject, @@ -604,7 +606,7 @@ const Select = forwardRef( }; return ( - + {header && ( {header} )} diff --git a/superset-frontend/src/components/Select/types.ts b/superset-frontend/src/components/Select/types.ts index 85d380bdfa05..404253ef15cf 100644 --- a/superset-frontend/src/components/Select/types.ts +++ b/superset-frontend/src/components/Select/types.ts @@ -72,6 +72,10 @@ export type AntdExposedProps = Pick< export type SelectOptionsType = Exclude; export interface BaseSelectProps extends AntdExposedProps { + /** + * Optional CSS class name to apply to the select container + */ + className?: string; /** * It enables the user to create new options. * Can be used with standard or async select types. diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx index 08bc03fb8b4e..76a162e7c589 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx @@ -109,7 +109,9 @@ const HorizontalOverflowFilterControlContainer = styled( } `; -const VerticalFormItem = styled(StyledFormItem)` +const VerticalFormItem = styled(StyledFormItem)<{ + inverseSelection: boolean; +}>` .ant-form-item-label { overflow: visible; label.ant-form-item-required:not(.ant-form-item-required-mark-optional) { @@ -118,9 +120,19 @@ const VerticalFormItem = styled(StyledFormItem)` } } } + + .select-container { + ${({ inverseSelection }) => + inverseSelection && + ` + width: 140px; + `} + } `; -const HorizontalFormItem = styled(StyledFormItem)` +const HorizontalFormItem = styled(StyledFormItem)<{ + inverseSelection: boolean; +}>` && { margin-bottom: 0; align-items: center; @@ -142,7 +154,15 @@ const HorizontalFormItem = styled(StyledFormItem)` } .ant-form-item-control { - width: ${({ theme }) => theme.gridUnit * 41}px; + width: ${({ inverseSelection }) => (inverseSelection ? 252 : 164)}px; + } + + .select-container { + ${({ inverseSelection, theme }) => + inverseSelection && + ` + width: 164px; + `} } `; @@ -151,31 +171,41 @@ const HorizontalOverflowFormItem = VerticalFormItem; const useFilterControlDisplay = ( orientation: FilterBarOrientation, overflow: boolean, + inverseSelection: boolean, ) => useMemo(() => { if (orientation === FilterBarOrientation.Horizontal) { if (overflow) { return { FilterControlContainer: HorizontalOverflowFilterControlContainer, - FormItem: HorizontalOverflowFormItem, + FormItem: (props: any) => ( + + ), FilterControlTitleBox: HorizontalOverflowFilterControlTitleBox, FilterControlTitle: HorizontalOverflowFilterControlTitle, }; } return { FilterControlContainer: HorizontalFilterControlContainer, - FormItem: HorizontalFormItem, + FormItem: (props: any) => ( + + ), FilterControlTitleBox: HorizontalFilterControlTitleBox, FilterControlTitle: HorizontalFilterControlTitle, }; } return { FilterControlContainer: VerticalFilterControlContainer, - FormItem: VerticalFormItem, + FormItem: (props: any) => ( + + ), FilterControlTitleBox: VerticalFilterControlTitleBox, FilterControlTitle: VerticalFilterControlTitle, }; - }, [orientation, overflow]); + }, [orientation, overflow, inverseSelection]); const ToolTipContainer = styled.div` font-size: ${({ theme }) => theme.typography.sizes.m}px; @@ -243,13 +273,14 @@ const FilterControl = ({ checkIsMissingRequiredValue(filter, filter.dataMask?.filterState); const validateStatus = isMissingRequiredValue ? 'error' : undefined; const isRequired = !!filter.controlValues?.enableEmptyFilter; + const inverseSelection = !!filter.controlValues?.inverseSelection; const { FilterControlContainer, FormItem, FilterControlTitleBox, FilterControlTitle, - } = useFilterControlDisplay(orientation, overflow); + } = useFilterControlDisplay(orientation, overflow, inverseSelection); const label = useMemo( () => ( diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx index ec6b33d95b01..cb1f2de7cc4a 100644 --- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx +++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx @@ -47,8 +47,10 @@ const selectMultipleProps = { urlParams: {}, vizType: 'filter_select', inputRef: { current: null }, + nativeFilterId: 'test-filter', }, height: 20, + width: 220, hooks: {}, ownState: {}, filterState: { value: ['boy'] }, @@ -62,7 +64,6 @@ const selectMultipleProps = { rejected_filters: [], }, ], - width: 220, behaviors: ['NATIVE_FILTER'], isRefreshing: false, appSection: AppSection.Dashboard, @@ -80,7 +81,38 @@ describe('SelectFilterPlugin', () => { formData: { ...selectMultipleProps.formData, ...props }, })} setDataMask={setDataMask} + showOverflow={false} />, + { + useRedux: true, + initialState: { + nativeFilters: { + filters: { + 'test-filter': { + name: 'Test Filter', + }, + }, + }, + dataMask: { + 'test-filter': { + extraFormData: { + filters: [ + { + col: 'gender', + op: 'IN', + val: ['boy'], + }, + ], + }, + filterState: { + value: ['boy'], + label: 'boy', + excludeFilterValues: true, + }, + }, + }, + }, + }, ); beforeEach(() => { @@ -102,9 +134,12 @@ describe('SelectFilterPlugin', () => { filterState: { label: 'boy', value: ['boy'], + excludeFilterValues: true, }, }); - userEvent.click(screen.getByRole('combobox')); + + const filterSelect = screen.getAllByRole('combobox')[0]; + userEvent.click(filterSelect); userEvent.click(screen.getByTitle('girl')); expect(await screen.findByTitle(/girl/i)).toBeInTheDocument(); expect(setDataMask).toHaveBeenCalledWith({ @@ -120,6 +155,7 @@ describe('SelectFilterPlugin', () => { filterState: { label: 'boy, girl', value: ['boy', 'girl'], + excludeFilterValues: true, }, }); }); @@ -145,6 +181,7 @@ describe('SelectFilterPlugin', () => { filterState: { label: undefined, value: null, + excludeFilterValues: true, }, }); }); @@ -162,13 +199,18 @@ describe('SelectFilterPlugin', () => { filterState: { label: undefined, value: null, + excludeFilterValues: true, }, }); }); test('Select single values with inverse', async () => { getWrapper({ multiSelect: false, inverseSelection: true }); - userEvent.click(screen.getByRole('combobox')); + + // Get the main filter select (second combobox) + const filterSelect = screen.getAllByRole('combobox')[1]; + userEvent.click(filterSelect); + expect(await screen.findByTitle('girl')).toBeInTheDocument(); userEvent.click(screen.getByTitle('girl')); expect(setDataMask).toHaveBeenCalledWith({ @@ -184,13 +226,15 @@ describe('SelectFilterPlugin', () => { filterState: { label: 'girl (excluded)', value: ['girl'], + excludeFilterValues: true, }, }); }); test('Select single null (empty) value', async () => { getWrapper(); - userEvent.click(screen.getByRole('combobox')); + const filterSelect = screen.getAllByRole('combobox')[0]; + userEvent.click(filterSelect); expect(await screen.findByRole('combobox')).toBeInTheDocument(); userEvent.click(screen.getByTitle(NULL_STRING)); expect(setDataMask).toHaveBeenLastCalledWith({ @@ -206,13 +250,15 @@ describe('SelectFilterPlugin', () => { filterState: { label: `boy, ${NULL_STRING}`, value: ['boy', null], + excludeFilterValues: true, }, }); }); test('receives the correct filter when search all options', async () => { getWrapper({ searchAllOptions: true, multiSelect: false }); - userEvent.click(screen.getByRole('combobox')); + const filterSelect = screen.getAllByRole('combobox')[0]; + userEvent.click(filterSelect); expect(await screen.findByRole('combobox')).toBeInTheDocument(); userEvent.click(screen.getByTitle('girl')); expect(setDataMask).toHaveBeenLastCalledWith( @@ -229,14 +275,14 @@ describe('SelectFilterPlugin', () => { }), ); }); + test('number of fired queries when searching', async () => { getWrapper({ searchAllOptions: 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'), 'a'); - // Closes the select userEvent.tab(); - // One call for the search term and other for the empty search expect(setDataMask).toHaveBeenCalledTimes(2); }); @@ -253,21 +299,77 @@ 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(); }); + 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'); expect(screen.queryByTitle('new value')).not.toBeInTheDocument(); }); - test('Should allow for new values when creatable is false', async () => { + test('Should allow for new values when creatable is true', async () => { getWrapper({ creatable: true }); userEvent.type(screen.getByRole('combobox'), 'new value'); expect(await screen.findByTitle('new value')).toBeInTheDocument(); diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx index ca8fbda8df7d..d083cd515026 100644 --- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx @@ -17,7 +17,7 @@ * under the License. */ /* eslint-disable no-param-reassign */ -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useState, useRef } from 'react'; import { AppSection, DataMask, @@ -29,25 +29,32 @@ import { finestTemporalGrainFormatter, t, tn, + styled, } from '@superset-ui/core'; // eslint-disable-next-line no-restricted-imports import { LabeledValue as AntdLabeledValue } from 'antd/lib/select'; // TODO: Remove antd -import { debounce } from 'lodash'; +import { debounce, isUndefined } from 'lodash'; import { useImmerReducer } from 'use-immer'; import { Select } from 'src/components'; +// eslint-disable-next-line no-restricted-imports +import { Space } from 'antd'; // Import Space directly from antd 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, StyledFormItem } from '../common'; import { getDataRecordFormatter, getSelectExtraFormData } from '../../utils'; +import { FilterPluginStyle, StatusMessage, StyledFormItem } from '../common'; +import { PluginFilterSelectProps, SelectValue } from './types'; type DataMaskAction = | { type: 'ownState'; ownState: JsonObject } | { type: 'filterState'; extraFormData: ExtraFormData; - filterState: { value: SelectValue; label?: string }; + filterState: { + value: SelectValue; + label?: string; + excludeFilterValues?: boolean; + }; }; function reducer(draft: DataMask, action: DataMaskAction) { @@ -77,6 +84,24 @@ function reducer(draft: DataMask, action: DataMaskAction) { } } +const StyledSpace = styled(Space)<{ $inverseSelection: boolean }>` + display: flex; + align-items: center; + width: 100%; + + .exclude-select { + width: 80px; + flex-shrink: 0; + } + + &.ant-space { + .ant-space-item { + width: ${({ $inverseSelection }) => + !$inverseSelection ? '100%' : 'auto'}; + } + } +`; + export default function PluginFilterSelect(props: PluginFilterSelectProps) { const { coltypeMap, @@ -107,6 +132,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { defaultToFirstItem, searchAllOptions, } = formData; + const groupby = useMemo( () => ensureIsArray(formData.groupby).map(getColumnLabel), [formData.groupby], @@ -126,6 +152,13 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { }), [data, col], ); + const [excludeFilterValues, setExcludeFilterValues] = useState( + isUndefined(filterState?.excludeFilterValues) + ? true + : filterState?.excludeFilterValues, + ); + + const prevExcludeFilterValues = useRef(excludeFilterValues); const updateDataMask = useCallback( (values: SelectValue) => { @@ -139,7 +172,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { col, values, emptyFilter, - inverseSelection, + excludeFilterValues && inverseSelection, ), filterState: { ...filterState, @@ -152,6 +185,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { appSection === AppSection.FilterConfigModal && defaultToFirstItem ? undefined : values, + excludeFilterValues, }, }); }, @@ -164,6 +198,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { dispatchDataMask, enableEmptyFilter, inverseSelection, + excludeFilterValues, JSON.stringify(filterState), labelFormatter, ], @@ -278,6 +313,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { defaultToFirstItem, enableEmptyFilter, inverseSelection, + excludeFilterValues, updateDataMask, data, groupby, @@ -288,45 +324,85 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { setDataMask(dataMask); }, [JSON.stringify(dataMask)]); + useEffect(() => { + if (prevExcludeFilterValues.current !== excludeFilterValues) { + dispatchDataMask({ + type: 'filterState', + extraFormData: getSelectExtraFormData( + col, + filterState.value, + !filterState.value?.length, + excludeFilterValues && inverseSelection, + ), + filterState: { + ...(filterState as { + value: SelectValue; + label?: string; + excludeFilterValues?: boolean; + }), + excludeFilterValues, + }, + }); + prevExcludeFilterValues.current = excludeFilterValues; + } + }, [excludeFilterValues]); + + const handleExclusionToggle = (value: string) => { + setExcludeFilterValues(value === 'true'); + }; + return ( - + )} +