Skip to content
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.

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

Support "any" step in NumberControl and RangeControl #34542

Merged
merged 14 commits into from
Oct 11, 2021
Merged
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
3 changes: 2 additions & 1 deletion packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
- Changed `RangeControl` component to not apply `shiftStep` to inputs from its `<input type="range"/>` ([35020](https://github.com/WordPress/gutenberg/pull/35020)).
- Removed `isAction` prop from `Item`. The component will now rely on `onClick` to render as a `button` ([35152](https://github.com/WordPress/gutenberg/pull/35152)).

### New Feature
### New Features

- Add an experimental `Navigator` components ([#34904](https://github.com/WordPress/gutenberg/pull/34904)) as a replacement for the previous `Navigation` related components.
- Added support for `step="any"` in `NumberControl` and `RangeControl` ([#34542](https://github.com/WordPress/gutenberg/pull/34542)).

### Bug Fix

Expand Down
20 changes: 18 additions & 2 deletions packages/components/src/number-control/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,22 @@ The position of the label (`top`, `side`, `bottom`, or `edge`).
- Type: `String`
- Required: No

### max

The maximum `value` allowed.

- Type: `Number`
- Required: No
- Default: `Infinity`

### min

The minimum `value` allowed.

- Type: `Number`
- Required: No
- Default: `-Infinity`

### required

If `true` enforces a valid number within the control's min/max range. If `false` allows an empty string as a valid value.
Expand All @@ -99,8 +115,8 @@ Amount to increment by when the `SHIFT` key is held down. This shift value is a

### step

Amount to increment by when incrementing/decrementing.
Amount by which the `value` is changed when incrementing/decrementing. It is also a factor in validation as `value` must be a multiple of `step` (offset by `min`, if specified) to be valid. Accepts the special string value `any` that voids the validation constraint and causes stepping actions to increment/decrement by `1`.

- Type: `Number`
- Type: `Number | "any"`
- Required: No
- Default: `1`
66 changes: 32 additions & 34 deletions packages/components/src/number-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { Input } from './styles/number-control-styles';
import * as inputControlActionTypes from '../input-control/reducer/actions';
import { composeStateReducers } from '../input-control/reducer/reducer';
import { add, subtract, roundClamp } from '../utils/math';
import { useJumpStep } from '../utils/hooks';
import { isValueEmpty } from '../utils/values';

export function NumberControl(
Expand All @@ -40,13 +39,15 @@ export function NumberControl(
},
ref
) {
const baseValue = roundClamp( 0, min, max, step );

const jumpStep = useJumpStep( {
step,
shiftStep,
isShiftStepEnabled,
} );
const isStepAny = step === 'any';
const baseStep = isStepAny ? 1 : parseFloat( step );
const baseValue = roundClamp( 0, min, max, baseStep );
const constrainValue = ( value, stepOverride ) => {
// When step is "any" clamp the value, otherwise round and clamp it
return isStepAny
? Math.min( max, Math.max( min, value ) )
: roundClamp( value, min, max, stepOverride ?? baseStep );
};

const autoComplete = typeProp === 'number' ? 'off' : null;
const classes = classNames( 'components-number-control', className );
Expand Down Expand Up @@ -75,8 +76,8 @@ export function NumberControl(
const enableShift = event.shiftKey && isShiftStepEnabled;

const incrementalValue = enableShift
? parseFloat( shiftStep ) * parseFloat( step )
: parseFloat( step );
? parseFloat( shiftStep ) * baseStep
: baseStep;
let nextValue = isValueEmpty( currentValue )
? baseValue
: currentValue;
Expand All @@ -93,58 +94,55 @@ export function NumberControl(
nextValue = subtract( nextValue, incrementalValue );
}

nextValue = roundClamp( nextValue, min, max, incrementalValue );

state.value = nextValue;
state.value = constrainValue(
nextValue,
enableShift ? incrementalValue : null
ciampo marked this conversation as resolved.
Show resolved Hide resolved
);
}

/**
* Handles drag to update events
*/
if ( type === inputControlActionTypes.DRAG && isDragEnabled ) {
const { delta, shiftKey } = payload;
const [ x, y ] = delta;
const modifier = shiftKey
? parseFloat( shiftStep ) * parseFloat( step )
: parseFloat( step );
const [ x, y ] = payload.delta;
const enableShift = payload.shiftKey && isShiftStepEnabled;
ciampo marked this conversation as resolved.
Show resolved Hide resolved
const modifier = enableShift
? parseFloat( shiftStep ) * baseStep
: baseStep;

let directionModifier;
let directionBaseValue;
let delta;

switch ( dragDirection ) {
case 'n':
directionBaseValue = y;
delta = y;
directionModifier = -1;
break;

case 'e':
directionBaseValue = x;
delta = x;
directionModifier = isRTL() ? -1 : 1;
break;

case 's':
directionBaseValue = y;
delta = y;
directionModifier = 1;
break;

case 'w':
directionBaseValue = x;
delta = x;
directionModifier = isRTL() ? 1 : -1;
break;
}

const distance = directionBaseValue * modifier * directionModifier;
let nextValue;
if ( delta !== 0 ) {
delta = Math.ceil( Math.abs( delta ) ) * Math.sign( delta );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The value of delta comes from react-use-gesture and is a decimal value. It's converted to an integer in order to preserve the NumberControl's decimal value as entered (when step="any"). Done via Math.ceil etc. because single pixel movements produce values like:
image

const distance = delta * modifier * directionModifier;

if ( distance !== 0 ) {
nextValue = roundClamp(
state.value = constrainValue(
add( currentValue, distance ),
min,
max,
modifier
enableShift ? modifier : null
ciampo marked this conversation as resolved.
Show resolved Hide resolved
);

state.value = nextValue;
}
}

Expand All @@ -159,7 +157,7 @@ export function NumberControl(

state.value = applyEmptyValue
? currentValue
: roundClamp( currentValue, min, max, step );
: constrainValue( currentValue );
}

return state;
Expand All @@ -179,7 +177,7 @@ export function NumberControl(
min={ min }
ref={ ref }
required={ required }
step={ jumpStep }
step={ step }
type={ typeProp }
value={ valueProp }
__unstableStateReducer={ composeStateReducers(
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/number-control/stories/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function Example() {
placeholder: text( 'placeholder', 0 ),
required: boolean( 'required', false ),
shiftStep: number( 'shiftStep', 10 ),
step: number( 'step', 1 ),
step: text( 'step', 1 ),
};

return (
Expand Down
40 changes: 40 additions & 0 deletions packages/components/src/number-control/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,16 @@ describe( 'NumberControl', () => {
expect( input.value ).toBe( '-4' );
} );

it( 'should increment while preserving the decimal value when `step` is “any”', () => {
render( <StatefulNumberControl value={ 866.5309 } step="any" /> );

const input = getInput();
input.focus();
fireKeyDown( { keyCode: UP } );

expect( input.value ).toBe( '867.5309' );
} );

it( 'should increment by shiftStep on key UP + shift press', () => {
render( <StatefulNumberControl value={ 5 } shiftStep={ 10 } /> );

Expand All @@ -180,6 +190,16 @@ describe( 'NumberControl', () => {
expect( input.value ).toBe( '20' );
} );

it( 'should increment by shiftStep while preserving the decimal value when `step` is “any”', () => {
render( <StatefulNumberControl value={ 857.5309 } step="any" /> );

const input = getInput();
input.focus();
fireKeyDown( { keyCode: UP, shiftKey: true } );

expect( input.value ).toBe( '867.5309' );
} );

it( 'should increment by custom shiftStep on key UP + shift press', () => {
render( <StatefulNumberControl value={ 5 } shiftStep={ 100 } /> );

Expand Down Expand Up @@ -254,6 +274,16 @@ describe( 'NumberControl', () => {
expect( input.value ).toBe( '-6' );
} );

it( 'should decrement while preserving the decimal value when `step` is “any”', () => {
render( <StatefulNumberControl value={ 868.5309 } step="any" /> );

const input = getInput();
input.focus();
fireKeyDown( { keyCode: DOWN } );

expect( input.value ).toBe( '867.5309' );
} );

it( 'should decrement by shiftStep on key DOWN + shift press', () => {
render( <StatefulNumberControl value={ 5 } /> );

Expand All @@ -264,6 +294,16 @@ describe( 'NumberControl', () => {
expect( input.value ).toBe( '0' );
} );

it( 'should decrement by shiftStep while preserving the decimal value when `step` is “any”', () => {
render( <StatefulNumberControl value={ 877.5309 } step="any" /> );

const input = getInput();
input.focus();
fireKeyDown( { keyCode: DOWN, shiftKey: true } );

expect( input.value ).toBe( '867.5309' );
} );

it( 'should decrement by custom shiftStep on key DOWN + shift press', () => {
render( <StatefulNumberControl value={ 5 } shiftStep={ 100 } /> );

Expand Down
8 changes: 4 additions & 4 deletions packages/components/src/range-control/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -272,17 +272,17 @@ The value to revert to if the Reset button is clicked (enabled by `allowReset`)

#### showTooltip

Forcing the Tooltip UI to show or hide.
Forcing the Tooltip UI to show or hide. This is overriden to `false` when `step` is set to the special string value `any`.

- Type: `Boolean`
- Required: No
- Platform: Web

#### step

The stepping interval between `min` and `max` values. Step is used both for user interface and validation purposes.
The minimum amount by which `value` changes. It is also a factor in validation as `value` must be a multiple of `step` (offset by `min`) to be valid. Accepts the special string value `any` that voids the validation constraint and overrides both `withInputField` and `showTooltip` props to `false`.

- Type: `Number`
- Type: `Number | "any"`
- Required: No
- Platform: Web

Expand Down Expand Up @@ -311,7 +311,7 @@ The current value of the range slider.

#### withInputField

Determines if the `input` number field will render next to the RangeControl.
Determines if the `input` number field will render next to the RangeControl. This is overriden to `false` when `step` is set to the special string value `any`.

- Type: `Boolean`
- Required: No
Expand Down
8 changes: 8 additions & 0 deletions packages/components/src/range-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ function RangeControl(
initial: initialPosition,
} );
const isResetPendent = useRef( false );

if ( step === 'any' ) {
// The tooltip and number input field are hidden when the step is "any"
// because the decimals get too lengthy to fit well.
showTooltipProp = false;
withInputField = false;
}

const [ showTooltip, setShowTooltip ] = useState( showTooltipProp );
const [ isFocused, setIsFocused ] = useState( false );

Expand Down
3 changes: 3 additions & 0 deletions packages/components/src/range-control/rail.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ function Marks( {
step = 1,
value = 0,
} ) {
if ( step === 'any' ) {
step = 1;
}
const marksData = useMarks( { marks, min, max, step, value } );

return (
Expand Down
10 changes: 9 additions & 1 deletion packages/components/src/range-control/stories/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const DefaultExample = () => {
max: number( 'max', 100 ),
min: number( 'min', 0 ),
showTooltip: boolean( 'showTooltip', false ),
step: number( 'step', 1 ),
step: text( 'step', 1 ),
railColor: text( 'railColor', null ),
trackColor: text( 'trackColor', null ),
withInputField: boolean( 'withInputField', true ),
Expand Down Expand Up @@ -81,6 +81,10 @@ export const InitialValueZero = () => {
);
};

export const withAnyStep = () => {
return <RangeControlWithState label="Brightness" step="any" />;
};

export const withHelp = () => {
const label = text( 'Label', 'How many columns should this use?' );
const help = text(
Expand Down Expand Up @@ -174,6 +178,10 @@ export const marks = () => {
<h2>Negative Range</h2>
<Range marks { ...rangeNegative } />
<Range marks={ marksWithNegatives } { ...rangeNegative } />

<h2>Any Step</h2>
<Range marks { ...{ ...stepInteger, step: 'any' } } />
<Range marks={ marksBase } { ...{ ...stepInteger, step: 'any' } } />
</Wrapper>
);
};
Expand Down