From 0f5a3897ae525a61d54962f62e7c799d71517ade Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 2 Sep 2022 03:59:23 +0900 Subject: [PATCH 01/21] NumberControl: Improve types --- .../number-control/{index.js => index.tsx} | 51 ++++++++---- .../components/src/number-control/types.ts | 80 +++++++++++++++++++ 2 files changed, 117 insertions(+), 14 deletions(-) rename packages/components/src/number-control/{index.js => index.tsx} (69%) create mode 100644 packages/components/src/number-control/types.ts diff --git a/packages/components/src/number-control/index.js b/packages/components/src/number-control/index.tsx similarity index 69% rename from packages/components/src/number-control/index.js rename to packages/components/src/number-control/index.tsx index 5e21b8ab970f56..46a32693c43a16 100644 --- a/packages/components/src/number-control/index.js +++ b/packages/components/src/number-control/index.tsx @@ -1,8 +1,8 @@ -// @ts-nocheck /** * External dependencies */ import classNames from 'classnames'; +import type { ForwardedRef } from 'react'; /** * WordPress dependencies @@ -17,8 +17,11 @@ import { Input } from './styles/number-control-styles'; import * as inputControlActionTypes from '../input-control/reducer/actions'; import { add, subtract, roundClamp } from '../utils/math'; import { isValueEmpty } from '../utils/values'; +import type { WordPressComponentProps } from '../ui/context/wordpress-component'; +import type { NumberControlProps } from './types'; +import type { InputState } from '../input-control/reducer/state'; -export function NumberControl( +function UnforwardedNumberControl( { __unstableStateReducer: stateReducerProp, className, @@ -35,20 +38,21 @@ export function NumberControl( type: typeProp = 'number', value: valueProp, ...props - }, - ref + }: WordPressComponentProps< NumberControlProps, 'input', false >, + ref: ForwardedRef< any > ) { const isStepAny = step === 'any'; + // @ts-expect-error step should be a number but could be string const baseStep = isStepAny ? 1 : parseFloat( step ); const baseValue = roundClamp( 0, min, max, baseStep ); - const constrainValue = ( value, stepOverride ) => { + const constrainValue = ( value: number, stepOverride?: number | null ) => { // 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 ); /** @@ -56,11 +60,14 @@ export function NumberControl( * 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 + * @return The updated state to apply to InputControl */ - const numberControlStateReducer = ( state, action ) => { + const numberControlStateReducer = ( + /** State from InputControl. */ + state: InputState, + /** Action triggering state change. */ + action: inputControlActionTypes.InputAction + ) => { const nextState = { ...state }; const { type, payload } = action; @@ -74,10 +81,12 @@ export function NumberControl( type === inputControlActionTypes.PRESS_UP || type === inputControlActionTypes.PRESS_DOWN ) { + // @ts-expect-error TODO: Investigate if this is wrong const enableShift = event.shiftKey && isShiftStepEnabled; const incrementalValue = enableShift - ? parseFloat( shiftStep ) * baseStep + ? // @ts-expect-error shiftStep should be a number but could be string + parseFloat( shiftStep ) * baseStep : baseStep; let nextValue = isValueEmpty( currentValue ) ? baseValue @@ -88,14 +97,18 @@ export function NumberControl( } if ( type === inputControlActionTypes.PRESS_UP ) { + // @ts-expect-error TODO: Investigate if this is wrong nextValue = add( nextValue, incrementalValue ); } if ( type === inputControlActionTypes.PRESS_DOWN ) { + // @ts-expect-error TODO: Investigate if this is wrong nextValue = subtract( nextValue, incrementalValue ); } + // @ts-expect-error TODO: Investigate if this is wrong nextState.value = constrainValue( + // @ts-expect-error TODO: Investigate if this is wrong nextValue, enableShift ? incrementalValue : null ); @@ -105,10 +118,13 @@ export function NumberControl( * Handles drag to update events */ if ( type === inputControlActionTypes.DRAG && isDragEnabled ) { + // @ts-expect-error TODO: Investigate const [ x, y ] = payload.delta; + // @ts-expect-error TODO: Investigate const enableShift = payload.shiftKey && isShiftStepEnabled; const modifier = enableShift - ? parseFloat( shiftStep ) * baseStep + ? // @ts-expect-error shiftStep should be a number but could be string + parseFloat( shiftStep ) * baseStep : baseStep; let directionModifier; @@ -140,7 +156,9 @@ export function NumberControl( delta = Math.ceil( Math.abs( delta ) ) * Math.sign( delta ); const distance = delta * modifier * directionModifier; + // @ts-expect-error TODO: Investigate if this is wrong nextState.value = constrainValue( + // @ts-expect-error TODO: Investigate if this is wrong add( currentValue, distance ), enableShift ? modifier : null ); @@ -156,9 +174,11 @@ export function NumberControl( ) { const applyEmptyValue = required === false && currentValue === ''; + // @ts-expect-error TODO: Investigate if this is wrong nextState.value = applyEmptyValue ? currentValue - : constrainValue( currentValue ); + : // @ts-expect-error TODO: Investigate if this is wrong + constrainValue( currentValue ); } return nextState; @@ -180,6 +200,7 @@ export function NumberControl( required={ required } step={ step } type={ typeProp } + // @ts-expect-error TODO: Resolve discrepancy value={ valueProp } __unstableStateReducer={ ( state, action ) => { const baseState = numberControlStateReducer( state, action ); @@ -189,4 +210,6 @@ export function NumberControl( ); } -export default forwardRef( NumberControl ); +export const NumberControl = forwardRef( UnforwardedNumberControl ); + +export default NumberControl; diff --git a/packages/components/src/number-control/types.ts b/packages/components/src/number-control/types.ts new file mode 100644 index 00000000000000..c74611147d88cb --- /dev/null +++ b/packages/components/src/number-control/types.ts @@ -0,0 +1,80 @@ +/** + * External dependencies + */ +import type { HTMLInputTypeAttribute, InputHTMLAttributes } from 'react'; + +/** + * Internal dependencies + */ +import type { InputControlProps } from '../input-control/types'; + +export type NumberControlProps = Omit< + InputControlProps, + 'isDragEnabled' | 'min' | 'max' | 'required' | 'type' | 'value' +> & { + /** + * If true, the default `input` HTML arrows will be hidden. + * + * @default false + */ + hideHTMLArrows?: boolean; + /** + * If true, enables mouse drag gestures. + * + * @default true + */ + isDragEnabled?: boolean; + /** + * If true, pressing `UP` or `DOWN` along with the `SHIFT` key will increment the + * value by the `shiftStep` value. + * + * @default true + */ + isShiftStepEnabled?: boolean; + /** + * The maximum `value` allowed. + * + * @default Infinity + */ + max?: number; + /** + * The minimum `value` allowed. + * + * @default -Infinity + */ + min?: number; + /** + * If `true` enforces a valid number within the control's min/max range. + * If `false` allows an empty string as a valid value. + * + * @default false + */ + required?: boolean; + /** + * Amount to increment by when the `SHIFT` key is held down. This shift value is + * a multiplier to the `step` value. For example, if the `step` value is `5`, + * and `shiftStep` is `10`, each jump would increment/decrement by `50`. + * + * @default 10 + */ + shiftStep?: number; + /** + * 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`. + * + * @default 1 + */ + step?: InputHTMLAttributes< HTMLInputElement >[ 'step' ] | 'any'; + /** + * The `type` attribute of the `input` element. + * + * @default 'number' + */ + type?: HTMLInputTypeAttribute; + /** + * The value of the input. + */ + value?: number | string; +}; From 8f6edb18656f150fec6a77f4005397573796a264 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 2 Sep 2022 04:00:29 +0900 Subject: [PATCH 02/21] Add description to InputControl classname --- packages/components/src/input-control/types.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/components/src/input-control/types.ts b/packages/components/src/input-control/types.ts index 509aae28648c84..d179e9a7c26a6f 100644 --- a/packages/components/src/input-control/types.ts +++ b/packages/components/src/input-control/types.ts @@ -165,6 +165,9 @@ export interface InputBaseProps extends BaseProps, FlexProps { * @default false */ disabled?: boolean; + /** + * The class name to be added to the wrapper element. + */ className?: string; id?: string; /** From 786c1aff6c0dad40f12895ea5a005c8979fded38 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 2 Sep 2022 04:01:42 +0900 Subject: [PATCH 03/21] RangeControl: Add ts-expect-errors --- packages/components/src/range-control/index.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/components/src/range-control/index.tsx b/packages/components/src/range-control/index.tsx index de3c60d9246273..c9c73ea25459d3 100644 --- a/packages/components/src/range-control/index.tsx +++ b/packages/components/src/range-control/index.tsx @@ -138,7 +138,8 @@ function UnforwardedRangeControl< IconProps = unknown >( onChange( nextValue ); }; - const handleOnChange = ( next: string ) => { + const handleOnChange = ( next?: string ) => { + // @ts-expect-error TODO: Investigate if this is problematic let nextValue = parseFloat( next ); setValue( nextValue ); @@ -304,6 +305,7 @@ function UnforwardedRangeControl< IconProps = unknown >( onChange={ handleOnChange } shiftStep={ shiftStep } step={ step } + // @ts-expect-error TODO: Investigate if the `null` value is necessary value={ inputSliderValue } /> ) } From cb4fdb0462e8f22c22174f27d57b8f5b8133e85e Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 2 Sep 2022 04:02:17 +0900 Subject: [PATCH 04/21] ColorPicker: Add ts-expect-errors --- packages/components/src/color-picker/input-with-slider.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/src/color-picker/input-with-slider.tsx b/packages/components/src/color-picker/input-with-slider.tsx index ca0376119f9463..fee14754f59440 100644 --- a/packages/components/src/color-picker/input-with-slider.tsx +++ b/packages/components/src/color-picker/input-with-slider.tsx @@ -33,6 +33,7 @@ export const InputWithSlider = ( { label={ label } hideLabelFromVision value={ value } + // @ts-expect-error TODO: Resolve discrepancy in NumberControl onChange={ onChange } prefix={ Date: Fri, 2 Sep 2022 04:03:02 +0900 Subject: [PATCH 05/21] UnitControl: Get types from NumberControl --- packages/components/src/unit-control/types.ts | 43 ++----------------- 1 file changed, 3 insertions(+), 40 deletions(-) diff --git a/packages/components/src/unit-control/types.ts b/packages/components/src/unit-control/types.ts index c88ec3ba91a117..c4d5ab044479bf 100644 --- a/packages/components/src/unit-control/types.ts +++ b/packages/components/src/unit-control/types.ts @@ -1,22 +1,17 @@ /** * External dependencies */ -import type { - CSSProperties, - FocusEventHandler, - ReactNode, - SyntheticEvent, -} from 'react'; +import type { FocusEventHandler, ReactNode, SyntheticEvent } from 'react'; /** * Internal dependencies */ -import type { StateReducer } from '../input-control/reducer/state'; import type { InputChangeCallback, InputControlProps, Size as InputSize, } from '../input-control/types'; +import type { NumberControlProps } from '../number-control/types'; export type SelectSize = InputSize; @@ -71,14 +66,8 @@ export type UnitSelectControlProps = Pick< InputControlProps, 'size' > & { units?: WPUnitControlUnit[]; }; -// TODO: when available, should (partially) extend `NumberControl` props. export type UnitControlProps = Omit< UnitSelectControlProps, 'unit' > & - Pick< - InputControlProps, - 'hideLabelFromVision' | 'prefix' | '__next36pxDefaultSize' - > & { - __unstableStateReducer?: StateReducer; - __unstableInputWidth?: CSSProperties[ 'width' ]; + NumberControlProps & { /** * The children elements. */ @@ -89,13 +78,6 @@ export type UnitControlProps = Omit< UnitSelectControlProps, 'unit' > & * @default false */ disableUnits?: boolean; - /** - * If `true`, the `ENTER` key press is required in order to trigger an `onChange`. - * If enabled, a change is also triggered when tabbing away (`onBlur`). - * - * @default false - */ - isPressEnterToChange?: boolean; /** * If `true`, and the selected unit provides a `default` value, this value is set * when changing units. @@ -103,10 +85,6 @@ export type UnitControlProps = Omit< UnitSelectControlProps, 'unit' > & * @default false */ isResetValueOnUnitChange?: boolean; - /** - * If this property is added, a label will be generated using label property as the content. - */ - label?: string; /** * Callback when the `unit` changes. */ @@ -122,21 +100,6 @@ export type UnitControlProps = Omit< UnitSelectControlProps, 'unit' > & * For example, a `value` of "50%" will set the current unit to `%`. */ value?: string | number; - /** - * If true, pressing `UP` or `DOWN` along with the `SHIFT` key will increment - * the value by the `shiftStep` value. - * - * @default true - */ - isShiftStepEnabled?: boolean; - /** - * Amount to increment by when the `SHIFT` key is held down. This shift value - * is a multiplier to the `step` value. For example, if the `step` value is `5`, - * and `shiftStep` is `10`, each jump would increment/decrement by `50`. - * - * @default 10 - */ - shiftStep?: number; /** * Callback when either the quantity or the unit inputs lose focus. */ From b9c1fe3ace4a56d89ecbb5ff014aba8be8880c5f Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 2 Sep 2022 04:03:30 +0900 Subject: [PATCH 06/21] UnitControl: Remove redundant aria-label --- packages/components/src/unit-control/index.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/components/src/unit-control/index.tsx b/packages/components/src/unit-control/index.tsx index 6f9725950ff00a..3cf5d128bacbfc 100644 --- a/packages/components/src/unit-control/index.tsx +++ b/packages/components/src/unit-control/index.tsx @@ -259,7 +259,6 @@ function UnforwardedUnitControl( return ( Date: Fri, 2 Sep 2022 04:04:02 +0900 Subject: [PATCH 07/21] NumberControl: Simplify controls in story --- .../src/number-control/stories/index.js | 22 ++++--------------- 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/packages/components/src/number-control/stories/index.js b/packages/components/src/number-control/stories/index.js index ab8a86281ebb5e..7b204cd8b2dfcf 100644 --- a/packages/components/src/number-control/stories/index.js +++ b/packages/components/src/number-control/stories/index.js @@ -12,13 +12,10 @@ export default { title: 'Components (Experimental)/NumberControl', component: NumberControl, argTypes: { - size: { - control: { - type: 'select', - options: [ 'default', 'small', '__unstable-large' ], - }, - }, onChange: { action: 'onChange' }, + prefix: { control: { type: 'text' } }, + suffix: { control: { type: 'text' } }, + type: { control: { type: 'text' } }, }, }; @@ -44,16 +41,5 @@ function Template( { onChange, ...props } ) { export const Default = Template.bind( {} ); Default.args = { - disabled: false, - hideLabelFromVision: false, - isPressEnterToChange: false, - isShiftStepEnabled: true, - label: 'Number', - min: 0, - max: 100, - placeholder: '0', - required: false, - shiftStep: 10, - size: 'default', - step: '1', + label: 'Value', }; From d77a143b85c4e1b8a5ad1bae62ce3b2fe8bd346e Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Tue, 6 Sep 2022 23:41:30 +0900 Subject: [PATCH 08/21] Simplify `stepOverride` type --- packages/components/src/number-control/index.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/src/number-control/index.tsx b/packages/components/src/number-control/index.tsx index 46a32693c43a16..1935a5c631a111 100644 --- a/packages/components/src/number-control/index.tsx +++ b/packages/components/src/number-control/index.tsx @@ -45,7 +45,7 @@ function UnforwardedNumberControl( // @ts-expect-error step should be a number but could be string const baseStep = isStepAny ? 1 : parseFloat( step ); const baseValue = roundClamp( 0, min, max, baseStep ); - const constrainValue = ( value: number, stepOverride?: number | null ) => { + 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 ) ) @@ -110,7 +110,7 @@ function UnforwardedNumberControl( nextState.value = constrainValue( // @ts-expect-error TODO: Investigate if this is wrong nextValue, - enableShift ? incrementalValue : null + enableShift ? incrementalValue : undefined ); } @@ -160,7 +160,7 @@ function UnforwardedNumberControl( nextState.value = constrainValue( // @ts-expect-error TODO: Investigate if this is wrong add( currentValue, distance ), - enableShift ? modifier : null + enableShift ? modifier : undefined ); } } From b2f34f19aae92856970b96d2cce791e60d2b11d6 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Tue, 6 Sep 2022 23:47:28 +0900 Subject: [PATCH 09/21] Add string number utils Co-authored-by: Marco Ciampini --- packages/components/src/utils/values.js | 44 +++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/components/src/utils/values.js b/packages/components/src/utils/values.js index 35d40cbdeddd63..8a124b70325de7 100644 --- a/packages/components/src/utils/values.js +++ b/packages/components/src/utils/values.js @@ -99,3 +99,47 @@ export function isValueNumeric( value, locale = window.navigator.language ) { : value; return ! isNaN( parseFloat( valueToCheck ) ) && isFinite( valueToCheck ); } + +/** + * Converts a string to a number. + * + * @param {string} value + * @return {number} String as a number. + */ +export const stringToNumber = ( value ) => { + return parseFloat( value ); +}; + +/** + * Converts a number to a string. + * + * @param {number} value + * @return {string} Number as a string. + */ +export const numberToString = ( value ) => { + return `${ value }`; +}; + +/** + * Regardless of the input being a string or a number, returns a number. + * + * Returns `undefined` in case the string is `undefined` or not a valid numeric value. + * + * @param {string | number} value + * @return {number} The parsed number. + */ +export const ensureNumber = ( value ) => { + return typeof value === 'string' ? stringToNumber( value ) : value; +}; + +/** + * Regardless of the input being a string or a number, returns a number. + * + * Returns `undefined` in case the string is `undefined` or not a valid numeric value. + * + * @param {string | number} value + * @return {string} The converted string, or `undefined` in case the input is `undefined` or `NaN`. + */ +export const ensureString = ( value ) => { + return typeof value === 'string' ? value : numberToString( value ); +}; From c30c715b8be7ed5fc1d1ba2511d70adf45ec5eec Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Tue, 6 Sep 2022 23:53:26 +0900 Subject: [PATCH 10/21] Replace `parseFloat` with `ensureNumber` --- packages/components/src/number-control/index.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/components/src/number-control/index.tsx b/packages/components/src/number-control/index.tsx index 1935a5c631a111..aae0b4a8454084 100644 --- a/packages/components/src/number-control/index.tsx +++ b/packages/components/src/number-control/index.tsx @@ -16,7 +16,7 @@ import { isRTL } from '@wordpress/i18n'; import { Input } from './styles/number-control-styles'; import * as inputControlActionTypes from '../input-control/reducer/actions'; import { add, subtract, roundClamp } from '../utils/math'; -import { isValueEmpty } from '../utils/values'; +import { ensureNumber, isValueEmpty } from '../utils/values'; import type { WordPressComponentProps } from '../ui/context/wordpress-component'; import type { NumberControlProps } from './types'; import type { InputState } from '../input-control/reducer/state'; @@ -42,8 +42,7 @@ function UnforwardedNumberControl( ref: ForwardedRef< any > ) { const isStepAny = step === 'any'; - // @ts-expect-error step should be a number but could be string - const baseStep = isStepAny ? 1 : parseFloat( step ); + const baseStep = isStepAny ? 1 : ensureNumber( step ); const baseValue = roundClamp( 0, min, max, baseStep ); const constrainValue = ( value: number, stepOverride?: number ) => { // When step is "any" clamp the value, otherwise round and clamp it. @@ -85,8 +84,7 @@ function UnforwardedNumberControl( const enableShift = event.shiftKey && isShiftStepEnabled; const incrementalValue = enableShift - ? // @ts-expect-error shiftStep should be a number but could be string - parseFloat( shiftStep ) * baseStep + ? ensureNumber( shiftStep ) * baseStep : baseStep; let nextValue = isValueEmpty( currentValue ) ? baseValue @@ -123,8 +121,7 @@ function UnforwardedNumberControl( // @ts-expect-error TODO: Investigate const enableShift = payload.shiftKey && isShiftStepEnabled; const modifier = enableShift - ? // @ts-expect-error shiftStep should be a number but could be string - parseFloat( shiftStep ) * baseStep + ? ensureNumber( shiftStep ) * baseStep : baseStep; let directionModifier; From ddcc53aecf2dd0a67e6e97127ce6252258bd1589 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Tue, 6 Sep 2022 23:58:18 +0900 Subject: [PATCH 11/21] Simplify `numberControlStateReducer` type --- .../components/src/number-control/index.tsx | 216 +++++++++--------- 1 file changed, 106 insertions(+), 110 deletions(-) diff --git a/packages/components/src/number-control/index.tsx b/packages/components/src/number-control/index.tsx index aae0b4a8454084..c10c5116f00bad 100644 --- a/packages/components/src/number-control/index.tsx +++ b/packages/components/src/number-control/index.tsx @@ -19,7 +19,6 @@ import { add, subtract, roundClamp } from '../utils/math'; import { ensureNumber, isValueEmpty } from '../utils/values'; import type { WordPressComponentProps } from '../ui/context/wordpress-component'; import type { NumberControlProps } from './types'; -import type { InputState } from '../input-control/reducer/state'; function UnforwardedNumberControl( { @@ -61,125 +60,122 @@ function UnforwardedNumberControl( * * @return The updated state to apply to InputControl */ - const numberControlStateReducer = ( - /** State from InputControl. */ - state: InputState, - /** Action triggering state change. */ - action: inputControlActionTypes.InputAction - ) => { - const nextState = { ...state }; - - const { type, payload } = action; - const event = payload?.event; - const currentValue = nextState.value; - - /** - * Handles custom UP and DOWN Keyboard events - */ - if ( - type === inputControlActionTypes.PRESS_UP || - type === inputControlActionTypes.PRESS_DOWN - ) { - // @ts-expect-error TODO: Investigate if this is wrong - const enableShift = event.shiftKey && isShiftStepEnabled; - - const incrementalValue = enableShift - ? ensureNumber( shiftStep ) * baseStep - : baseStep; - let nextValue = isValueEmpty( currentValue ) - ? baseValue - : currentValue; - - if ( event?.preventDefault ) { - event.preventDefault(); - } - - if ( type === inputControlActionTypes.PRESS_UP ) { + const numberControlStateReducer: NumberControlProps[ '__unstableStateReducer' ] = + ( state, action ) => { + const nextState = { ...state }; + + const { type, payload } = action; + const event = payload.event; + const currentValue = nextState.value; + + /** + * Handles custom UP and DOWN Keyboard events + */ + if ( + type === inputControlActionTypes.PRESS_UP || + type === inputControlActionTypes.PRESS_DOWN + ) { // @ts-expect-error TODO: Investigate if this is wrong - nextValue = add( nextValue, incrementalValue ); - } + const enableShift = event.shiftKey && isShiftStepEnabled; - if ( type === inputControlActionTypes.PRESS_DOWN ) { - // @ts-expect-error TODO: Investigate if this is wrong - nextValue = subtract( nextValue, incrementalValue ); - } + const incrementalValue = enableShift + ? ensureNumber( shiftStep ) * baseStep + : baseStep; + let nextValue = isValueEmpty( currentValue ) + ? baseValue + : currentValue; - // @ts-expect-error TODO: Investigate if this is wrong - nextState.value = constrainValue( - // @ts-expect-error TODO: Investigate if this is wrong - nextValue, - enableShift ? incrementalValue : undefined - ); - } - - /** - * Handles drag to update events - */ - if ( type === inputControlActionTypes.DRAG && isDragEnabled ) { - // @ts-expect-error TODO: Investigate - const [ x, y ] = payload.delta; - // @ts-expect-error TODO: Investigate - const enableShift = payload.shiftKey && isShiftStepEnabled; - const modifier = enableShift - ? ensureNumber( shiftStep ) * baseStep - : baseStep; - - let directionModifier; - let delta; - - switch ( dragDirection ) { - case 'n': - delta = y; - directionModifier = -1; - break; - - case 'e': - delta = x; - directionModifier = isRTL() ? -1 : 1; - break; - - case 's': - delta = y; - directionModifier = 1; - break; - - case 'w': - delta = x; - directionModifier = isRTL() ? 1 : -1; - break; - } + if ( event?.preventDefault ) { + event.preventDefault(); + } - if ( delta !== 0 ) { - delta = Math.ceil( Math.abs( delta ) ) * Math.sign( delta ); - const distance = delta * modifier * directionModifier; + if ( type === inputControlActionTypes.PRESS_UP ) { + // @ts-expect-error TODO: Investigate if this is wrong + nextValue = add( nextValue, incrementalValue ); + } + + if ( type === inputControlActionTypes.PRESS_DOWN ) { + // @ts-expect-error TODO: Investigate if this is wrong + nextValue = subtract( nextValue, incrementalValue ); + } // @ts-expect-error TODO: Investigate if this is wrong nextState.value = constrainValue( // @ts-expect-error TODO: Investigate if this is wrong - add( currentValue, distance ), - enableShift ? modifier : undefined + nextValue, + enableShift ? incrementalValue : undefined ); } - } - - /** - * Handles commit (ENTER key press or blur) - */ - if ( - type === inputControlActionTypes.PRESS_ENTER || - type === inputControlActionTypes.COMMIT - ) { - const applyEmptyValue = required === false && currentValue === ''; - - // @ts-expect-error TODO: Investigate if this is wrong - nextState.value = applyEmptyValue - ? currentValue - : // @ts-expect-error TODO: Investigate if this is wrong - constrainValue( currentValue ); - } - - return nextState; - }; + + /** + * Handles drag to update events + */ + if ( type === inputControlActionTypes.DRAG && isDragEnabled ) { + // @ts-expect-error TODO: Investigate + const [ x, y ] = payload.delta; + // @ts-expect-error TODO: Investigate + const enableShift = payload.shiftKey && isShiftStepEnabled; + const modifier = enableShift + ? ensureNumber( shiftStep ) * baseStep + : baseStep; + + let directionModifier; + let delta; + + switch ( dragDirection ) { + case 'n': + delta = y; + directionModifier = -1; + break; + + case 'e': + delta = x; + directionModifier = isRTL() ? -1 : 1; + break; + + case 's': + delta = y; + directionModifier = 1; + break; + + case 'w': + delta = x; + directionModifier = isRTL() ? 1 : -1; + break; + } + + if ( delta !== 0 ) { + delta = Math.ceil( Math.abs( delta ) ) * Math.sign( delta ); + const distance = delta * modifier * directionModifier; + + // @ts-expect-error TODO: Investigate if this is wrong + nextState.value = constrainValue( + // @ts-expect-error TODO: Investigate if this is wrong + add( currentValue, distance ), + enableShift ? modifier : undefined + ); + } + } + + /** + * Handles commit (ENTER key press or blur) + */ + if ( + type === inputControlActionTypes.PRESS_ENTER || + type === inputControlActionTypes.COMMIT + ) { + const applyEmptyValue = + required === false && currentValue === ''; + + // @ts-expect-error TODO: Investigate if this is wrong + nextState.value = applyEmptyValue + ? currentValue + : // @ts-expect-error TODO: Investigate if this is wrong + constrainValue( currentValue ); + } + + return nextState; + }; return ( Date: Wed, 7 Sep 2022 00:16:43 +0900 Subject: [PATCH 12/21] Cast keyboard event --- packages/components/src/number-control/index.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/components/src/number-control/index.tsx b/packages/components/src/number-control/index.tsx index c10c5116f00bad..32a0931021e235 100644 --- a/packages/components/src/number-control/index.tsx +++ b/packages/components/src/number-control/index.tsx @@ -75,8 +75,9 @@ function UnforwardedNumberControl( type === inputControlActionTypes.PRESS_UP || type === inputControlActionTypes.PRESS_DOWN ) { - // @ts-expect-error TODO: Investigate if this is wrong - const enableShift = event.shiftKey && isShiftStepEnabled; + const enableShift = + ( event as KeyboardEvent | undefined )?.shiftKey && + isShiftStepEnabled; const incrementalValue = enableShift ? ensureNumber( shiftStep ) * baseStep From 4212c0d4bcbf7bdf7f8a15b19f4c3adb23b10811 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Wed, 7 Sep 2022 00:32:57 +0900 Subject: [PATCH 13/21] Disable `value` control in story --- packages/components/src/number-control/stories/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/src/number-control/stories/index.js b/packages/components/src/number-control/stories/index.js index 7b204cd8b2dfcf..6cda47432789f3 100644 --- a/packages/components/src/number-control/stories/index.js +++ b/packages/components/src/number-control/stories/index.js @@ -16,6 +16,7 @@ export default { prefix: { control: { type: 'text' } }, suffix: { control: { type: 'text' } }, type: { control: { type: 'text' } }, + value: { control: null }, }, }; From f759d3585e453b333b179d5dafce0d7182ceaad8 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Wed, 7 Sep 2022 01:27:59 +0900 Subject: [PATCH 14/21] UnitControl: Remove `children` from types Fixup of https://github.com/WordPress/gutenberg/pull/43474/files#diff-b21e5b36bcbb96cd162206795bc262d1ecb0b2ccb1bac9f0096d13c4a1c9c03a --- packages/components/src/unit-control/index.tsx | 1 + packages/components/src/unit-control/types.ts | 6 +----- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/components/src/unit-control/index.tsx b/packages/components/src/unit-control/index.tsx index 3cf5d128bacbfc..73d2c692f0a29a 100644 --- a/packages/components/src/unit-control/index.tsx +++ b/packages/components/src/unit-control/index.tsx @@ -46,6 +46,7 @@ function UnforwardedUnitControl( const { __unstableStateReducer: stateReducerProp, autoComplete = 'off', + // @ts-expect-error Ensure that children is omitted from restProps children, className, disabled = false, diff --git a/packages/components/src/unit-control/types.ts b/packages/components/src/unit-control/types.ts index c4d5ab044479bf..735213e06763a1 100644 --- a/packages/components/src/unit-control/types.ts +++ b/packages/components/src/unit-control/types.ts @@ -1,7 +1,7 @@ /** * External dependencies */ -import type { FocusEventHandler, ReactNode, SyntheticEvent } from 'react'; +import type { FocusEventHandler, SyntheticEvent } from 'react'; /** * Internal dependencies @@ -68,10 +68,6 @@ export type UnitSelectControlProps = Pick< InputControlProps, 'size' > & { export type UnitControlProps = Omit< UnitSelectControlProps, 'unit' > & NumberControlProps & { - /** - * The children elements. - */ - children?: ReactNode; /** * If `true`, the unit `` is hidden. * From fc7bccae7fa8cec4add4519cf8d4c1026c3b404d Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Wed, 7 Sep 2022 01:38:16 +0900 Subject: [PATCH 17/21] UnitControl: Fixup controls --- .../src/unit-control/stories/index.tsx | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/packages/components/src/unit-control/stories/index.tsx b/packages/components/src/unit-control/stories/index.tsx index 98bfc649c90543..6fe4b49a5687db 100644 --- a/packages/components/src/unit-control/stories/index.tsx +++ b/packages/components/src/unit-control/stories/index.tsx @@ -18,24 +18,15 @@ const meta: ComponentMeta< typeof UnitControl > = { component: UnitControl, title: 'Components (Experimental)/UnitControl', argTypes: { - __unstableInputWidth: { - control: { type: 'text' }, - }, - __unstableStateReducer: { - control: { type: null }, - }, - onChange: { - action: 'onChange', - control: { type: null }, - }, - onUnitChange: { - control: { type: null }, - }, - value: { - control: { type: null }, - }, + __unstableInputWidth: { control: { type: 'text' } }, + __unstableStateReducer: { control: { type: null } }, + onChange: { control: { type: null } }, + onUnitChange: { control: { type: null } }, + prefix: { control: { type: 'text' } }, + value: { control: { type: null } }, }, parameters: { + actions: { argTypesRegex: '^on.*' }, controls: { expanded: true, }, From 9cf320378a979e14cc08d3206ce37127d8ba1eaf Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Wed, 7 Sep 2022 19:29:22 +0900 Subject: [PATCH 18/21] Use InputControlProps where possible --- packages/components/src/number-control/types.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/components/src/number-control/types.ts b/packages/components/src/number-control/types.ts index c4ee4cbf79404b..8b271538c1e196 100644 --- a/packages/components/src/number-control/types.ts +++ b/packages/components/src/number-control/types.ts @@ -1,8 +1,3 @@ -/** - * External dependencies - */ -import type { HTMLInputTypeAttribute, InputHTMLAttributes } from 'react'; - /** * Internal dependencies */ @@ -23,7 +18,7 @@ export type NumberControlProps = Omit< * * @default true */ - isDragEnabled?: boolean; + isDragEnabled?: InputControlProps[ 'isDragEnabled' ]; /** * If true, pressing `UP` or `DOWN` along with the `SHIFT` key will increment the * value by the `shiftStep` value. @@ -49,7 +44,7 @@ export type NumberControlProps = Omit< * * @default false */ - required?: boolean; + required?: InputControlProps[ 'required' ]; /** * Amount to increment by when the `SHIFT` key is held down. This shift value is * a multiplier to the `step` value. For example, if the `step` value is `5`, @@ -66,13 +61,13 @@ export type NumberControlProps = Omit< * * @default 1 */ - step?: InputHTMLAttributes< HTMLInputElement >[ 'step' ]; + step?: InputControlProps[ 'step' ]; /** * The `type` attribute of the `input` element. * * @default 'number' */ - type?: HTMLInputTypeAttribute; + type?: InputControlProps[ 'type' ]; /** * The value of the input. */ From 90948f0b1c9c1cace590e39d436336c74ffb57ed Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Wed, 7 Sep 2022 20:21:21 +0900 Subject: [PATCH 19/21] Improve TODO comments --- .../components/src/number-control/index.tsx | 22 +++++++++---------- .../components/src/range-control/index.tsx | 3 ++- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/components/src/number-control/index.tsx b/packages/components/src/number-control/index.tsx index 32a0931021e235..082418bb571d12 100644 --- a/packages/components/src/number-control/index.tsx +++ b/packages/components/src/number-control/index.tsx @@ -91,18 +91,18 @@ function UnforwardedNumberControl( } if ( type === inputControlActionTypes.PRESS_UP ) { - // @ts-expect-error TODO: Investigate if this is wrong + // @ts-expect-error TODO: isValueEmpty() needs to be typed properly nextValue = add( nextValue, incrementalValue ); } if ( type === inputControlActionTypes.PRESS_DOWN ) { - // @ts-expect-error TODO: Investigate if this is wrong + // @ts-expect-error TODO: isValueEmpty() needs to be typed properly nextValue = subtract( nextValue, incrementalValue ); } - // @ts-expect-error TODO: Investigate if this is wrong + // @ts-expect-error TODO: Resolve discrepancy between `value` types in InputControl based components nextState.value = constrainValue( - // @ts-expect-error TODO: Investigate if this is wrong + // @ts-expect-error TODO: isValueEmpty() needs to be typed properly nextValue, enableShift ? incrementalValue : undefined ); @@ -112,9 +112,9 @@ function UnforwardedNumberControl( * Handles drag to update events */ if ( type === inputControlActionTypes.DRAG && isDragEnabled ) { - // @ts-expect-error TODO: Investigate + // @ts-expect-error TODO: See if reducer actions can be typed better const [ x, y ] = payload.delta; - // @ts-expect-error TODO: Investigate + // @ts-expect-error TODO: See if reducer actions can be typed better const enableShift = payload.shiftKey && isShiftStepEnabled; const modifier = enableShift ? ensureNumber( shiftStep ) * baseStep @@ -149,9 +149,9 @@ function UnforwardedNumberControl( delta = Math.ceil( Math.abs( delta ) ) * Math.sign( delta ); const distance = delta * modifier * directionModifier; - // @ts-expect-error TODO: Investigate if this is wrong + // @ts-expect-error TODO: Resolve discrepancy between `value` types in InputControl based components nextState.value = constrainValue( - // @ts-expect-error TODO: Investigate if this is wrong + // @ts-expect-error TODO: isValueEmpty() needs to be typed properly add( currentValue, distance ), enableShift ? modifier : undefined ); @@ -168,10 +168,10 @@ function UnforwardedNumberControl( const applyEmptyValue = required === false && currentValue === ''; - // @ts-expect-error TODO: Investigate if this is wrong + // @ts-expect-error TODO: Resolve discrepancy between `value` types in InputControl based components nextState.value = applyEmptyValue ? currentValue - : // @ts-expect-error TODO: Investigate if this is wrong + : // @ts-expect-error TODO: isValueEmpty() needs to be typed properly constrainValue( currentValue ); } @@ -194,7 +194,7 @@ function UnforwardedNumberControl( required={ required } step={ step } type={ typeProp } - // @ts-expect-error TODO: Resolve discrepancy + // @ts-expect-error TODO: Resolve discrepancy between `value` types in InputControl based components value={ valueProp } __unstableStateReducer={ ( state, action ) => { const baseState = numberControlStateReducer( state, action ); diff --git a/packages/components/src/range-control/index.tsx b/packages/components/src/range-control/index.tsx index c9c73ea25459d3..cec69ac1dfa697 100644 --- a/packages/components/src/range-control/index.tsx +++ b/packages/components/src/range-control/index.tsx @@ -139,7 +139,8 @@ function UnforwardedRangeControl< IconProps = unknown >( }; const handleOnChange = ( next?: string ) => { - // @ts-expect-error TODO: Investigate if this is problematic + // @ts-expect-error TODO: Investigate if it's problematic for setValue() to + // potentially receive a NaN when next is undefined. let nextValue = parseFloat( next ); setValue( nextValue ); From 4cda1eaaf9974f4acd496c7cf2903b3da2a90291 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Thu, 8 Sep 2022 01:42:42 +0900 Subject: [PATCH 20/21] Add changelog --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 4771e00ab349e7..56acee814ccf75 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -20,6 +20,7 @@ - Remove unused `normalizeArrowKey` utility function ([#43640](https://github.com/WordPress/gutenberg/pull/43640/)). - `ToggleGroupControl`: Rename `__experimentalIsIconGroup` prop to `__experimentalIsBorderless` ([#43771](https://github.com/WordPress/gutenberg/pull/43771/)). +- `NumberControl`: Add TypeScript types ([#43791](https://github.com/WordPress/gutenberg/pull/43791/)). - Refactor `FocalPointPicker` to function component ([#39168](https://github.com/WordPress/gutenberg/pull/39168)). - `Guide`: use `code` instead of `keyCode` for keyboard events ([#43604](https://github.com/WordPress/gutenberg/pull/43604/)). - `ToggleControl`: Convert to TypeScript and streamline CSS ([#43717](https://github.com/WordPress/gutenberg/pull/43717)). From ba48ff7dcb63631ba78104cbfc4b6c4acb2383b7 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Thu, 8 Sep 2022 03:22:25 +0900 Subject: [PATCH 21/21] Fix e2e test --- .../e2e-tests/specs/editor/various/font-size-picker.test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/e2e-tests/specs/editor/various/font-size-picker.test.js b/packages/e2e-tests/specs/editor/various/font-size-picker.test.js index f308a56e0e7a3a..a4927878208cab 100644 --- a/packages/e2e-tests/specs/editor/various/font-size-picker.test.js +++ b/packages/e2e-tests/specs/editor/various/font-size-picker.test.js @@ -37,9 +37,7 @@ const toggleCustomInput = async ( showCustomInput ) => { }; const clickCustomInput = async () => { - const customInput = await page.waitForXPath( - "//input[@aria-label='Custom']" - ); + const customInput = await page.waitForXPath( "//input[@type='number']" ); return customInput.click(); };