fix(number-input): 🐛 bug with mouse spin

adaptui · Sep 7, 2020 · ec58392 · ec58392
1 parent 4b72a0d
commit ec58392
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 36 deletions.
diff --git a/src/number-input/NumberInputDecrementButton.ts b/src/number-input/NumberInputDecrementButton.ts
@@ -1,4 +1,4 @@
-import { useCallback } from "react";
+import { useCallback, useState } from "react";
 import { createComponent, createHook } from "reakit-system";
 import { ariaAttr, callAllHandlers } from "@chakra-ui/utils";
 import { ButtonHTMLProps, ButtonOptions, useButton } from "reakit/Button";
@@ -27,7 +27,12 @@ export const useNumberInputDecrementButton = createHook<
   keys: NUMBERINPUT_DECREMENTBUTTON_KEYS,
 
   useProps(options, htmlProps) {
-    const { keepWithinRange, focusInput, isAtMin, spinner } = options;
+    const {
+      keepWithinRange,
+      focusInput,
+      isAtMin: isAtMinProp,
+      spinner,
+    } = options;
     const {
       onMouseDown,
       onMouseUp,
@@ -37,23 +42,38 @@ export const useNumberInputDecrementButton = createHook<
       ...restHtmlProps
     } = htmlProps;
 
+    const [isAtMin, setIsAtMin] = useState(false);
+
     const spinDown = useCallback(
       (event: any) => {
         event.preventDefault();
         spinner.down();
         focusInput();
+        setIsAtMin(false);
       },
       [focusInput, spinner],
     );
 
+    const spinStop = useCallback(
+      (event: any) => {
+        event.preventDefault();
+        spinner.stop();
+
+        if (isAtMinProp) {
+          setIsAtMin(true);
+        }
+      },
+      [isAtMinProp, spinner],
+    );
+
     return {
       tabIndex: -1,
       onMouseDown: callAllHandlers(onMouseDown, spinDown),
       onTouchStart: callAllHandlers(onTouchStart, spinDown),
-      onMouseLeave: callAllHandlers(onMouseUp, spinner.stop),
-      onMouseUp: callAllHandlers(onMouseUp, spinner.stop),
-      onTouchEnd: callAllHandlers(onTouchEnd, spinner.stop),
-      disabled: keepWithinRange && isAtMin,
+      onMouseLeave: callAllHandlers(onMouseUp, spinStop),
+      onMouseUp: callAllHandlers(onMouseUp, spinStop),
+      onTouchEnd: callAllHandlers(onTouchEnd, spinStop),
+      disabled: keepWithinRange && isAtMinProp && isAtMin,
       "aria-disabled": ariaAttr(keepWithinRange && isAtMin),
       ...restHtmlProps,
     };

diff --git a/src/number-input/NumberInputIncrementButton.ts b/src/number-input/NumberInputIncrementButton.ts
@@ -1,4 +1,4 @@
-import { useCallback } from "react";
+import { useCallback, useState } from "react";
 import { createComponent, createHook } from "reakit-system";
 import { ariaAttr, callAllHandlers } from "@chakra-ui/utils";
 import { ButtonHTMLProps, ButtonOptions, useButton } from "reakit/Button";
@@ -27,7 +27,12 @@ export const useNumberInputIncrementButton = createHook<
   keys: NUMBERINPUT_INCREMENTBUTTON_KEYS,
 
   useProps(options, htmlProps) {
-    const { keepWithinRange, focusInput, isAtMax, spinner } = options;
+    const {
+      keepWithinRange,
+      focusInput,
+      isAtMax: isAtMaxProp,
+      spinner,
+    } = options;
     const {
       onMouseDown,
       onMouseUp,
@@ -37,23 +42,38 @@ export const useNumberInputIncrementButton = createHook<
       ...restHtmlProps
     } = htmlProps;
 
+    const [isAtMax, setIsAtMax] = useState(false);
+
     const spinUp = useCallback(
       (event: any) => {
         event.preventDefault();
         spinner.up();
         focusInput();
+        setIsAtMax(false);
       },
       [focusInput, spinner],
     );
 
+    const spinStop = useCallback(
+      (event: any) => {
+        event.preventDefault();
+        spinner.stop();
+
+        if (isAtMaxProp) {
+          setIsAtMax(true);
+        }
+      },
+      [isAtMaxProp, spinner],
+    );
+
     return {
       tabIndex: -1,
       onMouseDown: callAllHandlers(onMouseDown, spinUp),
       onTouchStart: callAllHandlers(onTouchStart, spinUp),
-      onMouseUp: callAllHandlers(onMouseUp, spinner.stop),
-      onMouseLeave: callAllHandlers(onMouseLeave, spinner.stop),
-      onTouchEnd: callAllHandlers(onTouchEnd, spinner.stop),
-      disabled: keepWithinRange && isAtMax,
+      onMouseUp: callAllHandlers(onMouseUp, spinStop),
+      onMouseLeave: callAllHandlers(onMouseLeave, spinStop),
+      onTouchEnd: callAllHandlers(onTouchEnd, spinStop),
+      disabled: keepWithinRange && isAtMaxProp && isAtMax,
       "aria-disabled": ariaAttr(keepWithinRange && isAtMax),
       ...restHtmlProps,
     };

diff --git a/src/number-input/__utils.ts b/src/number-input/__utils.ts
@@ -83,19 +83,6 @@ export function useSpinner(
   // Store the timeout instance id in a ref, so we can clear the timeout later
   const timeoutRef = useRef<any>(null);
 
-  // TODO: Think of an effective solution
-  const isAtMinRef = useRef<boolean>(isAtMin);
-
-  useEffect(() => {
-    isAtMinRef.current = isAtMin;
-  }, [isAtMin]);
-
-  const isAtMaxRef = useRef<boolean>(isAtMax);
-
-  useEffect(() => {
-    isAtMaxRef.current = isAtMax;
-  }, [isAtMax]);
-
   // Clears the timeout from memory
   const removeTimeout = () => clearTimeout(timeoutRef.current);
 
@@ -124,11 +111,9 @@ export function useSpinner(
 
     // after a delay, keep incrementing at interval ("spinning up")
     timeoutRef.current = setTimeout(() => {
-      if (!isAtMinRef.current) {
-        setRunOnce(false);
-        setIsSpinning(true);
-        setAction("increment");
-      }
+      setRunOnce(false);
+      setIsSpinning(true);
+      setAction("increment");
     }, CONTINUOUS_CHANGE_DELAY);
   }, [increment, runOnce]);
 
@@ -141,11 +126,9 @@ export function useSpinner(
 
     // after a delay, keep decrementing at interval ("spinning down")
     timeoutRef.current = setTimeout(() => {
-      if (!isAtMinRef.current) {
-        setRunOnce(false);
-        setIsSpinning(true);
-        setAction("decrement");
-      }
+      setRunOnce(false);
+      setIsSpinning(true);
+      setAction("decrement");
     }, CONTINUOUS_CHANGE_DELAY);
   }, [decrement, runOnce]);
 

diff --git a/src/number-input/stories/NumberInput.stories.tsx b/src/number-input/stories/NumberInput.stories.tsx
@@ -30,7 +30,6 @@ export const Default = () => {
   return <NumberInputComp {...props} />;
 };
 
-// TODO: Fix onClick on increment button after disabled
 // TODO: Also handle mouse wheel disabling for disabled input
 export const DefaultValue = () => {
   const props = {