Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/many-apes-appear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@knime/components": patch
"@knime/jsonforms": patch
---

Make step size of NumberInputWidget configurable
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ export default {
default: Number.MAX_SAFE_INTEGER,
type: Number,
},
step: {
default: null,
type: Number,
},
/**
* Validity controlled by the parent component to be flexible.
*/
Expand Down Expand Up @@ -87,6 +91,10 @@ export default {
return this.type === "integer";
},
stepSize() {
if (this.step !== null && this.step !== 0) {
return this.step;
}

return this.isInteger
? DEFAULT_STEP_SIZE_INTEGER
: DEFAULT_STEP_SIZE_DOUBLE;
Expand Down Expand Up @@ -204,7 +212,12 @@ export default {

/** Mimic stepping to nearest step with safe value rounding */
let parsedVal = value + increment;
parsedVal = Math.round(parsedVal * 10) / 10; // eslint-disable-line no-magic-numbers
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what was wrong with this old logic / why did we add the scaleFactor?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was hardcoded to a maximum precision of 1 decimal place. So a step interval of 0.001 would be treated as 0.1.

But there are other issues in my current implementation as you've pointed out. I'll fix it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I (and claude) added some unit tests which can double as quasi-documentation of expected behavior.

let scaleFactor = 1 / Math.abs(increment);
if (Math.abs(increment) < 1) {
// Avoid rounding errors induced by fractional increments
scaleFactor = Math.round(scaleFactor);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if increment < 0.5, then scaleFactor will be rounded to 0, right? Won't we have a division by zero in line 220 then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite. If increment < 0.5, scaleFactor > 2.

If increment > 2, scaleFactor < 0.5, but then we wouldn't be in this if block.

}
parsedVal = Math.round(parsedVal * scaleFactor) / scaleFactor;

/**
* All measures have been taken to ensure a valid value at this point, so if the last
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ describe("NumberInput", () => {
beforeEach(() => {
props = {
modelValue: 10,
stepSize: 1,
min: 0,
max: 20,
title: "knime",
Expand Down Expand Up @@ -172,4 +173,124 @@ describe("NumberInput", () => {
expect(getParsedValueSpy).toHaveNthReturnedWith(2, 1.5);
expect(wrapper.vm.localValue).toBe(1.5);
});

describe("changeValue", () => {
it("increments value by the specified amount when valid", () => {
expect(wrapper.vm.getParsedValue()).toBe(10);
wrapper.vm.changeValue(5);
expect(wrapper.vm.getParsedValue()).toBe(15);
});

it("decrements value by the specified amount when valid", () => {
expect(wrapper.vm.getParsedValue()).toBe(10);
wrapper.vm.changeValue(-2);
expect(wrapper.vm.getParsedValue()).toBe(8);
});

it("respects step size for double precision", async () => {
await wrapper.setProps({ modelValue: 10.5, type: "double" });
wrapper.vm.changeValue(0.1);
expect(wrapper.vm.getParsedValue()).toBe(10.6);
});

it("handles large step sizes correctly (100)", async () => {
await wrapper.setProps({ modelValue: 0, max: 1000 });
wrapper.vm.changeValue(100);
expect(wrapper.vm.getParsedValue()).toBe(100);
wrapper.vm.changeValue(100);
expect(wrapper.vm.getParsedValue()).toBe(200);
wrapper.vm.changeValue(-100);
expect(wrapper.vm.getParsedValue()).toBe(100);
});

it("handles very small step sizes correctly (0.001)", async () => {
await wrapper.setProps({ modelValue: 1.0, type: "double" });
wrapper.vm.changeValue(0.001);
expect(wrapper.vm.getParsedValue()).toBeCloseTo(1.001, 3);
wrapper.vm.changeValue(0.001);
expect(wrapper.vm.getParsedValue()).toBeCloseTo(1.002, 3);
wrapper.vm.changeValue(-0.001);
expect(wrapper.vm.getParsedValue()).toBeCloseTo(1.001, 3);
});

it("snaps to nearest step (double)", async () => {
await wrapper.setProps({ modelValue: 1.001, type: "double" });
wrapper.vm.changeValue(0.01);
expect(wrapper.vm.getParsedValue()).toBeCloseTo(1.01, 3);
});

it("snaps to nearest step (integer)", async () => {
await wrapper.setProps({ modelValue: 123, max: 1000, type: "integer" });
wrapper.vm.changeValue(100);
expect(wrapper.vm.getParsedValue()).toBe(200);

await wrapper.setProps({ modelValue: 10, type: "integer" });
wrapper.vm.changeValue(-3);
expect(wrapper.vm.getParsedValue()).toBe(6);
});

it("handles multiple small increments without floating point errors", async () => {
await wrapper.setProps({ modelValue: 0, type: "double" });
// Add 0.1 ten times
for (let i = 0; i < 10; i++) {
wrapper.vm.changeValue(0.1);
}
// Should be 1.0, not 0.9999999999999999 or similar
expect(wrapper.vm.getParsedValue()).toBeCloseTo(1.0, 1);
});

it("emits update:modelValue event when value changes", () => {
wrapper.vm.changeValue(1);
expect(wrapper.emitted("update:modelValue")).toBeTruthy();
expect(wrapper.emitted("update:modelValue").at(-1)[0]).toBe(11);
});

it("rounds values to avoid floating point precision issues", () => {
wrapper.vm.changeValue(0.1);
wrapper.vm.changeValue(0.1);
wrapper.vm.changeValue(0.1);
// Should be 10.3, not 10.300000000000001
expect(wrapper.vm.getParsedValue()).toBeCloseTo(10.3, 1);
});

it("uses findNearestValidValue when current value is invalid", async () => {
const findNearestValidValueSpy = vi.spyOn(
wrapper.vm,
"findNearestValidValue",
);
await wrapper.setProps({ modelValue: -5 }); // Below min (0)
wrapper.vm.changeValue(1);
expect(findNearestValidValueSpy).toHaveBeenCalledWith(-5);
});

it("does not change value when increment would exceed max", async () => {
await wrapper.setProps({ modelValue: 19, max: 20 });
wrapper.vm.changeValue(5); // Would make it 24, which is > max
expect(wrapper.vm.getParsedValue()).toBe(19); // Should stay at 19
});

it("does not change value when decrement would go below min", async () => {
await wrapper.setProps({ modelValue: 1, min: 0 });
wrapper.vm.changeValue(-5); // Would make it -4, which is < min
expect(wrapper.vm.getParsedValue()).toBe(1); // Should stay at 1
});

it("changes to nearest valid value when currently invalid and incrementing in valid direction", async () => {
await wrapper.setProps({ modelValue: -5, min: 0 }); // Invalid: below min
wrapper.vm.changeValue(1); // Increment towards valid range
expect(wrapper.vm.getParsedValue()).toBe(1); // Should jump to min (0) + increment (1)
});

it("changes to nearest valid value when currently invalid and decrementing in valid direction", async () => {
await wrapper.setProps({ modelValue: 25, max: 20 }); // Invalid: above max
wrapper.vm.changeValue(-1); // Decrement towards valid range
expect(wrapper.vm.getParsedValue()).toBe(19); // Should jump to max (20) + increment (-1)
});

it("gracefully handles an interval of 0", () => {
expect(wrapper.vm.getParsedValue()).toBe(10);
wrapper.vm.changeValue(0);
expect(wrapper.vm.getParsedValue()).toBe(10);
});
});
});
17 changes: 11 additions & 6 deletions packages/jsonforms/src/uiComponents/NumberControlBase.vue
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,14 @@ const props = defineProps<

const DEFAULT_STEP_SIZE_INTEGER = 1;
const DEFAULT_STEP_SIZE_DOUBLE = 0.1;
const stepSize =
props.type === "integer"
? DEFAULT_STEP_SIZE_INTEGER
: DEFAULT_STEP_SIZE_DOUBLE;

const stepSize = computed(
() =>
props.control.uischema.options?.stepSize ??
(props.type === "integer"
? DEFAULT_STEP_SIZE_INTEGER
: DEFAULT_STEP_SIZE_DOUBLE),
);

type BoundValidationParameters = {
isExclusive: boolean;
Expand Down Expand Up @@ -61,7 +65,7 @@ const onFocusOut = () => {
if (minParams.value && !respectsMin(minParams.value)(comparisonValue)) {
const { min, isExclusive } = minParams.value;
if (isExclusive) {
updatedValue = min + stepSize;
updatedValue = min + stepSize.value;
} else {
updatedValue = min;
}
Expand All @@ -71,7 +75,7 @@ const onFocusOut = () => {
) {
const { max, isExclusive } = maxParams.value;
if (isExclusive) {
updatedValue = max - stepSize;
updatedValue = max - stepSize.value;
} else {
updatedValue = max;
}
Expand All @@ -91,6 +95,7 @@ const onFocusOut = () => {
:type="type"
:min="minParams?.min"
:max="maxParams?.max"
:step="stepSize"
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The step prop is being passed the computed ref object instead of its value. Change :step=\"stepSize\" to :step=\"stepSize.value\" to pass the actual number value.

Suggested change
:step="stepSize"
:step="stepSize.value"

Copilot uses AI. Check for mistakes.
:is-valid
compact
@update:model-value="changeValue"
Expand Down
Loading