dxc-technology / halstack-react

Library of components for building SPAs with React and Halstack Design System
https://developer.dxc.com/halstack/
Apache License 2.0
15 stars 14 forks source link

Fixed bug in TextInputs when scrolling #1838

Closed Mil4n0r closed 7 months ago

Mil4n0r commented 7 months ago

Checklist (Check off all the items before submitting)

Description Fixes bug related to #1800 where, on scroll, text inputs where displaying NaN. The solution has been adding the pre-condition of the field being a number field before applying the handleWheel callback.

Mil4n0r commented 7 months ago

We can solve the useEffect dependencies issue splitting the onWheel functionality:

  1. Use the onWheel event of the Input component to do the increment/decrement functionality (remember to remove the event.preventDefault, is a passive event):
    const handleNumberInputWheel = (event: React.WheelEvent<HTMLInputElement>) => {
      if (document.activeElement === inputRef.current)
        event.deltaY < 0 ? incrementNumber(inputRef.current.value) : decrementNumber(inputRef.current.value);
    };  

onWheel={numberInputContext?.typeNumber === "number" ? handleNumberInputWheel : undefined}

  1. Remove the useEffect from the TextInput and move it to the NumberInput. This makes sense since this functionality should only be available in that component:
    const numberInputRef = React.useRef<HTMLInputElement>(null);

    useEffect(() => {
      const input = numberInputRef.current?.getElementsByTagName("input")[0] as HTMLInputElement;
      const preventDefault = (event: WheelEvent) => {
        event.preventDefault();
      };

      input?.addEventListener("wheel", preventDefault, { passive: false });
      return () => {
        input?.removeEventListener("wheel", preventDefault);
      };
    }, []);

    return (
      <NumberInputContext.Provider value={{ typeNumber: "number", minNumber: min, maxNumber: max, stepNumber: step }}>
        <NumberInputContainer ref={numberInputRef}>

What do you think?

This looks like a much more logical approach, encapsulating the NumberInput functionality encapsulated within the appropiate component and reducing all the unnecessary dependencies inside the TextInput component.

Thanks for your time 😄