feat: TimePicker always show correct position when open

Author: zombieJ

src/PanelContext.tsx

@@ -17,6 +17,7 @@ export interface PanelContextProps {
   onDateMouseLeave?: (date: any) => void;
   onSelect?: OnSelect<any>;
   hideRanges?: boolean;
+  open?: boolean;
 }
 
 const PanelContext = React.createContext<PanelContextProps>({});

src/Picker.tsx

@@ -401,6 +401,7 @@ function InnerPicker<DateType>(props: PickerProps<DateType>) {
         hideHeader: picker === 'time',
         panelRef: panelDivRef,
         onSelect: onContextSelect,
+        open: mergedOpen,
       }}
     >
       <PickerTrigger

src/RangePicker.tsx

@@ -414,8 +414,6 @@ function InnerRangePicker<DateType>(props: RangePickerProps<DateType>) {
     const canTrigger =
       values === null || (canStartValueTrigger && canEndValueTrigger);
 
-    console.log('>>>', canTrigger);
-
     if (canTrigger) {
       // Trigger onChange only when value is validate
       setInnerValue(values);
@@ -942,6 +940,7 @@ function InnerRangePicker<DateType>(props: RangePickerProps<DateType>) {
         onDateMouseLeave,
         hideRanges: true,
         onSelect: onContextSelect,
+        open: mergedOpen,
       }}
     >
       <PickerTrigger

src/panels/TimePanel/TimeUnitColumn.tsx

@@ -1,6 +1,7 @@
 import * as React from 'react';
 import classNames from 'classnames';
 import { scrollTo } from '../../utils/uiUtil';
+import PanelContext from '../../PanelContext';
 
 export interface Unit {
   label: React.ReactText;
@@ -27,21 +28,28 @@ function TimeUnitColumn(props: TimeUnitColumnProps) {
     hideDisabledOptions,
   } = props;
   const cellPrefixCls = `${prefixCls}-cell`;
+  const { open } = React.useContext(PanelContext);
 
-  const initRef = React.useRef(true);
   const ulRef = React.useRef<HTMLUListElement>(null);
   const liRefs = React.useRef<Map<number, HTMLElement | null>>(new Map());
 
   // `useLayoutEffect` here to avoid blink by duration is 0
   React.useLayoutEffect(() => {
     const li = liRefs.current.get(value!);
-    if (li) {
-      scrollTo(ulRef.current!, li.offsetTop, initRef.current ? 0 : 120);
+    if (li && open !== false) {
+      scrollTo(ulRef.current!, li.offsetTop, 120);
     }
-
-    initRef.current = false;
   }, [value]);
 
+  React.useLayoutEffect(() => {
+    if (open) {
+      const li = liRefs.current.get(value!);
+      if (li) {
+        scrollTo(ulRef.current!, li.offsetTop, 0);
+      }
+    }
+  }, [open]);
+
   return (
     <ul
       className={classNames(`${prefixCls}-column`, {

src/utils/uiUtil.ts

@@ -1,25 +1,37 @@
 import KeyCode from 'rc-util/lib/KeyCode';
 import { PanelMode, PickerMode } from '../interface';
 
+const scrollIds = new Map<HTMLElement, number>();
+
 /* eslint-disable no-param-reassign */
 export function scrollTo(element: HTMLElement, to: number, duration: number) {
+  if (scrollIds.get(element)) {
+    cancelAnimationFrame(scrollIds.get(element)!);
+  }
+
   // jump to target if duration zero
   if (duration <= 0) {
-    requestAnimationFrame(() => {
-      element.scrollTop = to;
-    });
+    scrollIds.set(
+      element,
+      requestAnimationFrame(() => {
+        element.scrollTop = to;
+      }),
+    );
 
     return;
   }
   const difference = to - element.scrollTop;
   const perTick = (difference / duration) * 10;
 
-  requestAnimationFrame(() => {
-    element.scrollTop += perTick;
-    if (element.scrollTop !== to) {
-      scrollTo(element, to, duration - 10);
-    }
-  });
+  scrollIds.set(
+    element,
+    requestAnimationFrame(() => {
+      element.scrollTop += perTick;
+      if (element.scrollTop !== to) {
+        scrollTo(element, to, duration - 10);
+      }
+    }),
+  );
 }
 /* eslint-enable */
 

tests/panel.spec.tsx

@@ -92,52 +92,57 @@ describe('Picker.Panel', () => {
   });
 
   describe('time click to scroll', () => {
-    let domSpy: ReturnType<typeof spyElementPrototypes>;
-    const requestAnimationFrameSpy = jest.spyOn(
-      global,
-      'requestAnimationFrame' as any,
-    );
-
-    beforeEach(() => {
-      let scrollTop = 90;
+    [true, false].forEach(bool => {
+      it(`spy requestAnimationFrame: ${bool}`, () => {
+        let scrollTop = 90;
+        const domSpy = spyElementPrototypes(HTMLElement, {
+          scrollTop: {
+            get: () => scrollTop,
+            set: ((_: Function, value: number) => {
+              scrollTop = value;
+            }) as any,
+          },
+        });
+
+        let requestAnimationFrameSpy = jest.spyOn(
+          global,
+          'requestAnimationFrame' as any,
+        );
 
-      domSpy = spyElementPrototypes(HTMLElement, {
-        scrollTop: {
-          get: () => scrollTop,
-          set: ((_: Function, value: number) => {
-            scrollTop = value;
-          }) as any,
-        },
+        // Spy to trigger 2 way of test for checking case cover
+        if (bool) {
+          requestAnimationFrameSpy = requestAnimationFrameSpy.mockImplementation(
+            window.setTimeout as any,
+          );
+        }
+
+        jest.useFakeTimers();
+        const wrapper = mount(<MomentPickerPanel picker="time" />);
+
+        // Multiple times should only one work
+        wrapper
+          .find('ul')
+          .first()
+          .find('li')
+          .at(3)
+          .simulate('click');
+
+        wrapper
+          .find('ul')
+          .first()
+          .find('li')
+          .at(11)
+          .simulate('click');
+        jest.runAllTimers();
+
+        expect(requestAnimationFrameSpy).toHaveBeenCalled();
+
+        jest.useRealTimers();
+
+        domSpy.mockRestore();
+        requestAnimationFrameSpy.mockRestore();
       });
     });
-    afterEach(() => {
-      domSpy.mockRestore();
-    });
-
-    it('scroll', () => {
-      jest.useFakeTimers();
-      const wrapper = mount(<MomentPickerPanel picker="time" />);
-
-      // Multiple times should only one work
-      wrapper
-        .find('ul')
-        .first()
-        .find('li')
-        .at(3)
-        .simulate('click');
-
-      wrapper
-        .find('ul')
-        .first()
-        .find('li')
-        .at(11)
-        .simulate('click');
-      jest.runAllTimers();
-
-      expect(requestAnimationFrameSpy).toHaveBeenCalled();
-
-      jest.useRealTimers();
-    });
   });
 
   describe('click button to switch', () => {