fix: Tooltip logic (#1008)

* fix: remove logic * test: add test case
react-component · Jul 18, 2024 · 417e9af · 417e9af
1 parent f2206cf
commit 417e9af
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 18 deletions.
diff --git a/src/Handles/index.tsx b/src/Handles/index.tsx
@@ -1,4 +1,5 @@
 import * as React from 'react';
+import { flushSync } from 'react-dom';
 import type { OnStartMove } from '../interface';
 import { getIndex } from '../util';
 import type { HandleProps } from './Handle';
@@ -27,6 +28,7 @@ export interface HandlesProps {
 
 export interface HandlesRef {
   focus: (index: number) => void;
+  hideHelp: VoidFunction;
 }
 
 const Handles = React.forwardRef<HandlesRef, HandlesProps>((props, ref) => {
@@ -45,24 +47,36 @@ const Handles = React.forwardRef<HandlesRef, HandlesProps>((props, ref) => {
   } = props;
   const handlesRef = React.useRef<Record<number, HTMLDivElement>>({});
 
-  React.useImperativeHandle(ref, () => ({
-    focus: (index: number) => {
-      handlesRef.current[index]?.focus();
-    },
-  }));
-
   // =========================== Active ===========================
-  const [activeIndex, setActiveIndex] = React.useState<number>(-1);
+  const [activeVisible, setActiveVisible] = React.useState(false);
+  const [activeIndex, setActiveIndex] = React.useState(-1);
 
-  const onHandleFocus = (e: React.FocusEvent<HTMLDivElement>, index: number) => {
+  const onActive = (index: number) => {
     setActiveIndex(index);
+    setActiveVisible(true);
+  };
+
+  const onHandleFocus = (e: React.FocusEvent<HTMLDivElement>, index: number) => {
+    onActive(index);
     onFocus?.(e);
   };
 
   const onHandleMouseEnter = (e: React.MouseEvent<HTMLDivElement>, index: number) => {
-    setActiveIndex(index);
+    onActive(index);
   };
 
+  // =========================== Render ===========================
+  React.useImperativeHandle(ref, () => ({
+    focus: (index: number) => {
+      handlesRef.current[index]?.focus();
+    },
+    hideHelp: () => {
+      flushSync(() => {
+        setActiveVisible(false);
+      });
+    },
+  }));
+
   // =========================== Render ===========================
   // Handle Props
   const handleProps = {
@@ -101,7 +115,7 @@ const Handles = React.forwardRef<HandlesRef, HandlesProps>((props, ref) => {
       })}
 
       {/* Used for render tooltip, this is not a real handle */}
-      {activeHandleRender && (
+      {activeHandleRender && activeVisible && (
         <Handle
           key="a11y"
           {...handleProps}

diff --git a/src/Slider.tsx b/src/Slider.tsx
@@ -296,7 +296,12 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
     setValue(cloneNextValues);
   });
 
-  const finishChange = useEvent(() => {
+  const finishChange = useEvent((draggingDelete?: boolean) => {
+    // Trigger from `useDrag` will tell if it's a delete action
+    if (draggingDelete) {
+      handlesRef.current.hideHelp();
+    }
+
     const finishValue = getTriggerValue(rawValues);
     onAfterChange?.(finishValue);
     warning(
@@ -315,6 +320,7 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
       triggerChange(cloneNextValues);
 
       const nextFocusIndex = Math.max(0, index - 1);
+      handlesRef.current.hideHelp();
       handlesRef.current.focus(nextFocusIndex);
     }
   };

diff --git a/src/hooks/useDrag.ts b/src/hooks/useDrag.ts
@@ -20,7 +20,7 @@ function useDrag(
   max: number,
   formatValue: (value: number) => number,
   triggerChange: (values: number[]) => void,
-  finishChange: () => void,
+  finishChange: (draggingDelete: boolean) => void,
   offsetValues: OffsetValues,
   editable: boolean,
 ): [
@@ -121,6 +121,9 @@ function useDrag(
 
     const { pageX: startX, pageY: startY } = getPosition(e);
 
+    // We declare it here since closure can't get outer latest value
+    let deleteMark = false;
+
     // Moving
     const onMouseMove = (event: MouseEvent | TouchEvent) => {
       event.preventDefault();
@@ -156,7 +159,7 @@ function useDrag(
       }
 
       // Check if need mark remove
-      const deleteMark = editable ? Math.abs(removeDist) > REMOVE_DIST : false;
+      deleteMark = editable ? Math.abs(removeDist) > REMOVE_DIST : false;
       setDraggingDelete(deleteMark);
 
       updateCacheValue(valueIndex, offSetPercent, deleteMark);
@@ -173,8 +176,10 @@ function useDrag(
       mouseMoveEventRef.current = null;
       mouseUpEventRef.current = null;
 
+      finishChange(deleteMark);
+
       setDraggingIndex(-1);
-      finishChange();
+      setDraggingDelete(false);
     };
 
     document.addEventListener('mouseup', onMouseUp);

diff --git a/tests/Range.test.tsx b/tests/Range.test.tsx
@@ -26,15 +26,17 @@ describe('Range', () => {
   });
 
   function doMouseDown(container: HTMLElement, start: number, element = 'rc-slider-handle') {
-    const mouseDown = createEvent.mouseDown(container.getElementsByClassName(element)[0]);
+    const ele = container.getElementsByClassName(element)[0];
+    const mouseDown = createEvent.mouseDown(ele);
     (mouseDown as any).pageX = start;
     (mouseDown as any).pageY = start;
     Object.defineProperties(mouseDown, {
       clientX: { get: () => start },
       clientY: { get: () => start },
     });
 
-    fireEvent(container.getElementsByClassName(element)[0], mouseDown);
+    fireEvent.mouseEnter(ele);
+    fireEvent(ele, mouseDown);
   }
 
   function doMouseMove(
@@ -709,14 +711,31 @@ describe('Range', () => {
           max={100}
           defaultValue={[0, 50, 100]}
           range={{ editable: true }}
+          // Test for active handle render
+          activeHandleRender={(ori) => ori}
         />,
       );
 
-      fireEvent.keyDown(container.querySelectorAll('.rc-slider-handle')[1], {
+      const handle = container.querySelectorAll('.rc-slider-handle')[1];
+
+      fireEvent.mouseEnter(handle);
+      fireEvent.keyDown(handle, {
         keyCode: keyCode.DELETE,
       });
 
       expect(onChange).toHaveBeenCalledWith([0, 100]);
+
+      // Clear all
+      fireEvent.keyDown(container.querySelector('.rc-slider-handle'), {
+        keyCode: keyCode.DELETE,
+      });
+      fireEvent.keyDown(container.querySelector('.rc-slider-handle'), {
+        keyCode: keyCode.DELETE,
+      });
+      expect(onChange).toHaveBeenCalledWith([]);
+
+      // 2 handle
+      expect(container.querySelectorAll('.rc-slider-handle')).toHaveLength(0);
     });
   });
 });
diff --git a/tests/Tooltip.test.js b/tests/Tooltip.test.js
@@ -17,10 +17,10 @@ describe('Slider.Tooltip', () => {
         }
       />,
     );
-    expect(container.querySelector('.rc-slider-handle[data-test]')).toBeTruthy();
 
     // Click second
     fireEvent.mouseEnter(container.querySelectorAll('.rc-slider-handle')[1]);
+    expect(container.querySelector('.rc-slider-handle[data-test]')).toBeTruthy();
     expect(
       container.querySelector('.rc-slider-handle[data-value]').getAttribute('data-value'),
     ).toBe('50');