From 54b56fe12f18c034b301289cecb935342c0e3f5a Mon Sep 17 00:00:00 2001 From: Matt Houston <33639770+mhoustonataegis@users.noreply.github.com> Date: Wed, 1 Dec 2021 17:28:11 -0600 Subject: [PATCH] feat: Add single select and inverse selection to numeric range (#16722) (#17372) * feat: add single select and inverse selection to numeric range (#16722) * Ignore invalid eslint errors regarding conditionally called hooks. * Add license header to new file. * Flipped the numerical range values for the minimum slider so that the highlighted range value accurately reflects the applied filter. * Resolved linting errors * Remove unnecessary important flag from css --- .../FiltersConfigForm/FiltersConfigForm.tsx | 73 +++++++- .../getControlItemsMap.test.tsx | 10 ++ .../FiltersConfigForm/getControlItemsMap.tsx | 3 +- .../Range/RangeFilterPlugin.test.tsx | 58 +++++++ .../components/Range/RangeFilterPlugin.tsx | 162 ++++++++++++++++-- .../components/Range/SingleValueType.ts | 24 +++ .../filters/components/Range/controlPanel.ts | 11 ++ superset-frontend/src/filters/utils.test.ts | 7 +- superset-frontend/src/filters/utils.ts | 13 +- 9 files changed, 332 insertions(+), 29 deletions(-) create mode 100644 superset-frontend/src/filters/components/Range/SingleValueType.ts diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx index ffd64d3746950..37ae2ca8788ba 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +/* eslint-disable react-hooks/rules-of-hooks */ import { ColumnMeta, InfoTooltipWithTrigger, @@ -35,7 +36,7 @@ import { t, } from '@superset-ui/core'; import { FormInstance } from 'antd/lib/form'; -import { isEmpty, isEqual } from 'lodash'; +import { isEqual } from 'lodash'; import React, { forwardRef, useCallback, @@ -73,6 +74,7 @@ import { Filter, NativeFilterType, } from 'src/dashboard/components/nativeFilters/types'; +import { SingleValueType } from 'src/filters/components/Range/SingleValueType'; import { getFormData } from 'src/dashboard/components/nativeFilters/utils'; import { CASCADING_FILTERS, @@ -543,6 +545,15 @@ const FiltersConfigForm = ( !!filterToEdit?.adhoc_filters?.length || !!filterToEdit?.time_range; + const hasEnableSingleValue = + formFilter?.controlValues?.enableSingleValue !== undefined || + filterToEdit?.controlValues?.enableSingleValue !== undefined; + + let enableSingleValue = filterToEdit?.controlValues?.enableSingleValue; + if (formFilter?.controlValues?.enableSingleMaxValue !== undefined) { + ({ enableSingleValue } = formFilter.controlValues); + } + const hasSorting = typeof formFilter?.controlValues?.sortAscending === 'boolean' || typeof filterToEdit?.controlValues?.sortAscending === 'boolean'; @@ -568,6 +579,17 @@ const FiltersConfigForm = ( forceUpdate(); }; + const onEnableSingleValueChanged = (value: SingleValueType | undefined) => { + const previous = form.getFieldValue('filters')?.[filterId].controlValues; + setNativeFilterFieldValues(form, filterId, { + controlValues: { + ...previous, + enableSingleValue: value, + }, + }); + forceUpdate(); + }; + const validatePreFilter = () => setTimeout( () => @@ -669,12 +691,13 @@ const FiltersConfigForm = ( ]); useEffect(() => { - // Run only once when the control items are available - if (isActive && !isEmpty(controlItems)) { + // Run only once + if (isActive) { const hasCheckedAdvancedControl = hasParentFilter || hasPreFilter || hasSorting || + hasEnableSingleValue || Object.keys(controlItems) .filter(key => !BASIC_CONTROL_ITEMS.includes(key)) .some(key => controlItems[key].checked); @@ -1137,7 +1160,7 @@ const FiltersConfigForm = ( )} - {formFilter?.filterType !== 'filter_range' && ( + {formFilter?.filterType !== 'filter_range' ? ( + ) : ( + + { + onEnableSingleValueChanged( + checked ? SingleValueType.Exact : undefined, + ); + formChanged(); + }} + > + {t('Single value type')} + } + > + + onEnableSingleValueChanged(value.target.value) + } + > + + {t('Minimum')} + + + {t('Exact')} + + + {t('Maximum')} + + + + + )} )} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx index f87a43467502f..25ed9ea3b793b 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx @@ -125,6 +125,16 @@ test('Should render null empty when "getControlItems" return []', () => { expect(container.children).toHaveLength(0); }); +test('Should render null empty when "getControlItems" return enableSingleValue', () => { + const props = createProps(); + (getControlItems as jest.Mock).mockReturnValue([ + { name: 'enableSingleValue', config: { renderTrigger: true } }, + ]); + const controlItemsMap = getControlItemsMap(props); + const { container } = renderControlItems(controlItemsMap); + expect(container.children).toHaveLength(0); +}); + test('Should render null empty when "controlItems" are falsy', () => { const props = createProps(); const controlItems = [null, false, {}, { config: { renderTrigger: false } }]; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx index ce8978abaa1ca..d202552f3903e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx @@ -145,7 +145,8 @@ export default function getControlItemsMap({ .filter( (controlItem: CustomControlItem) => controlItem?.config?.renderTrigger && - controlItem.name !== 'sortAscending', + controlItem.name !== 'sortAscending' && + controlItem.name !== 'enableSingleValue', ) .forEach(controlItem => { const initialValue = diff --git a/superset-frontend/src/filters/components/Range/RangeFilterPlugin.test.tsx b/superset-frontend/src/filters/components/Range/RangeFilterPlugin.test.tsx index cf9420ecffed5..7dac323d6b137 100644 --- a/superset-frontend/src/filters/components/Range/RangeFilterPlugin.test.tsx +++ b/superset-frontend/src/filters/components/Range/RangeFilterPlugin.test.tsx @@ -20,6 +20,7 @@ import { AppSection, GenericDataType } from '@superset-ui/core'; import React from 'react'; import { render } from 'spec/helpers/testing-library'; import RangeFilterPlugin from './RangeFilterPlugin'; +import { SingleValueType } from './SingleValueType'; import transformProps from './transformProps'; const rangeProps = { @@ -118,4 +119,61 @@ describe('RangeFilterPlugin', () => { }, }); }); + + it('should call setDataMask with correct greater than filter', () => { + getWrapper({ enableSingleValue: SingleValueType.Minimum }); + expect(setDataMask).toHaveBeenCalledWith({ + extraFormData: { + filters: [ + { + col: 'SP_POP_TOTL', + op: '>=', + val: 70, + }, + ], + }, + filterState: { + label: 'x ≥ 70', + value: [70, 100], + }, + }); + }); + + it('should call setDataMask with correct less than filter', () => { + getWrapper({ enableSingleValue: SingleValueType.Maximum }); + expect(setDataMask).toHaveBeenCalledWith({ + extraFormData: { + filters: [ + { + col: 'SP_POP_TOTL', + op: '<=', + val: 70, + }, + ], + }, + filterState: { + label: 'x ≤ 70', + value: [10, 70], + }, + }); + }); + + it('should call setDataMask with correct exact filter', () => { + getWrapper({ enableSingleValue: SingleValueType.Exact }); + expect(setDataMask).toHaveBeenCalledWith({ + extraFormData: { + filters: [ + { + col: 'SP_POP_TOTL', + op: '==', + val: 10, + }, + ], + }, + filterState: { + label: 'x = 10', + value: [10, 10], + }, + }); + }); }); diff --git a/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx b/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx index 3441cae8bb59d..b768511baa31d 100644 --- a/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx @@ -30,6 +30,40 @@ import { rgba } from 'emotion-rgba'; import { PluginFilterRangeProps } from './types'; import { StatusMessage, StyledFormItem, FilterPluginStyle } from '../common'; import { getRangeExtraFormData } from '../../utils'; +import { SingleValueType } from './SingleValueType'; + +const LIGHT_BLUE = '#99e7f0'; +const DARK_BLUE = '#6dd3e3'; +const LIGHT_GRAY = '#f5f5f5'; +const DARK_GRAY = '#e1e1e1'; + +const StyledMinSlider = styled(Slider)<{ + validateStatus?: 'error' | 'warning' | 'info'; +}>` + ${({ theme, validateStatus }) => ` + .ant-slider-rail { + background-color: ${ + validateStatus ? theme.colors[validateStatus]?.light1 : LIGHT_BLUE + }; + } + + .ant-slider-track { + background-color: ${LIGHT_GRAY}; + } + + &:hover { + .ant-slider-rail { + background-color: ${ + validateStatus ? theme.colors[validateStatus]?.base : DARK_BLUE + }; + } + + .ant-slider-track { + background-color: ${DARK_GRAY}; + } + } + `} +`; const Wrapper = styled.div<{ validateStatus?: 'error' | 'warning' | 'info' }>` ${({ theme, validateStatus }) => ` @@ -80,6 +114,9 @@ const numberFormatter = getNumberFormatter(NumberFormats.SMART_NUMBER); const tipFormatter = (value: number) => numberFormatter(value); const getLabel = (lower: number | null, upper: number | null): string => { + if (lower !== null && upper !== null && lower === upper) { + return `x = ${numberFormatter(lower)}`; + } if (lower !== null && upper !== null) { return `${numberFormatter(lower)} ≤ x ≤ ${numberFormatter(upper)}`; } @@ -120,24 +157,38 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) { const [row] = data; // @ts-ignore const { min, max }: { min: number; max: number } = row; - const { groupby, defaultValue, inputRef } = formData; + const { groupby, defaultValue, inputRef, enableSingleValue } = formData; + + const enableSingleMinValue = enableSingleValue === SingleValueType.Minimum; + const enableSingleMaxValue = enableSingleValue === SingleValueType.Maximum; + const enableSingleExactValue = enableSingleValue === SingleValueType.Exact; + const rangeValue = enableSingleValue === undefined; + const [col = ''] = ensureIsArray(groupby).map(getColumnLabel); const [value, setValue] = useState<[number, number]>( - defaultValue ?? [min, max], + defaultValue ?? [min, enableSingleExactValue ? min : max], ); const [marks, setMarks] = useState<{ [key: number]: string }>({}); + const minIndex = 0; + const maxIndex = 1; + const minMax = value ?? [min, max]; const getBounds = useCallback( ( value: [number, number], ): { lower: number | null; upper: number | null } => { const [lowerRaw, upperRaw] = value; + + if (enableSingleExactValue) { + return { lower: lowerRaw, upper: upperRaw }; + } + return { lower: lowerRaw > min ? lowerRaw : null, upper: upperRaw < max ? upperRaw : null, }; }, - [max, min], + [max, min, enableSingleExactValue], ); const handleAfterChange = useCallback( @@ -166,8 +217,34 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) { if (row?.min === undefined && row?.max === undefined) { return; } - handleAfterChange(filterState.value ?? [min, max]); - }, [JSON.stringify(filterState.value), JSON.stringify(data)]); + + let filterStateValue = filterState.value ?? [min, max]; + if (enableSingleMaxValue) { + const filterStateMax = + filterStateValue[maxIndex] <= minMax[maxIndex] + ? filterStateValue[maxIndex] + : minMax[maxIndex]; + + filterStateValue = [min, filterStateMax]; + } else if (enableSingleMinValue) { + const filterStateMin = + filterStateValue[minIndex] >= minMax[minIndex] + ? filterStateValue[minIndex] + : minMax[minIndex]; + + filterStateValue = [filterStateMin, max]; + } else if (enableSingleExactValue) { + filterStateValue = [minMax[minIndex], minMax[minIndex]]; + } + + handleAfterChange(filterStateValue); + }, [ + enableSingleMaxValue, + enableSingleMinValue, + enableSingleExactValue, + JSON.stringify(filterState.value), + JSON.stringify(data), + ]); const formItemExtra = useMemo(() => { if (filterState.validateMessage) { @@ -180,7 +257,23 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) { return undefined; }, [filterState.validateMessage, filterState.validateStatus]); - const minMax = useMemo(() => value ?? [min, max], [max, min, value]); + useEffect(() => { + if (enableSingleMaxValue) { + handleAfterChange([min, minMax[minIndex]]); + } + }, [enableSingleMaxValue]); + + useEffect(() => { + if (enableSingleMinValue) { + handleAfterChange([minMax[maxIndex], max]); + } + }, [enableSingleMinValue]); + + useEffect(() => { + if (enableSingleExactValue) { + handleAfterChange([minMax[minIndex], minMax[minIndex]]); + } + }, [enableSingleExactValue]); return ( @@ -197,16 +290,53 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) { onMouseEnter={setFocusedFilter} onMouseLeave={unsetFocusedFilter} > - + {enableSingleMaxValue && ( + handleAfterChange([min, value])} + onChange={value => handleChange([min, value])} + /> + )} + {enableSingleMinValue && ( + handleAfterChange([value, max])} + onChange={value => handleChange([value, max])} + /> + )} + {enableSingleExactValue && ( + handleAfterChange([value, value])} + onChange={value => handleChange([value, value])} + /> + )} + {rangeValue && ( + + )} )} diff --git a/superset-frontend/src/filters/components/Range/SingleValueType.ts b/superset-frontend/src/filters/components/Range/SingleValueType.ts new file mode 100644 index 0000000000000..cdf88320e5b12 --- /dev/null +++ b/superset-frontend/src/filters/components/Range/SingleValueType.ts @@ -0,0 +1,24 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export enum SingleValueType { + Minimum, + Exact, + Maximum, +} diff --git a/superset-frontend/src/filters/components/Range/controlPanel.ts b/superset-frontend/src/filters/components/Range/controlPanel.ts index 555b019210e85..c5c04b7e74bce 100644 --- a/superset-frontend/src/filters/components/Range/controlPanel.ts +++ b/superset-frontend/src/filters/components/Range/controlPanel.ts @@ -22,6 +22,7 @@ import { sections, sharedControls, } from '@superset-ui/chart-controls'; +import { SingleValueType } from './SingleValueType'; const config: ControlPanelConfig = { controlPanelSections: [ @@ -57,6 +58,16 @@ const config: ControlPanelConfig = { description: t('User must select a value for this filter.'), }, }, + { + name: 'enableSingleValue', + config: { + type: 'CheckboxControl', + label: t('Single value'), + default: SingleValueType.Exact, + renderTrigger: true, + description: t('Use only a single value.'), + }, + }, ], ], }, diff --git a/superset-frontend/src/filters/utils.test.ts b/superset-frontend/src/filters/utils.test.ts index 9d225f4c2fd11..aeba5d32defb3 100644 --- a/superset-frontend/src/filters/utils.test.ts +++ b/superset-frontend/src/filters/utils.test.ts @@ -54,12 +54,7 @@ describe('Filter utils', () => { filters: [ { col: 'testCol', - op: '>=', - val: 0, - }, - { - col: 'testCol', - op: '<=', + op: '==', val: 0, }, ], diff --git a/superset-frontend/src/filters/utils.ts b/superset-frontend/src/filters/utils.ts index 1ab401b11dd2a..4908f1a2893c5 100644 --- a/superset-frontend/src/filters/utils.ts +++ b/superset-frontend/src/filters/utils.ts @@ -60,12 +60,21 @@ export const getRangeExtraFormData = ( upper?: number | null, ) => { const filters: QueryObjectFilterClause[] = []; - if (lower !== undefined && lower !== null) { + if (lower !== undefined && lower !== null && lower !== upper) { filters.push({ col, op: '>=', val: lower }); } - if (upper !== undefined && upper !== null) { + if (upper !== undefined && upper !== null && upper !== lower) { filters.push({ col, op: '<=', val: upper }); } + if ( + upper !== undefined && + upper !== null && + lower !== undefined && + lower !== null && + upper === lower + ) { + filters.push({ col, op: '==', val: upper }); + } return filters.length ? {