diff --git a/changelogs/DP-13167.md b/changelogs/DP-13167.md new file mode 100644 index 0000000000..1b2bc3ed5a --- /dev/null +++ b/changelogs/DP-13167.md @@ -0,0 +1,4 @@ +Patch +Fixed +- (React) [InputCurrency, InputNumber] DP-13167: Fixed handleAdjust logic so that min/max are not required for up/down buttons to work. #518 +- (React) [InputNumber] DP-13167: Fixed the initial steps when using up/down without a default value and decimal formatting onBlur. #518 diff --git a/react/src/components/atoms/forms/InputCurrency/index.js b/react/src/components/atoms/forms/InputCurrency/index.js index 15f5bc0bac..2e8d8e4d30 100644 --- a/react/src/components/atoms/forms/InputCurrency/index.js +++ b/react/src/components/atoms/forms/InputCurrency/index.js @@ -47,107 +47,88 @@ const Currency = (props) => { } return number; }; - const hasProperty = (obj, property) => Object.prototype.hasOwnProperty.call(obj, property) && !is.nil(obj[property]); + const hasNumberProperty = (obj, property) => Object.prototype.hasOwnProperty.call(obj, property) && is.number(obj[property]); + const greaterThanMin = (val) => !hasNumberProperty(props, 'min') || (val >= props.min); + const lessThanMax = (val) => !hasNumberProperty(props, 'max') || (val <= props.max); + + const displayErrorMessage = (val) => { + const { min, max, required } = props; + if (required && !is.number(val)) { + const errorMsg = 'Please enter a value.'; + return{ + showError: true, + errorMsg + }; + } else if (is.number(val)) { + const { showError, errorMsg } = validNumber(val, min, max); + return{ + showError, errorMsg + }; + } + return{ + showError: false, + errorMsg: '' + }; + }; + const handleChange = (e) => { const { type } = e; const stringValue = ref.current.value; - let numberValue; - const update = { - value: stringValue - }; - if (is.empty(stringValue)) { - numberValue = 0; - } else { - numberValue = Number(numbro.unformat(stringValue)); - } - // This validation is needed here as onKeyDown does not - // get the new value in the input after a key press. - if (props.required && is.empty(stringValue)) { - errorMsg = 'Please enter a value.'; - update.showError = true; - update.errorMsg = errorMsg; - } else if (is.number(numberValue) && !is.empty(stringValue)) { - const validate = validNumber(numberValue, props.min, props.max); - update.showError = validate.showError; - update.errorMsg = validate.errorMsg; - } else { - errorMsg = ''; - update.showError = false; - update.errorMsg = errorMsg; - } - context.updateState(update, () => { - if (typeof props.onChange === 'function') { + const numberValue = stringValue ? Number(numbro.unformat(stringValue)) : 0; + // If the stringvalue is empty, set to empty string so the required error + // message is rendered. Otherwise pass the number value for the min/max check. + const updateError = displayErrorMessage(!is.empty(stringValue) ? numberValue : ''); + context.updateState({ value: stringValue, ...updateError }, () => { + if (is.fn(props.onChange)) { props.onChange(numberValue, props.id, type); } }); }; + const handleAdjust = (e) => { const direction = (e.currentTarget === upRef.current) ? 'up' : 'down'; const { type } = e; - let numberValue; const stringValue = ref.current.value; - if (is.empty(stringValue)) { - numberValue = 0; - } else { - numberValue = Number(numbro.unformat(ref.current.value)); + const numberValue = stringValue ? Number(numbro.unformat(stringValue)) : 0; + let newValue; + if (direction === 'up') { + newValue = Number(numbro(numberValue).add(props.step).format({ mantissa: countDecimals(props.step) })); + } else if (direction === 'down') { + newValue = Number(numbro(numberValue).subtract(props.step).format({ mantissa: countDecimals(props.step) })); } - if (is.number(numberValue)) { - let newValue; - if (direction === 'up') { - newValue = Number(numbro(numberValue).add(props.step).format({ mantissa: countDecimals(props.step) })); - } else if (direction === 'down') { - newValue = Number(numbro(numberValue).subtract(props.step).format({ mantissa: countDecimals(props.step) })); - } - if ((!hasProperty(props, 'min') || newValue >= props.min) && (!hasProperty(props, 'max') || (newValue <= props.max))) { - const { showError, errorMsg } = validNumber(newValue, props.min, props.max); - context.updateState({ - showError, - errorMsg, - value: toCurrency(newValue, countDecimals(props.step)) - }, () => { - if (typeof props.onChange === 'function') { - props.onChange(newValue, props.id, type, direction); - } - }); - } + if (greaterThanMin(newValue) && lessThanMax(newValue)) { + const updateError = displayErrorMessage(!is.empty(stringValue) ? newValue : ''); + context.updateState({ value: toCurrency(newValue, countDecimals(props.step)), ...updateError }, () => { + if (is.fn(props.onChange)) { + props.onChange(newValue, props.id, type, direction); + } + }); } }; + const handleKeyDown = (e) => { const { type, key } = e; const stringValue = ref.current.value; - let numberValue; - if (is.empty(stringValue)) { - numberValue = 0; - } else { - numberValue = Number(numbro.unformat(stringValue)); - } + const numberValue = stringValue ? Number(numbro.unformat(stringValue)) : 0; // default to 0 if defaultValue is NaN if (is.number(numberValue) && !is.empty(stringValue)) { let newValue = numberValue; if (key === 'ArrowDown') { newValue = Number(numbro(numberValue).subtract(props.step).format({ mantissa: countDecimals(props.step) })); - if ((!hasProperty(props, 'min') || newValue >= props.min) && (!hasProperty(props, 'max') || (newValue <= props.max))) { - const { showError, errorMsg } = validNumber(newValue, props.min, props.max); - context.updateState({ - showError, - errorMsg, - value: toCurrency(newValue, countDecimals(props.step)) - }, () => { - if (typeof props.onChange === 'function') { + if (greaterThanMin(newValue) && lessThanMax(newValue)) { + const updateError = displayErrorMessage(!is.empty(stringValue) ? newValue : ''); + context.updateState({ value: toCurrency(newValue, countDecimals(props.step)), ...updateError }, () => { + if (is.fn(props.onChange)) { props.onChange(newValue, props.id, type, key); } }); } } else if (key === 'ArrowUp') { newValue = Number(numbro(numberValue).add(props.step).format({ mantissa: countDecimals(props.step) })); - if ((!hasProperty(props, 'min') || newValue >= props.min) && (!hasProperty(props, 'max') || (newValue <= props.max))) { - const { showError, errorMsg } = validNumber(newValue, props.min, props.max); - context.updateState({ - showError, - errorMsg, - value: toCurrency(newValue, countDecimals(props.step)) - }, () => { - if (typeof props.onChange === 'function') { + if (greaterThanMin(newValue) && lessThanMax(newValue)) { + const updateError = displayErrorMessage(!is.empty(stringValue) ? newValue : ''); + context.updateState({ value: toCurrency(newValue, countDecimals(props.step)), ...updateError }, () => { + if (is.fn(props.onChange)) { props.onChange(newValue, props.id, type, key); } }); @@ -155,40 +136,42 @@ const Currency = (props) => { } } }; + const handleBlur = () => { const inputEl = ref.current; - if (is.empty(inputEl.value)) { + const stringValue = inputEl.value; + // isNotNumber returns true if stringValue is null, undefined or 'NaN' + const isNotNumber = !stringValue || isNaN(Number(numbro.unformat(stringValue))); + if (isNotNumber) { inputEl.setAttribute('placeholder', props.placeholder); } - const stringValue = inputEl.value; - const numberValue = Number(numbro.unformat(stringValue)); + let newValue = isNotNumber ? '' : Number(numbro.unformat(stringValue)); if (props.required && is.empty(stringValue)) { errorMsg = 'Please enter a value.'; context.updateState({ showError: true, errorMsg }); } else if (!is.empty(stringValue)) { - let newValue = numberValue; - if (hasProperty(props, 'max') && newValue > props.max) { + if (!hasNumberProperty(props, 'max') || newValue > props.max) { newValue = props.max; } - if (hasProperty(props, 'min') && newValue < props.min) { + if (!hasNumberProperty(props, 'min') || newValue < props.min) { newValue = props.min; } - const { showError, errorMsg } = validNumber(newValue, props.min, props.max); - context.updateState({ showError, errorMsg, value: toCurrency(newValue, countDecimals(props.step)) }, () => { - // invokes custom function if passed in the component + const updateError = displayErrorMessage(!is.empty(stringValue) ? newValue : ''); + context.updateState({ value: toCurrency(newValue, countDecimals(props.step)), ...updateError }, () => { if (is.fn(props.onBlur)) { - // context.value won't be immediately changed, so pass new value over. - props.onBlur(numberValue); + props.onBlur(newValue); } }); } }; + const handleFocus = () => { const inputEl = ref.current; if (is.empty(inputEl.value)) { inputEl.removeAttribute('placeholder'); } }; + const inputAttr = { className: inputClasses, name: props.name, @@ -207,6 +190,7 @@ const Currency = (props) => { value: context.getValue(), disabled: props.disabled }; + return(
diff --git a/react/src/components/atoms/forms/InputNumber/index.js b/react/src/components/atoms/forms/InputNumber/index.js index ad6cc877c0..76c4ebced7 100644 --- a/react/src/components/atoms/forms/InputNumber/index.js +++ b/react/src/components/atoms/forms/InputNumber/index.js @@ -52,45 +52,35 @@ const NumberInput = (props) => { errorMsg: '' }; }; - const hasProperty = (obj, property) => Object.prototype.hasOwnProperty.call(obj, property) && !is.nil(obj[property]); + + const hasNumberProperty = (obj, property) => Object.prototype.hasOwnProperty.call(obj, property) && is.number(obj[property]); const handleOnBlur = (e) => { e.persist(); const inputEl = ref.current; - let newValue = Number(inputEl.value); - if ((hasProperty(props, 'max') && newValue > props.max) || (hasProperty(props, 'min') && newValue < props.min)) { - if (hasProperty(props, 'max') && newValue > props.max) { - newValue = props.max; - } - if (hasProperty(props, 'min') && newValue < props.min) { - newValue = props.min; - } + let newValue = inputEl.value ? Number(inputEl.value) : inputEl.value; + if (hasNumberProperty(props, 'max') && newValue > props.max) { + newValue = props.max; } - if (!is.empty(inputEl.value)) { - inputEl.value = Number(numbro(newValue) - .format({ mantissa: countDecimals(props.step) })); + if (hasNumberProperty(props, 'min') && newValue < props.min) { + newValue = props.min; + } + if (is.number(newValue)) { + // Since to Fixed returns a string, we have to cast it back to a Number + newValue = Number(newValue.toFixed(countDecimals(props.step))); const updateError = displayErrorMessage(newValue); - context.updateState({ value: inputEl.value, ...updateError }, () => { + context.updateState({ value: newValue, ...updateError }, () => { if (is.fn(props.onBlur)) { - props.onBlur(e, inputEl.value); + props.onBlur(e, newValue); } }); } }; const handleChange = (e) => { - const inputEl = ref.current; e.persist(); - let newValue; - if (is.empty(inputEl.value)) { - newValue = inputEl.value; - } else { - newValue = Number(inputEl.value); - if (is.number(newValue)) { - newValue = Number(numbro(inputEl.value) - .format({ mantissa: countDecimals(props.step) })); - } - } + const inputEl = ref.current; + const newValue = inputEl.value ? Number(inputEl.value) : inputEl.value; const updateError = displayErrorMessage(newValue); context.updateState({ value: newValue, ...updateError }, () => { if (is.fn(props.onChange)) { @@ -100,6 +90,7 @@ const NumberInput = (props) => { }; const handleAdjust = (e) => { + e.persist(); let direction; if (e.currentTarget === upRef.current) { direction = 'up'; @@ -107,22 +98,25 @@ const NumberInput = (props) => { direction = 'down'; } const inputEl = ref.current; - if (direction === 'up' && (!hasProperty(props, 'max') || inputEl.value < props.max)) { - if (is.empty(inputEl.value)) { - inputEl.value = 1; - } else { - inputEl.value = Number(numbro(inputEl.value) - .add(props.step).value()); - } - handleChange(e); - } else if (direction === 'down' && (!hasProperty(props, 'min') || inputEl.value > props.min)) { - if (is.empty(inputEl.value)) { - inputEl.value = -1; - } else { - inputEl.value = Number(numbro(inputEl.value) - .subtract(props.step).value()); - } - handleChange(e); + let newValue = inputEl.value ? Number(inputEl.value) : inputEl.value; + if (direction === 'up' && (!hasNumberProperty(props, 'max') || newValue < props.max)) { + // Since to Fixed returns a string, we have to cast it back to a Number + newValue = newValue ? Number((newValue + props.step).toFixed(countDecimals(props.step))) : props.step; + const updateError = displayErrorMessage(newValue); + context.updateState({ value: newValue, ...updateError }, () => { + if (is.fn(props.onChange)) { + props.onChange(e, newValue, props.id); + } + }); + } else if (direction === 'down' && (!hasNumberProperty(props, 'min') || newValue > props.min)) { + // Since to Fixed returns a string, we have to cast it back to a Number + newValue = newValue ? Number((newValue + props.step * -1).toFixed(countDecimals(props.step))) : (props.step * -1); + const updateError = displayErrorMessage(newValue); + context.updateState({ value: newValue, ...updateError }, () => { + if (is.fn(props.onChange)) { + props.onChange(e, newValue, props.id); + } + }); } };