Unaligned step (#142)

examples/kitchen-sink/kitchen-sink.js

@@ -52,16 +52,24 @@ make( { title: 'Implicit step' }, gui => {
 
 make( { title: 'Explicit step' }, gui => {
 
-	const explicitStep = ( min, max, step, label = step ) => {
-		gui.add( { x: max }, 'x', min, max, step ).name( `[${min},${max}] step ${label}` );
+	const explicitStep = ( min, max, step, label, folder = gui ) => {
+		let x = min === undefined ? max : min;
+		folder.add( { x }, 'x', min, max, step ).name( label || `[${min},${max}] step ${step}` );
 	};
 
 	explicitStep( 0, 100, 1 );
 	explicitStep( 0, 1, 0.1 );
 	explicitStep( -1, 1, 0.25 );
 	explicitStep( 1, 16, .01 );
 	explicitStep( 0, 15, .015 );
-	explicitStep( 0, 5, 1 / 3, '1/3' );
+	explicitStep( 0, 5, 1 / 3, '[0,5] step 1/3' );
+
+	const folder = gui.addFolder( 'Unaligned step' );
+
+	explicitStep( 1, 11, 2, '', folder );
+	explicitStep( 1, 11, 3, '', folder );
+	explicitStep( 1, undefined, 3, '[1,∞] step 3', folder );
+	explicitStep( undefined, 10, 3, '[-∞,10] step 3', folder );
 
 } );
 

src/NumberController.js

@@ -470,16 +470,24 @@ export default class NumberController extends Controller {
 
 	_snap( value ) {
 
-		// This would be the logical way to do things, but floating point errors.
-		// return Math.round( value / this._step ) * this._step;
+		// Make the steps "start" at min or max.
+		let offset = 0;
+		if ( this._hasMin ) {
+			offset = this._min;
+		} else if ( this._hasMax ) {
+			offset = this._max;
+		}
+
+		value -= offset;
+
+		value = Math.round( value / this._step ) * this._step;
 
-		// Using inverse step solves a lot of them, but not all
-		// const inverseStep = 1 / this._step;
-		// return Math.round( value * inverseStep ) / inverseStep;
+		value += offset;
 
-		// Not happy about this, but haven't seen it break.
-		const r = Math.round( value / this._step ) * this._step;
-		return parseFloat( r.toPrecision( 15 ) );
+		// Used to prevent "flyaway" decimals like 1.00000000000001
+		value = parseFloat( value.toPrecision( 15 ) );
+
+		return value;
 
 	}
 

tests/number-step-alignment.js

@@ -0,0 +1,67 @@
+import assert from 'assert';
+import { GUI } from '../dist/lil-gui.esm.min.js';
+
+export default () => {
+
+	stepTest( {
+		start: 0,
+		min: 1,
+		max: 10,
+		step: 1,
+		expectedValues: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ]
+	} );
+
+	stepTest( {
+		start: 0,
+		min: 1,
+		max: 11,
+		step: 2,
+		expectedValues: [ 3, 5, 7, 9, 11 ]
+	} );
+
+	stepTest( {
+		start: 0,
+		min: 10,
+		step: 3,
+		expectedValues: [ 10, 13, 16, 19, 22, 25, 28 ]
+	} );
+
+	stepTest( {
+		start: 21,
+		max: 20,
+		step: 7,
+		expectedValues: [ 13, 6, -1, -8, -15, -22 ]
+	} );
+
+	function stepTest( { start, min, max, step, expectedValues } ) {
+
+		const gui = new GUI();
+
+		const target = { x: start };
+		const controller = gui.add( target, 'x', min, max, step );
+
+		const actualValues = [];
+
+		assert.strictEqual( target.x, start, "value isn't modified until user interaction." );
+
+		for ( let i = 0; i < expectedValues.length; i++ ) {
+
+			// key up if min is defined, otherwise key down
+			const code = min !== undefined ? 'ArrowUp' : 'ArrowDown';
+
+			controller.$input.$callEventListener( 'keydown', { code } );
+			actualValues.push( target.x );
+
+		}
+
+		assert.deepStrictEqual(
+			actualValues,
+			expectedValues,
+			'slider steps correctly even when min/max are not divisible by step. ' +
+			`actual: ${[ ...actualValues ]} ` +
+			`expected: ${[ ...expectedValues ]}`
+		);
+
+	}
+
+};