blibliki-js / grid

https://www.blibliki.com
9 stars 1 forks source link

Repeated value change of `Input` components with `type="number"` #7

Closed karate closed 1 week ago

karate commented 1 week ago

I found this bug on the Grid, but it may be en engine bug.

I've confirmed this with Chromium and Brave browsers. The bug is not present in Firefox. Not tested in Chrome.

Description: On every <Input type="number" ... /> element (see Sequencer and LFO filter), the browser adds two small arrow buttons that allow the user to increment or decrement the element's value by 1. If the user clicks any of these buttons, the number keeps increasing (or decreasing) until the element loses focus. If the user is not able to stop this (by clicking somewhere else, pressing tab, or focusing another window), the application breaks.

This is not happening if you press the up/down arrows on your keyboard, or if you type the number directly.

mikezaby commented 1 week ago

Thanks that reporting a bug, I have seen it and I'll try to fix it!

mikezaby commented 1 week ago

For some reason, I saw that you have to click out of input to stop increasing or decreasing. I don't know why is behaving like this, but we probably we should handle in some way.

karate commented 1 week ago

Yes, you have to remove focus from the input element, either by clicking outside, pressing tab, or moving your mouse out of the browser.

karate commented 1 week ago

I was thinking that maybe the updateProps function triggers the onChange function and vice-versa, but commenting-out line 59 below didn't help: https://github.com/blibliki-js/grid/blob/df0cdb00d94550f84794d597649bef2445707db5/src/components/AudioModule/Sequencer/GlobalProps.tsx#L58-L74

mikezaby commented 1 week ago

The problem came from react-flow, that probably prevent same event. The easy fix is to add nodrag class to this input, but I'm thinking if there is an easy way, I don't like to fill the codebase with nodrag.

mikezaby commented 1 week ago

I'm thinking something like that any element that isn't a div, is not dragable.

karate commented 1 week ago

Maybe a nodrag class to all <Input> components is that bad. We could add this in line #14 below: https://github.com/blibliki-js/grid/blob/df0cdb00d94550f84794d597649bef2445707db5/src/components/ui/input.tsx#L8-L23

mikezaby commented 1 week ago

I think for now the easiest fix for now is what you suggesting. I was looking if is possible to cancel every handling from react-flow if the element is not a div, but I thinks is not doable.

karate commented 1 week ago

Another solution would be to add "drag handles" to every module, and only use them for drag-and-drop

https://reactflow.dev/examples/nodes/drag-handle