feat(filter): Add Slider Range Inputs Option for Numerical Range Filters (apache#33170)

Co-authored-by: Geido <60598000+geido@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Mehmet Salih Yavuz <salih.yavuz@proton.me>

Author: 5 people

superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts

@@ -366,9 +366,26 @@ export function saveNativeFilterSettings(charts: ChartSpec[]) {
   cy.get(nativeFilters.modal.footer)
     .contains('Save')
     .should('be.visible')
-    .click();
+    .click({ force: true });
+
+  // Wait for modal to either close or remain open
+  cy.get('body').should($body => {
+    const modalExists = $body.find(nativeFilters.modal.container).length > 0;
+    if (modalExists) {
+      cy.get(nativeFilters.modal.footer)
+        .contains('Save')
+        .should('be.visible')
+        .click({ force: true });
+    }
+  });
+
+  // Ensure modal is closed
   cy.get(nativeFilters.modal.container).should('not.exist');
-  charts.forEach(waitForChartLoad);
+
+  // Wait for all charts to load
+  charts.forEach(chart => {
+    waitForChartLoad(chart);
+  });
 }
 
 /** ************************************************************************

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx

@@ -147,6 +147,8 @@ const HorizontalFormItem = styled(StyledFormItem)<{
   }
 
   .ant-form-item-label {
+    display: flex;
+    align-items: center;
     overflow: visible;
     padding-bottom: 0;
     margin-right: ${({ theme }) => theme.gridUnit * 2}px;
@@ -162,7 +164,7 @@ const HorizontalFormItem = styled(StyledFormItem)<{
   }
 
   .ant-form-item-control {
-    width: ${({ inverseSelection }) => (inverseSelection ? 252 : 164)}px;
+    min-width: ${({ inverseSelection }) => (inverseSelection ? 252 : 164)}px;
   }
 
   .select-container {

superset-frontend/src/dashboard/components/nativeFilters/selectors.ts

@@ -342,6 +342,7 @@ const cachedNativeFilterDataForChart: Record<
     rejectedColumns: Set<string>;
   }
 > = {};
+
 export const selectNativeIndicatorsForChart = (
   nativeFilters: Filters,
   dataMask: DataMaskStateWithId,
@@ -355,17 +356,16 @@ export const selectNativeIndicatorsForChart = (
 
   const cachedFilterData = cachedNativeFilterDataForChart[chartId];
   if (
-    cachedNativeIndicatorsForChart[chartId] &&
-    areObjectsEqual(cachedFilterData?.appliedColumns, appliedColumns) &&
-    areObjectsEqual(cachedFilterData?.rejectedColumns, rejectedColumns) &&
-    cachedFilterData?.nativeFilters === nativeFilters &&
-    cachedFilterData?.chartLayoutItems === chartLayoutItems &&
-    cachedFilterData?.chartConfiguration === chartConfiguration &&
-    cachedFilterData?.dataMask === dataMask
+    cachedNativeIndicatorsForChart[chartId]?.length &&
+    areObjectsEqual(cachedFilterData.appliedColumns, appliedColumns) &&
+    areObjectsEqual(cachedFilterData.rejectedColumns, rejectedColumns) &&
+    areObjectsEqual(cachedFilterData.nativeFilters, nativeFilters) &&
+    areObjectsEqual(cachedFilterData.chartLayoutItems, chartLayoutItems) &&
+    areObjectsEqual(cachedFilterData.chartConfiguration, chartConfiguration) &&
+    areObjectsEqual(cachedFilterData.dataMask, dataMask)
   ) {
     return cachedNativeIndicatorsForChart[chartId];
   }
-
   const nativeFilterIndicators =
     nativeFilters &&
     Object.values(nativeFilters)

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

@@ -17,8 +17,10 @@
  * under the License.
  */
 import { AppSection, GenericDataType } from '@superset-ui/core';
-import { fireEvent, render, screen } from 'spec/helpers/testing-library';
+import userEvent from '@testing-library/user-event';
+import { render, screen } from 'spec/helpers/testing-library';
 import RangeFilterPlugin from './RangeFilterPlugin';
+import { RangeDisplayMode } from './types';
 import { SingleValueType } from './SingleValueType';
 import transformProps from './transformProps';
 
@@ -101,34 +103,40 @@ describe('RangeFilterPlugin', () => {
     jest.clearAllMocks();
   });
 
-  it('should render two numerical inputs', () => {
+  it('should render two numerical inputs and a slider by default', () => {
     getWrapper();
 
     const inputs = screen.getAllByRole('spinbutton');
     expect(inputs).toHaveLength(2);
 
     expect(inputs[0]).toHaveValue('10');
     expect(inputs[1]).toHaveValue('70');
+
+    // For a range slider, there are two slider handles
+    const sliders = screen.getAllByRole('slider');
+    expect(sliders.length).toBeGreaterThan(0);
   });
 
-  it('should set the data mask to error when the range is incorrect', () => {
+  it('should set the data mask to error when the range is incorrect', async () => {
     getWrapper({ filterState: { value: [null, null] } });
 
     const inputs = screen.getAllByRole('spinbutton');
     const fromInput = inputs[0];
     const toInput = inputs[1];
 
-    fireEvent.change(fromInput, { target: { value: 20 } });
+    userEvent.clear(fromInput);
+    userEvent.type(fromInput, '20');
 
-    fireEvent.change(toInput, { target: { value: 10 } });
+    userEvent.clear(toInput);
+    userEvent.type(toInput, '10');
 
-    fireEvent.blur(toInput);
+    userEvent.tab();
 
     expect(setDataMask).toHaveBeenCalledWith({
       extraFormData: {},
       filterState: {
         label: '',
-        validateMessage: 'Please provide a valid range',
+        validateMessage: 'Numbers must be within 10 and 100',
         validateStatus: 'error',
         value: null,
       },
@@ -186,8 +194,10 @@ describe('RangeFilterPlugin', () => {
         value: [20, null],
       },
     });
-    const input = screen.getByRole('spinbutton');
-    expect(input).toHaveValue('20');
+
+    const inputs = screen.getAllByRole('spinbutton');
+    expect(inputs).toHaveLength(1);
+    expect(inputs[0]).toHaveValue('20');
   });
 
   it('should call setDataMask with correct less than filter', () => {
@@ -214,8 +224,10 @@ describe('RangeFilterPlugin', () => {
         validateStatus: undefined,
       },
     });
-    const input = screen.getByRole('spinbutton');
-    expect(input).toHaveValue('60');
+
+    const inputs = screen.getAllByRole('spinbutton');
+    expect(inputs).toHaveLength(1);
+    expect(inputs[0]).toHaveValue('60');
   });
 
   it('should call setDataMask with correct exact filter', () => {
@@ -242,5 +254,69 @@ describe('RangeFilterPlugin', () => {
         validateMessage: '',
       },
     });
+
+    const inputs = screen.getAllByRole('spinbutton');
+    expect(inputs).toHaveLength(1);
+    expect(inputs[0]).toHaveValue('10');
+  });
+
+  describe('Range Display Modes', () => {
+    it('should render only the slider in slider mode', () => {
+      getWrapper({
+        formData: {
+          rangeDisplayMode: RangeDisplayMode.Slider,
+        },
+      });
+
+      const sliders = screen.getAllByRole('slider');
+      expect(sliders.length).toBeGreaterThan(0);
+
+      expect(screen.queryAllByRole('spinbutton')).toHaveLength(0);
+    });
+
+    it('should render only inputs in input mode', () => {
+      getWrapper({
+        formData: {
+          rangeDisplayMode: RangeDisplayMode.Input,
+        },
+      });
+
+      const inputs = screen.getAllByRole('spinbutton');
+      expect(inputs).toHaveLength(2);
+
+      expect(screen.queryAllByRole('slider')).toHaveLength(0);
+    });
+
+    it('should render both slider and inputs in slider-and-input mode', () => {
+      getWrapper({
+        formData: {
+          rangeDisplayMode: RangeDisplayMode.SliderAndInput,
+        },
+      });
+
+      // Should show inputs
+      const inputs = screen.getAllByRole('spinbutton');
+      expect(inputs).toHaveLength(2);
+
+      // Should show slider
+      const sliders = screen.getAllByRole('slider');
+      expect(sliders.length).toBeGreaterThan(0);
+    });
+
+    it('should default to slider-and-input mode when not specified', () => {
+      getWrapper({
+        formData: {
+          // No rangeDisplayMode specified
+        },
+      });
+
+      // Should show inputs
+      const inputs = screen.getAllByRole('spinbutton');
+      expect(inputs).toHaveLength(2);
+
+      // Should show slider
+      const sliders = screen.getAllByRole('slider');
+      expect(sliders.length).toBeGreaterThan(0);
+    });
   });
 });

superset-frontend/src/filters/components/Range/types.ts

@@ -26,9 +26,16 @@ import { RefObject } from 'react';
 import { PluginFilterHooks, PluginFilterStylesProps } from '../types';
 import { FilterBarOrientation } from '../../../dashboard/types';
 
+export enum RangeDisplayMode {
+  Slider = 'slider',
+  Input = 'input',
+  SliderAndInput = 'slider-and-input',
+}
+
 interface PluginFilterSelectCustomizeProps {
   max?: number;
   min?: number;
+  rangeDisplayMode?: RangeDisplayMode;
 }
 
 export type PluginFilterRangeQueryFormData = QueryFormData &

superset-frontend/src/filters/components/Time/TimeFilterPlugin.tsx

@@ -44,6 +44,9 @@ const ControlContainer = styled.div<{
     ${({ validateStatus, theme }) =>
       validateStatus && `border-color: ${theme.colors[validateStatus]?.base}`}
   }
+  & > div {
+    width: 100%;
+  }
 `;
 
 export default function TimeFilterPlugin(props: PluginFilterTimeProps) {