UIEXT-2930: Handle stepSize setting for number inputs #100
Conversation
🦋 Changeset detectedLatest commit: e07d7ea The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for configurable step sizes in number input widgets by introducing a stepSize option in the UI schema. Previously, step sizes were hardcoded based on the input type (integer or double). Now users can override the default behavior by specifying a custom step size in the UI schema options.
Key changes:
- Added
stepSizeoption support in the UI schema for NumberControlBase - Implemented custom step size prop in NumberInput component with fallback to defaults
- Improved rounding logic to handle arbitrary step sizes without precision errors
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/jsonforms/src/uiComponents/NumberControlBase.vue | Added stepSize option reading from UI schema and converted stepSize to computed property |
| packages/components/src/components/forms/NumberInput/NumberInput.vue | Added step prop to NumberInput component and improved rounding logic for arbitrary step sizes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/components/src/components/forms/NumberInput/NumberInput.vue
Outdated
Show resolved
Hide resolved
tcrundall-tng
force-pushed
the
enh/UIEXT-2930-make-number-widget-stepsize-configurable
branch
from
8410f0c to
cc8a56e
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/components/src/components/forms/NumberInput/NumberInput.vue
Outdated
Show resolved
Hide resolved
UIEXT-2930 (Add configurable step size to NumberInputWidget)
tcrundall-tng
force-pushed
the
enh/UIEXT-2930-make-number-widget-stepsize-configurable
branch
from
c4a1386 to
ef7ca64
Compare
I need to do this. |
UIEXT-2930 (Add configurable step size to NumberInputWidget)
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| :type="type" | ||
| :min="minParams?.min" | ||
| :max="maxParams?.max" | ||
| :step="stepSize" |
There was a problem hiding this comment.
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.
| :step="stepSize" | |
| :step="stepSize.value" |
| /** 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 | ||
| let scaleFactor = 1 / Math.abs(increment); // eslint-disable-line no-magic-numbers |
There was a problem hiding this comment.
can we be sure that increments are never zero?
There was a problem hiding this comment.
True. Currently the increment is only ever the stepsize.
I've added an extra check that if step is ever 0 then stepSize uses the default.
| let scaleFactor = 1 / Math.abs(increment); // eslint-disable-line no-magic-numbers | ||
| if (Math.abs(increment) < 1) { | ||
| // Avoid rounding errors induced by fractional increments | ||
| scaleFactor = Math.round(scaleFactor); |
There was a problem hiding this comment.
if increment < 0.5, then scaleFactor will be rounded to 0, right? Won't we have a division by zero in line 220 then?
There was a problem hiding this comment.
Not quite. If increment < 0.5, scaleFactor > 2.
If increment > 2, scaleFactor < 0.5, but then we wouldn't be in this if block.
| /** 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 |
There was a problem hiding this comment.
what was wrong with this old logic / why did we add the scaleFactor?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I (and claude) added some unit tests which can double as quasi-documentation of expected behavior.
UIEXT-2930 (Add configurable step size to NumberInputWidget)
UIEXT-2930 (Add configurable step size to NumberInputWidget)
2 participants
UIEXT-2930 (Add configurable step size to NumberInputWidget)