fix(RangeFilterPlugin): Manage state correctly

superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.noInitState.test.ts

@@ -192,23 +192,34 @@ describe('Native filters', () => {
         testItems.filterNumericalColumn,
       );
       saveNativeFilterSettings([]);
-      // assertions
-      cy.get(nativeFilters.slider.slider).should('be.visible').click('center');
+
+      // Assertions
+      cy.get('[data-test="native-filter-from-input"]')
+        .should('be.visible')
+        .click();
+
+      cy.get('[data-test="native-filter-from-input"]').type('{selectall}5');
+
+      cy.get('[data-test="native-filter-to-input"]')
+        .should('be.visible')
+        .click();
+
+      cy.get('[data-test="native-filter-to-input"]').type('{selectall}50');
       cy.get(nativeFilters.applyFilter).click();
-      // assert that the url contains 'native_filters' in the url
+
+      // Assert that the URL contains 'native_filters'
       cy.url().then(u => {
         const ur = new URL(u);
         expect(ur.search).to.include('native_filters');
-        // assert that the start handle has a value
-        cy.get(nativeFilters.slider.startHandle)
-          .invoke('attr', 'aria-valuenow')
-          .should('exist');
-        // assert that the end handle has a value
-        cy.get(nativeFilters.slider.endHandle)
-          .invoke('attr', 'aria-valuenow')
-          .should('exist');
-        // assert slider text matches what we should have
-        cy.get(nativeFilters.slider.sliderText).should('have.text', '49');
+
+        cy.get('[data-test="native-filter-from-input"]')
+          .invoke('val')
+          .should('equal', '5');
+
+        // Assert that the "To" input has the correct value
+        cy.get('[data-test="native-filter-to-input"]')
+          .invoke('val')
+          .should('equal', '50');
       });
     });
 

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx

@@ -1249,7 +1249,10 @@ const FiltersConfigForm = (
                     rules={[
                       {
                         validator: () => {
-                          if (formFilter?.defaultDataMask?.filterState?.value) {
+                          if (
+                            formFilter?.defaultDataMask?.filterState?.value !==
+                            undefined
+                          ) {
                             // requires managing the error as the DefaultValue
                             // component does not use an Antdesign compatible input
                             const formValidationFields = form.getFieldsError();

superset-frontend/src/filters/components/Range/RangeFilterPlugin.test.tsx

@@ -17,7 +17,7 @@
  * under the License.
  */
 import { AppSection, GenericDataType } from '@superset-ui/core';
-import { render, screen } from 'spec/helpers/testing-library';
+import { fireEvent, render, screen } from 'spec/helpers/testing-library';
 import RangeFilterPlugin from './RangeFilterPlugin';
 import { SingleValueType } from './SingleValueType';
 import transformProps from './transformProps';
@@ -100,6 +100,44 @@ describe('RangeFilterPlugin', () => {
     jest.clearAllMocks();
   });
 
+  it('should render two numerical inputs', () => {
+    getWrapper();
+
+    const inputs = screen.getAllByRole('spinbutton');
+    expect(inputs).toHaveLength(2);
+
+    expect(inputs[0]).toHaveValue('10');
+    expect(inputs[1]).toHaveValue('70');
+  });
+
+  it('should set the "To" input to be equal to "From" when a lower value is entered', () => {
+    getWrapper();
+
+    const inputs = screen.getAllByRole('spinbutton');
+    const fromInput = inputs[0];
+    const toInput = inputs[1];
+
+    fireEvent.change(fromInput, { target: { value: 20 } });
+
+    fireEvent.change(toInput, { target: { value: 10 } });
+
+    expect(toInput).toHaveValue('20');
+  });
+
+  it('should set the "From" input to be equal to "To" when a higher value is entered', () => {
+    getWrapper();
+
+    const inputs = screen.getAllByRole('spinbutton');
+    const fromInput = inputs[0];
+    const toInput = inputs[1];
+
+    fireEvent.change(toInput, { target: { value: 30 } });
+
+    fireEvent.change(fromInput, { target: { value: 40 } });
+
+    expect(fromInput).toHaveValue('30');
+  });
+
   it('should call setDataMask with correct filter', () => {
     getWrapper();
     expect(setDataMask).toHaveBeenCalledWith({
@@ -139,7 +177,7 @@ describe('RangeFilterPlugin', () => {
         value: [20, 100],
       },
     });
-    expect(screen.getByRole('slider')).toHaveAttribute('aria-valuenow', '20');
+    expect(screen.getAllByRole('spinbutton')[0]).toHaveValue('20');
   });
 
   it('should call setDataMask with correct less than filter', () => {
@@ -162,7 +200,7 @@ describe('RangeFilterPlugin', () => {
         value: [10, 60],
       },
     });
-    expect(screen.getByRole('slider')).toHaveAttribute('aria-valuenow', '60');
+    expect(screen.getAllByRole('spinbutton')[1]).toHaveValue('60');
   });
 
   it('should call setDataMask with correct exact filter', () => {

superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx

@@ -25,45 +25,19 @@ import {
   t,
 } from '@superset-ui/core';
 import { useCallback, useEffect, useMemo, useState } from 'react';
-import { rgba } from 'emotion-rgba';
-import { AntdSlider } from 'src/components';
+import { InputNumber } from 'src/components/Input';
 import { FilterBarOrientation } from 'src/dashboard/types';
 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(AntdSlider)<{
-  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 StyledDivider = styled.span`
+  margin: 0 10px;
+  color: ${({ theme }) => theme.colors.grayscale.light1};
+  font-weight: ${({ theme }) => theme.typography.weights.bold};
+  font-size: ${({ theme }) => theme.typography.sizes.m}px;
+  align-content: center;
 `;
 
 const Wrapper = styled.div<{
@@ -71,67 +45,12 @@ const Wrapper = styled.div<{
   orientation?: FilterBarOrientation;
   isOverflowing?: boolean;
 }>`
-  ${({ theme, validateStatus, orientation, isOverflowing }) => `
-    border: 1px solid transparent;
-    &:focus {
-      border: 1px solid
-        ${theme.colors[validateStatus || 'primary']?.base};
-      outline: 0;
-      box-shadow: 0 0 0 3px
-        ${rgba(theme.colors[validateStatus || 'primary']?.base, 0.2)};
-    }
-    & .ant-slider {
-      margin-top: ${
-        orientation === FilterBarOrientation.Horizontal ? 0 : theme.gridUnit
-      }px;
-      margin-bottom: ${
-        orientation === FilterBarOrientation.Horizontal ? 0 : theme.gridUnit * 5
-      }px;
-
-      ${
-        orientation === FilterBarOrientation.Horizontal &&
-        !isOverflowing &&
-        `line-height: 1.2;`
-      }
-
-      & .ant-slider-track {
-        background-color: ${
-          validateStatus && theme.colors[validateStatus]?.light1
-        };
-      }
-      & .ant-slider-handle {
-        border: ${
-          validateStatus && `2px solid ${theme.colors[validateStatus]?.light1}`
-        };
-        &:focus {
-          box-shadow: 0 0 0 3px
-            ${rgba(theme.colors[validateStatus || 'primary']?.base, 0.2)};
-        }
-      }
-      & .ant-slider-mark {
-        font-size: ${theme.typography.sizes.s}px;
-      }
-
-      &:hover {
-        & .ant-slider-track {
-          background-color: ${
-            validateStatus && theme.colors[validateStatus]?.base
-          };
-        }
-        & .ant-slider-handle {
-          border: ${
-            validateStatus && `2px solid ${theme.colors[validateStatus]?.base}`
-          };
-        }
-      }
-    }
-  `}
+  display: flex;
+  justify-content: space-between;
 `;
 
 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)}`;
@@ -148,20 +67,6 @@ const getLabel = (lower: number | null, upper: number | null): string => {
   return '';
 };
 
-const getMarks = (
-  lower: number | null,
-  upper: number | null,
-): { [key: number]: string } => {
-  const newMarks: { [key: number]: string } = {};
-  if (lower !== null) {
-    newMarks[lower] = numberFormatter(lower);
-  }
-  if (upper !== null) {
-    newMarks[upper] = numberFormatter(upper);
-  }
-  return newMarks;
-};
-
 export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
   const {
     data,
@@ -187,13 +92,12 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
   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, enableSingleExactValue ? min : max],
   );
-  const [marks, setMarks] = useState<{ [key: number]: string }>({});
+
   const minIndex = 0;
   const maxIndex = 1;
   const minMax = value ?? [min, max];
@@ -218,10 +122,7 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
 
   const handleAfterChange = useCallback(
     (value: [number, number]): void => {
-      setValue(value);
       const { lower, upper } = getBounds(value);
-      setMarks(getMarks(lower, upper));
-
       setDataMask({
         extraFormData: getRangeExtraFormData(col, lower, upper),
         filterState: {
@@ -233,9 +134,35 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
     [col, getBounds, setDataMask],
   );
 
-  const handleChange = useCallback((value: [number, number]) => {
-    setValue(value);
-  }, []);
+  const handleChange = (newValue: number, index: 0 | 1) => {
+    const updatedValue: [number, number] = [...value];
+
+    if (enableSingleExactValue) {
+      setValue([newValue, newValue]);
+      return;
+    }
+
+    if (enableSingleMinValue && index === minIndex) {
+      updatedValue[minIndex] = Math.min(newValue, updatedValue[maxIndex]);
+      setValue(updatedValue);
+      return;
+    }
+
+    if (enableSingleMaxValue && index === maxIndex) {
+      updatedValue[maxIndex] = Math.max(newValue, updatedValue[minIndex]);
+      setValue(updatedValue);
+      return;
+    }
+
+    if (index === minIndex && newValue > updatedValue[maxIndex]) {
+      updatedValue[minIndex] = updatedValue[maxIndex];
+    } else if (index === maxIndex && newValue < updatedValue[minIndex]) {
+      updatedValue[maxIndex] = updatedValue[minIndex];
+    } else {
+      updatedValue[index] = newValue;
+    }
+    setValue(updatedValue);
+  };
 
   useEffect(() => {
     // when switch filter type and queriesData still not updated we need ignore this case (in FilterBar)
@@ -261,7 +188,7 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
     } else if (enableSingleExactValue) {
       filterStateValue = [minMax[minIndex], minMax[minIndex]];
     }
-
+    setValue(value);
     handleAfterChange(filterStateValue);
   }, [
     enableSingleMaxValue,
@@ -300,16 +227,6 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
     }
   }, [enableSingleExactValue]);
 
-  const MIN_NUM_STEPS = 20;
-  const stepHeuristic = (min: number, max: number) => {
-    const maxStepSize = (max - min) / MIN_NUM_STEPS;
-    // normalizedStepSize: .06 -> .01, .003 -> .001
-    const normalizedStepSize = `1E${Math.floor(Math.log10(maxStepSize))}`;
-    return Math.min(1, parseFloat(normalizedStepSize));
-  };
-
-  const step = max - min <= 1 ? stepHeuristic(min, max) : 1;
-
   return (
     <FilterPluginStyle height={height} width={width}>
       {Number.isNaN(Number(min)) || Number.isNaN(Number(max)) ? (
@@ -332,57 +249,23 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
             onMouseDown={() => setFilterActive(true)}
             onMouseUp={() => setFilterActive(false)}
           >
-            {enableSingleMaxValue && (
-              <AntdSlider
-                min={min}
-                max={max}
-                step={step}
-                value={minMax[maxIndex]}
-                tipFormatter={tipFormatter}
-                marks={marks}
-                onAfterChange={value => handleAfterChange([min, value])}
-                onChange={value => handleChange([min, value])}
-              />
-            )}
-            {enableSingleMinValue && (
-              <StyledMinSlider
-                validateStatus={filterState.validateStatus}
-                min={min}
-                max={max}
-                step={step}
-                value={minMax[minIndex]}
-                tipFormatter={tipFormatter}
-                marks={marks}
-                onAfterChange={value => handleAfterChange([value, max])}
-                onChange={value => handleChange([value, max])}
-              />
-            )}
-            {enableSingleExactValue && (
-              <AntdSlider
-                min={min}
-                max={max}
-                step={step}
-                included={false}
-                value={minMax[minIndex]}
-                tipFormatter={tipFormatter}
-                marks={marks}
-                onAfterChange={value => handleAfterChange([value, value])}
-                onChange={value => handleChange([value, value])}
-              />
-            )}
-            {rangeValue && (
-              <AntdSlider
-                range
-                min={min}
-                max={max}
-                step={step}
-                value={minMax}
-                onAfterChange={handleAfterChange}
-                onChange={handleChange}
-                tipFormatter={tipFormatter}
-                marks={marks}
-              />
-            )}
+            <InputNumber
+              value={minMax[minIndex]}
+              min={min}
+              max={max}
+              onChange={value => handleChange(Number(value), minIndex)}
+              placeholder="From"
+              data-test="native-filter-from-input"
+            />
+            <StyledDivider>-</StyledDivider>
+            <InputNumber
+              value={minMax[maxIndex]}
+              min={min}
+              max={max}
+              onChange={value => handleChange(Number(value), maxIndex)}
+              placeholder="To"
+              data-test="native-filter-to-input"
+            />
           </Wrapper>
         </StyledFormItem>
       )}