feat: Steppers with shift +/-10 modifiers for number inputs #26
Conversation
Added a new wrapper around the NumberInput that listens to shift up/down events and sets the step to +10 instead of +1 when shift is held down
| }>(); | ||
| useEffect(() => { | ||
| // Here will be adding the static listener so we can keep the reference | ||
| eventListeners.current = { keydown: keyDownHandler, keyup: keyUpHandler }; |
There was a problem hiding this comment.
Hey, what's the point of useRef vs a local variable in the useEffect callback function?
| (e: KeyboardEvent) => e.key == 'Shift' && setInputStep(10), | ||
| [], | ||
| ); | ||
| const keyUpHandler = useCallback( |
There was a problem hiding this comment.
Why use useCallback instead of setting directly inside the useEffect? Seems like a a simple functionality with extra steps
There was a problem hiding this comment.
Hi @aniforprez and thanks for the contribution! About the events setup, I have two notes:
- I think the @almarzn suggestion is right here, we can probably get it in an easier way without having to handle refs
- In general, I am a bit worried about all the event listeners we're adding (two for every Input).
Since we don't really need any specific functionaly on shift clicking, we could just have one handler (with useHotkeys or with window.addEventListener should be similar, I'd prefer useHotkeys since it's battle tested).
On shift/unshift we can store the value a custom zustand slice (technically it could just be a custom context, but I'd prefer to keep it in zustand to make it easier for DX)
So like an hotkeysSlice with setInputStep.
Then in every FactoryNumberInput we'd just need to do const inputStep = useStore(state => state.hotkeys.inputStep);` and use that as step
What do you think?
Number inputs for resources now have the steppers added back and will increment/decrement by 10 when shift is pressed.
Screen.Recording.2024-11-11.at.7.18.30.PM.mov