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

NumberControl: refactor to TypeScript #38753

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
bab61da
Rename index and styles
ciampo Feb 11, 2022
7f50084
Add prop types
ciampo Feb 11, 2022
bb7befb
Fix styles
ciampo Feb 11, 2022
1072a51
Type component signature
ciampo Feb 11, 2022
ee9e385
Ensure `min`, `max` amd `step` are converted to numbers for internal …
ciampo Feb 11, 2022
91cb750
Remove unnecessary `parseFloat`, since `shiftStep` is a `number`
ciampo Feb 11, 2022
126bdc7
Type `numberControlStateReducer` using the `Props` type
ciampo Feb 11, 2022
44b503f
Type `contrainValue`
ciampo Feb 11, 2022
b56da69
Default to `0` if `currentValue` is `undefined`, convert to number
ciampo Feb 11, 2022
f6a2df0
Commit and forward `value` as a `string`
ciampo Feb 11, 2022
800b8be
Cast even to `KeyboardEvent` when reacting to key up/down events
ciampo Feb 11, 2022
0167076
WIP README
ciampo Feb 11, 2022
b47c769
Ignore remaining TS errors
ciampo Feb 11, 2022
1c04a00
ShiftStep also as a string
ciampo Feb 16, 2022
8f20af5
Add `ensureString` utility function
ciampo Feb 16, 2022
62a3e27
Add `onDrag`, `onDragStart` and `onDragEnd` props from `InputControl`…
ciampo Feb 16, 2022
ace5d44
Force `nextValue` to be alway a string in the `UnitControl` onChange …
ciampo Feb 16, 2022
bd7491f
Tweak `value` and `onChange` so that they can accept also `number` in…
ciampo Feb 16, 2022
7d282d7
Update `InputControl` README to reflect that `value` can only be a `s…
ciampo Feb 16, 2022
486b099
Fix `ColorPicker` s TS errors
ciampo Feb 16, 2022
cb0b607
Tweak Storybook values
ciampo Feb 16, 2022
367c59d
`NumberControl` s value as a `string`
ciampo Feb 18, 2022
a9fa8a1
Fix story
ciampo Feb 18, 2022
8ecd990
Type reducer s DRAG action
ciampo Feb 18, 2022
0f9fc2d
Refactor common number/string utils
ciampo Feb 18, 2022
d507a8f
Update styles to assume `value` is a `string`
ciampo Feb 18, 2022
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: 9 additions & 2 deletions packages/components/src/color-picker/input-with-slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Spacer } from '../spacer';
import { space } from '../ui/utils/space';
import { RangeControl, NumberControlWrapper } from './styles';
import { COLORS } from '../utils/colors-values';
import { ensureNumber, ensureString } from '../utils/values';

interface InputWithSliderProps {
min: number;
Expand All @@ -32,8 +33,14 @@ export const InputWithSlider = ( {
max={ max }
label={ label }
hideLabelFromVision
value={ value }
onChange={ onChange }
value={ ensureString( value ) }
onChange={ ( nextValue?: string ) => {
if ( typeof nextValue === 'undefined' ) {
return;
}

onChange?.( ensureNumber( nextValue ) );
} }
prefix={
<Spacer
as={ Text }
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/input-control/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,5 +105,5 @@ Type of the input element to render. Defaults to "text".

The current value of the input.

- Type: `String | Number`
- Type: `String`
- Required: Yes
28 changes: 15 additions & 13 deletions packages/components/src/number-control/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,58 +27,60 @@ const Example = () => {

## Props

TODO: Should point at `InputControl` for overlapping props

### dragDirection

Determines the drag axis to increment/decrement the value.
Directions: `n` | `e` | `s` | `w`

- Type: `String`
- Type: `n` | `e` | `s` | `w`
- Required: No
- Default: `n`

### dragThreshold

If `isDragEnabled` is true, this controls the amount of `px` to have been dragged before the value changes.
If `isDragEnabled` is `true`, this controls the amount of `px` to have been dragged before the value changes.

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

### hideHTMLArrows

If true, the default `input` HTML arrows will be hidden.
If `true`, the default `input` HTML arrows will be hidden.

- Type: `Boolean`
- Type: `boolean`
- Required: No
- Default: `false`

### isDragEnabled

If true, enables mouse drag gesture to increment/decrement the number value. Holding `SHIFT` while dragging will increase the value by the `shiftStep`.
If `true`, enables mouse drag gesture to increment/decrement the value. Holding `SHIFT` while dragging will increase the value by the `shiftStep`.

- Type: `Boolean`
- Type: `boolean`
- Required: No
- Default: `true`

### isShiftStepEnabled

If true, pressing `UP` or `DOWN` along with the `SHIFT` key will increment the value by the `shiftStep` value.
If `true`, pressing `UP` or `DOWN` along with the `SHIFT` key will increment the value by the `shiftStep` value.

- Type: `Boolean`
- Type: `boolean`
- Required: No
- Default: `true`

### label

If this property is added, a label will be generated using label property as the content.
The text content of the label. If not specified, a label won't be displayed.

- Type: `String`
- Type: `string` // It's actually a React.Node
- Required: No

### labelPosition

The position of the label (`top`, `side`, `bottom`, or `edge`).
The position of the label.

- Type: `String`
- Type: `top` | `side` | `bottom` | `edge`
- Required: No

### max
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// @ts-nocheck
/**
* External dependencies
*/
import classNames from 'classnames';
import type { Ref } from 'react';

/**
* WordPress dependencies
Expand All @@ -17,7 +17,9 @@ 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 { isValueEmpty } from '../utils/values';
import { isValueEmpty, ensureNumber, ensureString } from '../utils/values';
import type { Props } from './types';
import type { DragAction } from '../input-control/reducer/actions';

export function NumberControl(
{
Expand All @@ -28,40 +30,41 @@ export function NumberControl(
isDragEnabled = true,
isShiftStepEnabled = true,
label,
max = Infinity,
min = -Infinity,
max: maxProp = Infinity,
min: minProp = -Infinity,
required = false,
shiftStep = 10,
step = 1,
shiftStep: shiftStepProp = 10,
step: stepProp = 1,
type: typeProp = 'number',
value: valueProp,
...props
},
ref
}: Props,
ref: Ref< any >
) {
const isStepAny = step === 'any';
const baseStep = isStepAny ? 1 : parseFloat( step );
const min = ensureNumber( minProp );
const max = ensureNumber( maxProp );
const shiftStep = ensureNumber( shiftStepProp );

const isStepAny = stepProp === 'any';
const baseStep = isStepAny ? 1 : ensureNumber( stepProp );
const baseValue = roundClamp( 0, min, max, baseStep );
const constrainValue = ( value, stepOverride ) => {
const constrainValue = ( value: number, stepOverride?: number ) => {
// 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 autoComplete = typeProp === 'number' ? 'off' : undefined;
const classes = classNames( 'components-number-control', className );

/**
* "Middleware" function that intercepts updates from InputControl.
* This allows us to tap into actions to transform the (next) state for
* InputControl.
*
* @param {Object} state State from InputControl
* @param {Object} action Action triggering state change
* @return {Object} The updated state to apply to InputControl
*/
const numberControlStateReducer = ( state, action ) => {
// "Middleware" function that intercepts updates from InputControl.
// This allows us to tap into actions to transform the (next) state for
// `InputControl`.
const numberControlStateReducer: Props[ '__unstableStateReducer' ] = (
state,
action
) => {
const { type, payload } = action;
const event = payload?.event;
const currentValue = state.value;
Expand All @@ -73,14 +76,16 @@ export function NumberControl(
type === inputControlActionTypes.PRESS_UP ||
type === inputControlActionTypes.PRESS_DOWN
) {
const enableShift = event.shiftKey && isShiftStepEnabled;
const enableShift =
( event as KeyboardEvent | undefined )?.shiftKey &&
isShiftStepEnabled;

const incrementalValue = enableShift
? parseFloat( shiftStep ) * baseStep
? shiftStep * baseStep
: baseStep;
let nextValue = isValueEmpty( currentValue )
? baseValue
: currentValue;
: ensureNumber( currentValue );

if ( event?.preventDefault ) {
event.preventDefault();
Expand All @@ -94,21 +99,22 @@ export function NumberControl(
nextValue = subtract( nextValue, incrementalValue );
}

state.value = constrainValue(
nextValue,
enableShift ? incrementalValue : null
state.value = ensureString(
constrainValue(
nextValue,
enableShift ? incrementalValue : undefined
)
);
}

/**
* Handles drag to update events
*/
if ( type === inputControlActionTypes.DRAG && isDragEnabled ) {
const [ x, y ] = payload.delta;
const enableShift = payload.shiftKey && isShiftStepEnabled;
const modifier = enableShift
? parseFloat( shiftStep ) * baseStep
: baseStep;
const { payload: dragPayload } = action as DragAction;
const [ x, y ] = dragPayload.delta;
const enableShift = dragPayload.shiftKey && isShiftStepEnabled;
const modifier = enableShift ? shiftStep * baseStep : baseStep;

let directionModifier;
let delta;
Expand Down Expand Up @@ -139,9 +145,11 @@ export function NumberControl(
delta = Math.ceil( Math.abs( delta ) ) * Math.sign( delta );
const distance = delta * modifier * directionModifier;

state.value = constrainValue(
add( currentValue, distance ),
enableShift ? modifier : null
state.value = ensureString(
constrainValue(
add( ensureNumber( currentValue ?? 0 ), distance ),
enableShift ? modifier : undefined
)
);
}
}
Expand All @@ -157,7 +165,9 @@ export function NumberControl(

state.value = applyEmptyValue
? currentValue
: constrainValue( currentValue );
: ensureString(
constrainValue( ensureNumber( currentValue ?? 0 ) )
);
}

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

return (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
// @ts-nocheck
/**
* External dependencies
*/
import { css } from '@emotion/react';
import styled from '@emotion/styled';

/**
* Internal dependencies
*/
import InputControl from '../../input-control';
import type { Props } from '../types';

const htmlArrowStyles = ( { hideHTMLArrows } ) => {
const htmlArrowStyles = ( {
hideHTMLArrows,
}: Pick< Props, 'hideHTMLArrows' > ) => {
if ( ! hideHTMLArrows ) return ``;

// TODO: rewrite using Emotion selection from `InputControl` component
return css`
input[type='number']::-webkit-outer-spin-button,
input[type='number']::-webkit-inner-spin-button {
Expand Down
Loading