-
Notifications
You must be signed in to change notification settings - Fork 109
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
feat: implement number field debouncing #998
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
import Big from 'big.js'; | ||
import classNames from 'classnames'; | ||
import { useCallback, useMemo, useRef, useState } from 'preact/hooks'; | ||
import { useCallback, useEffect, useMemo, useRef, useState } from 'preact/hooks'; | ||
import { useFlushDebounce, usePrevious } from '../../hooks'; | ||
|
||
import { Description } from '../Description'; | ||
import { Errors } from '../Errors'; | ||
|
@@ -32,8 +33,7 @@ export function Numberfield(props) { | |
onFocus, | ||
field, | ||
value, | ||
readonly, | ||
onChange | ||
readonly | ||
} = props; | ||
|
||
const { | ||
|
@@ -42,7 +42,6 @@ export function Numberfield(props) { | |
appearance = {}, | ||
validate = {}, | ||
decimalDigits, | ||
serializeToString = false, | ||
increment: incrementValue | ||
} = field; | ||
|
||
|
@@ -55,94 +54,120 @@ export function Numberfield(props) { | |
|
||
const inputRef = useRef(); | ||
|
||
const [ stringValueCache, setStringValueCache ] = useState(''); | ||
const [ cachedValue, setCachedValue ] = useState(value); | ||
const [ displayValue, setDisplayValue ] = useState(value); | ||
|
||
// checks whether the value currently in the form data is practically different from the one in the input field cache | ||
// this allows us to guarantee the field always displays valid form data, but without auto-simplifying values like 1.000 to 1 | ||
const cacheValueMatchesState = useMemo(() => Numberfield.config.sanitizeValue({ value, formField: field }) === Numberfield.config.sanitizeValue({ value: stringValueCache, formField: field }), [ stringValueCache, value, field ]); | ||
const sanitize = useCallback((value) => Numberfield.config.sanitizeValue({ value, formField: field }), [ field ]); | ||
|
||
const displayValue = useMemo(() => { | ||
const [ debouncedOnChange, flushOnChange ] = useFlushDebounce(props.onChange); | ||
|
||
if (value === 'NaN') return 'NaN'; | ||
if (stringValueCache === '-') return '-'; | ||
return cacheValueMatchesState ? stringValueCache : ((value || value === 0) ? Big(value).toFixed() : ''); | ||
const previousCachedValue = usePrevious(value); | ||
|
||
}, [ stringValueCache, value, cacheValueMatchesState ]); | ||
|
||
const arrowIncrementValue = useMemo(() => { | ||
|
||
if (incrementValue) return Big(incrementValue); | ||
if (decimalDigits) return Big(`1e-${decimalDigits}`); | ||
return Big('1'); | ||
useEffect(() => { | ||
if (previousCachedValue !== cachedValue) { | ||
debouncedOnChange({ field, value: cachedValue }); | ||
} | ||
}, [ debouncedOnChange, cachedValue, field, previousCachedValue ]); | ||
|
||
}, [ decimalDigits, incrementValue ]); | ||
const onInputBlur = () => { | ||
flushOnChange && flushOnChange(); | ||
onBlur && onBlur(); | ||
}; | ||
|
||
const onInputFocus = () => { | ||
onFocus && onFocus(); | ||
}; | ||
|
||
// all value changes must go through this function | ||
const setValue = useCallback((stringValue) => { | ||
|
||
if (isNullEquivalentValue(stringValue)) { | ||
setStringValueCache(''); | ||
onChange({ field, value: null }); | ||
setDisplayValue(''); | ||
setCachedValue(null); | ||
return; | ||
} | ||
|
||
// treat commas as dots | ||
// converts automatically for countries where the comma is used as a decimal separator | ||
stringValue = stringValue.replaceAll(',', '.'); | ||
|
||
if (stringValue === '-') { | ||
setStringValueCache('-'); | ||
setDisplayValue('-'); | ||
return; | ||
} | ||
|
||
// provides feedback for invalid numbers entered via pasting as opposed to just ignoring the paste | ||
if (isNaN(Number(stringValue))) { | ||
setStringValueCache('NaN'); | ||
onChange({ field, value: 'NaN' }); | ||
setDisplayValue('NaN'); | ||
setCachedValue(null); | ||
return; | ||
} | ||
|
||
setStringValueCache(stringValue); | ||
onChange({ field, value: serializeToString ? stringValue : Number(stringValue) }); | ||
setDisplayValue(stringValue); | ||
setCachedValue(sanitize(stringValue)); | ||
|
||
}, [ sanitize ]); | ||
|
||
// when external changes occur independently of the input, we update the display and cache values of the component | ||
|
||
const previousValue = usePrevious(value); | ||
|
||
useEffect(() => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. Don't know if you need this wrapped in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah probably not, but then again it does encapsulate the behavior in a nice block. I mean either way works for me, I've tested without the effects and it's fine as well. This is more of a style thing, since executing directly in the render is basically an effect that runs every time. In terms of efficiency I'd say no effect is better, since we're just doing a single check as opposed to the multi checks involved in effect eval. But that's not something we should factor in. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll follow the react guidelines on this one, and we'll go without the effects. Not particularly opinionated on this. |
||
const outerValueChanged = previousValue != value; | ||
const outerValueEqualsCache = sanitize(value) === sanitize(cachedValue); | ||
|
||
if (outerValueChanged && !outerValueEqualsCache) { | ||
setValue(value && value.toString() || ''); | ||
} | ||
|
||
}, [ cachedValue, previousValue, sanitize, setValue, value ]); | ||
|
||
// caches the value an increment/decrement operation will be based on | ||
const incrementAmount = useMemo(() => { | ||
|
||
}, [ field, onChange, serializeToString ]); | ||
if (incrementValue) return Big(incrementValue); | ||
if (decimalDigits) return Big(`1e-${decimalDigits}`); | ||
return Big('1'); | ||
|
||
}, [ decimalDigits, incrementValue ]); | ||
|
||
const increment = () => { | ||
if (readonly) { | ||
return; | ||
} | ||
|
||
const base = isValidNumber(value) ? Big(value) : Big(0); | ||
const stepFlooredValue = base.minus(base.mod(arrowIncrementValue)); | ||
const base = isValidNumber(cachedValue) ? Big(cachedValue) : Big(0); | ||
const stepFlooredValue = base.minus(base.mod(incrementAmount)); | ||
|
||
// note: toFixed() behaves differently in big.js | ||
setValue(stepFlooredValue.plus(arrowIncrementValue).toFixed()); | ||
setValue(stepFlooredValue.plus(incrementAmount).toFixed()); | ||
}; | ||
|
||
const decrement = () => { | ||
if (readonly) { | ||
return; | ||
} | ||
|
||
const base = isValidNumber(value) ? Big(value) : Big(0); | ||
const offset = base.mod(arrowIncrementValue); | ||
const base = isValidNumber(cachedValue) ? Big(cachedValue) : Big(0); | ||
const offset = base.mod(incrementAmount); | ||
|
||
if (offset.cmp(0) === 0) { | ||
|
||
// if we're already on a valid step, decrement | ||
setValue(base.minus(arrowIncrementValue).toFixed()); | ||
setValue(base.minus(incrementAmount).toFixed()); | ||
} | ||
else { | ||
|
||
// otherwise floor to the step | ||
const stepFlooredValue = base.minus(base.mod(arrowIncrementValue)); | ||
const stepFlooredValue = base.minus(base.mod(incrementAmount)); | ||
setValue(stepFlooredValue.toFixed()); | ||
} | ||
}; | ||
|
||
const onKeyDown = (e) => { | ||
|
||
// delete the NaN state all at once on backspace or delete | ||
if (value === 'NaN' && (e.code === 'Backspace' || e.code === 'Delete')) { | ||
setValue(null); | ||
if (displayValue === 'NaN' && (e.code === 'Backspace' || e.code === 'Delete')) { | ||
setValue(''); | ||
e.preventDefault(); | ||
return; | ||
} | ||
|
@@ -187,14 +212,15 @@ export function Numberfield(props) { | |
id={ domId } | ||
onKeyDown={ onKeyDown } | ||
onKeyPress={ onKeyPress } | ||
onBlur={ onBlur } | ||
onFocus={ onFocus } | ||
onBlur={ onInputBlur } | ||
onFocus={ onInputFocus } | ||
|
||
// @ts-ignore | ||
onInput={ (e) => setValue(e.target.value) } | ||
onInput={ (e) => setValue(e.target.value, true) } | ||
onPaste={ (e) => displayValue === 'NaN' && e.preventDefault() } | ||
type="text" | ||
autoComplete="off" | ||
step={ arrowIncrementValue } | ||
step={ incrementAmount } | ||
value={ displayValue } | ||
aria-describedby={ errorMessageId } /> | ||
<div class={ classNames('fjs-number-arrow-container', { 'fjs-disabled': disabled, 'fjs-readonly': readonly }) }> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can take out the
useEffect
here and just have the if statement running in the render.