Skip to content

Commit 417e9af

Browse files
authored
fix: Tooltip logic (#1008)
* fix: remove logic * test: add test case
1 parent f2206cf commit 417e9af

5 files changed

Lines changed: 62 additions & 18 deletions

File tree

src/Handles/index.tsx

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as React from 'react';
2+
import { flushSync } from 'react-dom';
23
import type { OnStartMove } from '../interface';
34
import { getIndex } from '../util';
45
import type { HandleProps } from './Handle';
@@ -27,6 +28,7 @@ export interface HandlesProps {
2728

2829
export interface HandlesRef {
2930
focus: (index: number) => void;
31+
hideHelp: VoidFunction;
3032
}
3133

3234
const Handles = React.forwardRef<HandlesRef, HandlesProps>((props, ref) => {
@@ -45,24 +47,36 @@ const Handles = React.forwardRef<HandlesRef, HandlesProps>((props, ref) => {
4547
} = props;
4648
const handlesRef = React.useRef<Record<number, HTMLDivElement>>({});
4749

48-
React.useImperativeHandle(ref, () => ({
49-
focus: (index: number) => {
50-
handlesRef.current[index]?.focus();
51-
},
52-
}));
53-
5450
// =========================== Active ===========================
55-
const [activeIndex, setActiveIndex] = React.useState<number>(-1);
51+
const [activeVisible, setActiveVisible] = React.useState(false);
52+
const [activeIndex, setActiveIndex] = React.useState(-1);
5653

57-
const onHandleFocus = (e: React.FocusEvent<HTMLDivElement>, index: number) => {
54+
const onActive = (index: number) => {
5855
setActiveIndex(index);
56+
setActiveVisible(true);
57+
};
58+
59+
const onHandleFocus = (e: React.FocusEvent<HTMLDivElement>, index: number) => {
60+
onActive(index);
5961
onFocus?.(e);
6062
};
6163

6264
const onHandleMouseEnter = (e: React.MouseEvent<HTMLDivElement>, index: number) => {
63-
setActiveIndex(index);
65+
onActive(index);
6466
};
6567

68+
// =========================== Render ===========================
69+
React.useImperativeHandle(ref, () => ({
70+
focus: (index: number) => {
71+
handlesRef.current[index]?.focus();
72+
},
73+
hideHelp: () => {
74+
flushSync(() => {
75+
setActiveVisible(false);
76+
});
77+
},
78+
}));
79+
6680
// =========================== Render ===========================
6781
// Handle Props
6882
const handleProps = {
@@ -101,7 +115,7 @@ const Handles = React.forwardRef<HandlesRef, HandlesProps>((props, ref) => {
101115
})}
102116

103117
{/* Used for render tooltip, this is not a real handle */}
104-
{activeHandleRender && (
118+
{activeHandleRender && activeVisible && (
105119
<Handle
106120
key="a11y"
107121
{...handleProps}

src/Slider.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,12 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
296296
setValue(cloneNextValues);
297297
});
298298

299-
const finishChange = useEvent(() => {
299+
const finishChange = useEvent((draggingDelete?: boolean) => {
300+
// Trigger from `useDrag` will tell if it's a delete action
301+
if (draggingDelete) {
302+
handlesRef.current.hideHelp();
303+
}
304+
300305
const finishValue = getTriggerValue(rawValues);
301306
onAfterChange?.(finishValue);
302307
warning(
@@ -315,6 +320,7 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
315320
triggerChange(cloneNextValues);
316321

317322
const nextFocusIndex = Math.max(0, index - 1);
323+
handlesRef.current.hideHelp();
318324
handlesRef.current.focus(nextFocusIndex);
319325
}
320326
};

src/hooks/useDrag.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ function useDrag(
2020
max: number,
2121
formatValue: (value: number) => number,
2222
triggerChange: (values: number[]) => void,
23-
finishChange: () => void,
23+
finishChange: (draggingDelete: boolean) => void,
2424
offsetValues: OffsetValues,
2525
editable: boolean,
2626
): [
@@ -121,6 +121,9 @@ function useDrag(
121121

122122
const { pageX: startX, pageY: startY } = getPosition(e);
123123

124+
// We declare it here since closure can't get outer latest value
125+
let deleteMark = false;
126+
124127
// Moving
125128
const onMouseMove = (event: MouseEvent | TouchEvent) => {
126129
event.preventDefault();
@@ -156,7 +159,7 @@ function useDrag(
156159
}
157160

158161
// Check if need mark remove
159-
const deleteMark = editable ? Math.abs(removeDist) > REMOVE_DIST : false;
162+
deleteMark = editable ? Math.abs(removeDist) > REMOVE_DIST : false;
160163
setDraggingDelete(deleteMark);
161164

162165
updateCacheValue(valueIndex, offSetPercent, deleteMark);
@@ -173,8 +176,10 @@ function useDrag(
173176
mouseMoveEventRef.current = null;
174177
mouseUpEventRef.current = null;
175178

179+
finishChange(deleteMark);
180+
176181
setDraggingIndex(-1);
177-
finishChange();
182+
setDraggingDelete(false);
178183
};
179184

180185
document.addEventListener('mouseup', onMouseUp);

tests/Range.test.tsx

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,17 @@ describe('Range', () => {
2626
});
2727

2828
function doMouseDown(container: HTMLElement, start: number, element = 'rc-slider-handle') {
29-
const mouseDown = createEvent.mouseDown(container.getElementsByClassName(element)[0]);
29+
const ele = container.getElementsByClassName(element)[0];
30+
const mouseDown = createEvent.mouseDown(ele);
3031
(mouseDown as any).pageX = start;
3132
(mouseDown as any).pageY = start;
3233
Object.defineProperties(mouseDown, {
3334
clientX: { get: () => start },
3435
clientY: { get: () => start },
3536
});
3637

37-
fireEvent(container.getElementsByClassName(element)[0], mouseDown);
38+
fireEvent.mouseEnter(ele);
39+
fireEvent(ele, mouseDown);
3840
}
3941

4042
function doMouseMove(
@@ -709,14 +711,31 @@ describe('Range', () => {
709711
max={100}
710712
defaultValue={[0, 50, 100]}
711713
range={{ editable: true }}
714+
// Test for active handle render
715+
activeHandleRender={(ori) => ori}
712716
/>,
713717
);
714718

715-
fireEvent.keyDown(container.querySelectorAll('.rc-slider-handle')[1], {
719+
const handle = container.querySelectorAll('.rc-slider-handle')[1];
720+
721+
fireEvent.mouseEnter(handle);
722+
fireEvent.keyDown(handle, {
716723
keyCode: keyCode.DELETE,
717724
});
718725

719726
expect(onChange).toHaveBeenCalledWith([0, 100]);
727+
728+
// Clear all
729+
fireEvent.keyDown(container.querySelector('.rc-slider-handle'), {
730+
keyCode: keyCode.DELETE,
731+
});
732+
fireEvent.keyDown(container.querySelector('.rc-slider-handle'), {
733+
keyCode: keyCode.DELETE,
734+
});
735+
expect(onChange).toHaveBeenCalledWith([]);
736+
737+
// 2 handle
738+
expect(container.querySelectorAll('.rc-slider-handle')).toHaveLength(0);
720739
});
721740
});
722741
});

tests/Tooltip.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ describe('Slider.Tooltip', () => {
1717
}
1818
/>,
1919
);
20-
expect(container.querySelector('.rc-slider-handle[data-test]')).toBeTruthy();
2120

2221
// Click second
2322
fireEvent.mouseEnter(container.querySelectorAll('.rc-slider-handle')[1]);
23+
expect(container.querySelector('.rc-slider-handle[data-test]')).toBeTruthy();
2424
expect(
2525
container.querySelector('.rc-slider-handle[data-value]').getAttribute('data-value'),
2626
).toBe('50');

0 commit comments

Comments
 (0)