Skip to content

Commit

Permalink
Switch to using required prop for accepting empty strings
Browse files Browse the repository at this point in the history
To be more in line with an HTML number input, this changes the NumberControl to check the required prop to determine whether an empty string is valid or not.
  • Loading branch information
aaronrobertshaw committed Jul 16, 2021
1 parent 3a8706e commit f796784
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 12 deletions.
15 changes: 7 additions & 8 deletions packages/components/src/number-control/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,6 @@ const Example = () => {

## Props

### allowEmpty

If true, allows an empty string as a valid value.

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

### dragDirection

Determines the drag axis to increment/decrement the value.
Expand Down Expand Up @@ -89,6 +81,13 @@ The position of the label (`top`, `side`, `bottom`, or `edge`).
- Type: `String`
- Required: No

### required

If boolean false, allows an empty string as a valid value.

- Type: `Boolean`
- Required: No

### shiftStep

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`.
Expand Down
5 changes: 3 additions & 2 deletions packages/components/src/number-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ export function NumberControl(
hideHTMLArrows = false,
isDragEnabled = true,
isShiftStepEnabled = true,
allowEmpty = false,
label,
max = Infinity,
min = -Infinity,
required,
shiftStep = 10,
step = 1,
type: typeProp = 'number',
Expand Down Expand Up @@ -156,7 +156,7 @@ export function NumberControl(
type === inputControlActionTypes.PRESS_ENTER ||
type === inputControlActionTypes.COMMIT
) {
const applyEmptyValue = allowEmpty && currentValue === '';
const applyEmptyValue = required === false && currentValue === '';

state.value = applyEmptyValue
? currentValue
Expand All @@ -179,6 +179,7 @@ export function NumberControl(
max={ max }
min={ min }
ref={ ref }
required={ required }
step={ jumpStep }
type={ typeProp }
value={ valueProp }
Expand Down
26 changes: 24 additions & 2 deletions packages/components/src/number-control/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ describe( 'NumberControl', () => {
expect( input.value ).toBe( '0' );
} );

it( 'should accept empty string on ENTER keypress', () => {
render( <NumberControl value={ 5 } allowEmpty={ true } /> );
it( 'should accept empty string on ENTER keypress for optional field', () => {
render( <NumberControl value={ 5 } required={ false } /> );

const input = getInput();
input.focus();
Expand All @@ -103,6 +103,28 @@ describe( 'NumberControl', () => {

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

it( 'should enforce numerical value for empty string when required is omitted', () => {
render( <NumberControl value={ 5 } /> );

const input = getInput();
input.focus();
fireEvent.change( input, { target: { value: '' } } );
fireKeyDown( { keyCode: ENTER } );

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

it( 'should enforce numerical value for empty string when required', () => {
render( <NumberControl value={ 5 } required={ true } /> );

const input = getInput();
input.focus();
fireEvent.change( input, { target: { value: '' } } );
fireKeyDown( { keyCode: ENTER } );

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

describe( 'Key UP interactions', () => {
Expand Down

0 comments on commit f796784

Please sign in to comment.