feat: implement number field debouncing #998
Conversation
github-actions
bot
temporarily deployed
to
demo-958-2-debounce-numberfiel
| if (incrementValue) return Big(incrementValue); | ||
| if (decimalDigits) return Big(`1e-${decimalDigits}`); | ||
| return Big('1'); | ||
| useEffect(() => { |
There was a problem hiding this comment.
I think you can take out the useEffect here and just have the if statement running in the render.
|
|
||
| const previousValue = usePrevious(value); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
Same as above. Don't know if you need this wrapped in useEffect
There was a problem hiding this comment.
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.
I'll follow the react guidelines on this one, and we'll go without the effects. Not particularly opinionated on this.
Related to #958
github-actions
bot
temporarily deployed
to
demo-958-2-debounce-numberfiel
|
@douglasbouttell-camunda These are the cross-library screenshots and I'm not sure why but they always fail. Within form-js anyways |
@douglasbouttell-camunda @Skaiir We have to update the screenshots on the Tasklist repo, but I'm waiting for the next release because we have some breaking changes on |
Closes #958
(again)
After a lot of reworks and problems I finally got this sorted.
For testing, make sure the debouncing is working, cleaning up when tabbing out. This should debounce when holding up or down arrow, when typing, and when clicking the arrows via the mouse.
I've also as part of this removed some problem things, like the fact that we were still sending an onChange with 'NaN' (a concept we had removed, but not fully, via #894), the test case is respectively changed.
What we are keeping is the NaN state when pasting something that's not a valid number, this is only visual and is here to give users feedback, otherwise they might think pasting just doesn't do anything. In the future a toast element might be more appropriate. Anyways this state is also more robust now.