Skip to content

fix: round to correct precision when step uses exponential notation #8290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: round to correct precision when step uses exponential notation #8290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 10 additions & 1 deletion packages/@react-stately/utils/src/number.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,18 @@ export function clamp(value: number, min: number = -Infinity, max: number = Infi

export function roundToStepPrecision(value: number, step: number): number {
let roundedValue = value;
let precision = 0;
let stepString = step.toString();
let pointIndex = stepString.indexOf('.');
let precision = pointIndex >= 0 ? stepString.length - pointIndex : 0;
if (pointIndex >= 0) {
precision = stepString.length - pointIndex;
} else {
// Handle negative exponents in exponential notation (e.g., "1e-7" → precision 8)
let eIndex = stepString.toLowerCase().indexOf('e-');
if (eIndex > 0) {
precision = Math.abs(Number(stepString.slice(eIndex + 1))) + 1;
}
}
if (precision > 0) {
let pow = Math.pow(10, precision);
roundedValue = Math.round(roundedValue * pow) / pow;
Expand Down
26 changes: 25 additions & 1 deletion packages/@react-stately/utils/test/number.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {clamp, snapValueToStep} from '../src/number';
import {clamp, roundToStepPrecision, snapValueToStep} from '../src/number';

describe('number utils', () => {
describe('clamp', () => {
Expand All @@ -8,6 +8,30 @@ describe('number utils', () => {
});
});

describe('roundToStepPrecision', () => {
it('should return the input unchanged for integer steps', () => {
expect(roundToStepPrecision(7.123, 1)).toBe(7.123);
expect(roundToStepPrecision(5, 10)).toBe(5);
});
it('should round to the correct decimal places for steps with decimals', () => {
expect(roundToStepPrecision(1.24, 0.1)).toBe(1.24);
Copy link
Member

@snowystinger snowystinger May 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

maybe I'm over thinking this, but shouldn't this be

Suggested change
expect(roundToStepPrecision(1.24, 0.1)).toBe(1.24);
expect(roundToStepPrecision(1.24, 0.1)).toBe(1.2);

I realize it's the existing behaviour, I'll have to look back to remember why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for looking into this! I was wondering the same thing while looking at the original implementation 🤔

let precision = pointIndex >= 0 ? stepString.length - pointIndex : 0;

I also thought precision should maybe be stepString.length - pointIndex - 1, but the original behavior seemed correct, since all the existing test cases in NumberField.test and number.test passed. Also, this #8274 (comment) suggests that for an exponential notation like 1e-7, the precision should be 8. So, I implemented the existing behavior for exponential notation as well.

expect(roundToStepPrecision(1.456, 0.01)).toBe(1.456);
// Should not overcount precision
expect(roundToStepPrecision(2.349, 0.100)).toBe(2.35);
expect(roundToStepPrecision(2.35, 0.100)).toBe(2.35);
// Should handle negative values
expect(roundToStepPrecision(-1.456, 0.01)).toBe(-1.456);
// Should handle zero value
expect(roundToStepPrecision(0, 0.01)).toBe(0);
});
it('should handle rounding for exponential step values', () => {
expect(roundToStepPrecision(0.123456789, 1e-3)).toBe(0.1235);
expect(roundToStepPrecision(0.123456789, 1e-7)).toBe(0.12345679);
// Should handle exponential notation steps regardless of e/E case
expect(roundToStepPrecision(0.123456789, 1E-8)).toBe(0.123456789);
});
});

describe('snapValueToStep', () => {
it('should snap value to nearest step based on min and max', () => {
expect(snapValueToStep(2, -0.5, 100, 3)).toBe(2.5);
Expand Down