rusackas commented on code in PR #34742:
URL: https://github.com/apache/superset/pull/34742#discussion_r3330283686
##########
superset-frontend/src/filters/components/Range/RangeFilterPlugin.test.tsx:
##########
@@ -322,4 +322,127 @@ describe('RangeFilterPlugin', () => {
expect(sliders.length).toBeGreaterThan(0);
});
});
+
+ describe('Decimal value handling', () => {
+ it('should handle decimal ranges correctly (0.03 to 1.08)', () => {
+ const decimalProps = {
+ queriesData: [
+ {
+ rowcount: 1,
+ colnames: ['min', 'max'],
+ coltypes: [GenericDataType.Numeric, GenericDataType.Numeric],
+ data: [{ min: 0.03, max: 1.08 }],
+ applied_filters: [],
+ rejected_filters: [],
+ },
+ ],
+ filterState: { value: [0.5, 0.8] },
+ };
+ getWrapper(decimalProps);
+
+ const inputs = screen.getAllByRole('spinbutton');
+ expect(inputs).toHaveLength(2);
+ expect(inputs[0]).toHaveValue('0.5');
+ expect(inputs[1]).toHaveValue('0.8');
+
+ // Verify the slider exists and can handle decimal values
+ const sliders = screen.getAllByRole('slider');
+ expect(sliders.length).toBeGreaterThan(0);
+ });
+
+ it('should calculate appropriate step size for small decimal ranges', ()
=> {
+ const smallRangeProps = {
+ queriesData: [
+ {
+ rowcount: 1,
+ colnames: ['min', 'max'],
+ coltypes: [GenericDataType.Numeric, GenericDataType.Numeric],
+ data: [{ min: 0.001, max: 0.01 }],
+ applied_filters: [],
+ rejected_filters: [],
+ },
+ ],
+ filterState: { value: [0.005, 0.008] },
+ };
+ getWrapper(smallRangeProps);
+
+ const inputs = screen.getAllByRole('spinbutton');
+ expect(inputs[0]).toHaveValue('0.005');
+ expect(inputs[1]).toHaveValue('0.008');
+ });
+
+ it('should handle very large ranges with appropriate step size', () => {
+ const largeRangeProps = {
+ queriesData: [
+ {
+ rowcount: 1,
+ colnames: ['min', 'max'],
+ coltypes: [GenericDataType.Numeric, GenericDataType.Numeric],
+ data: [{ min: 0, max: 1000000 }],
+ applied_filters: [],
+ rejected_filters: [],
+ },
+ ],
+ filterState: { value: [100000, 500000] },
+ };
+ getWrapper(largeRangeProps);
+
+ const inputs = screen.getAllByRole('spinbutton');
+ expect(inputs[0]).toHaveValue('100000');
+ expect(inputs[1]).toHaveValue('500000');
+ });
+
+ it('should handle negative decimal ranges', () => {
+ const negativeDecimalProps = {
+ queriesData: [
+ {
+ rowcount: 1,
+ colnames: ['min', 'max'],
+ coltypes: [GenericDataType.Numeric, GenericDataType.Numeric],
+ data: [{ min: -1.5, max: 2.5 }],
+ applied_filters: [],
+ rejected_filters: [],
+ },
+ ],
+ filterState: { value: [-0.5, 1.5] },
+ };
+ getWrapper(negativeDecimalProps);
+
+ const inputs = screen.getAllByRole('spinbutton');
+ expect(inputs[0]).toHaveValue('-0.5');
+ expect(inputs[1]).toHaveValue('1.5');
+ });
+
+ it('should allow decimal input via keyboard', async () => {
+ const decimalProps = {
+ queriesData: [
+ {
+ rowcount: 1,
+ colnames: ['min', 'max'],
+ coltypes: [GenericDataType.Numeric, GenericDataType.Numeric],
+ data: [{ min: 0, max: 10 }],
+ applied_filters: [],
+ rejected_filters: [],
+ },
+ ],
+ filterState: { value: [null, null] },
+ };
+ getWrapper(decimalProps);
+
+ const inputs = screen.getAllByRole('spinbutton');
+ const fromInput = inputs[0];
+
+ userEvent.clear(fromInput);
+ userEvent.type(fromInput, '2.5');
+ userEvent.tab();
Review Comment:
Thanks for saving the instruction.
##########
superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx:
##########
@@ -234,6 +234,40 @@ export default function RangeFilterPlugin(props:
PluginFilterRangeProps) {
const [row] = data;
// @ts-ignore
const { min, max }: { min: number; max: number } = row;
+
+ // Calculate appropriate step size for decimal values
+ const calculateStep = useCallback((minValue: number, maxValue: number) => {
+ const range = maxValue - minValue;
+
+ // If the range is very small (less than 1), use smaller steps
+ if (range < 1) {
+ // Find the number of decimal places needed
+ const rangeStr = range.toString();
+ const decimalMatch = rangeStr.match(/\.(\d+)/);
+ if (decimalMatch) {
+ const decimalPlaces = decimalMatch[1].length;
+ // Use a step that gives approximately 100 steps across the range
+ return Math.pow(10, -Math.min(decimalPlaces + 1, 6));
+ }
+ return 0.01;
+ }
+
+ // For larger ranges, calculate step to give approximately 100-1000 steps
+ const idealSteps = 100;
+ let step = range / idealSteps;
+
+ // Round step to a nice value (0.001, 0.01, 0.1, 1, 10, etc.)
+ const magnitude = Math.pow(10, Math.floor(Math.log10(step)));
+ step = Math.round(step / magnitude) * magnitude;
+
+ return step;
+ }, []);
Review Comment:
Acknowledged — yes, the unified approach is already in place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
