RG-2288 Date Picker: scroll on mobiles#9258
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the date picker calendar to support native scrolling on mobile by moving month navigation from wheel-driven virtual scrolling to a real scroll container, and refactors the month slider to use pointer events.
Changes:
- Refactor
Monthsto render a scrollable months list and derivescrollDatefrom nativescrollTop. - Refactor
MonthSliderfrom a class (mouse events + window listeners) to a functional component using pointer events. - Update calendar CSS to make the months area scrollable, hide scrollbars, and make month names sticky; remove popup-level wheel suppression.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/date-picker/months.tsx |
Switches to native scrolling and adds logic to map between scrollTop and scrollDate, including month-buffer recentering. |
src/date-picker/month-slider.tsx |
Reworks dragging to pointer events and recalculates slider offsets based on scrollDate. |
src/date-picker/month-names.tsx |
Updates MonthSlider usage to match the new API (no pxToDate). |
src/date-picker/date-popup.tsx |
Removes wheel event suppression from the popup container. |
src/date-picker/date-picker.css |
Makes .months scrollable and .monthNames sticky; adds touch-action settings for slider. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/date-picker/months.tsx
Outdated
| if (componentRef.current) { | ||
| componentRef.current.scrollTop = initialScrollTop; | ||
| setTimeout(() => { | ||
| pauseScrollHandlingRef.current = false; | ||
| }, SCROLL_HANDLE_RESUME_DELAY); | ||
| } |
There was a problem hiding this comment.
useLayoutEffect sets scrollTop programmatically but never sets pauseScrollHandlingRef.current = true beforehand. In browsers where setting scrollTop triggers a scroll event, handleScroll can run during this adjustment and call onScroll, causing spurious state updates/loops. Consider setting pauseScrollHandlingRef.current = true before changing scrollTop, then resuming after the adjustment (e.g., via requestAnimationFrame or a timeout).
| if (componentRef.current) { | |
| componentRef.current.scrollTop = initialScrollTop; | |
| setTimeout(() => { | |
| pauseScrollHandlingRef.current = false; | |
| }, SCROLL_HANDLE_RESUME_DELAY); | |
| } | |
| if (!componentRef.current) return; | |
| pauseScrollHandlingRef.current = true; | |
| componentRef.current.scrollTop = initialScrollTop; | |
| const resumeScrollHandlingTimeout = setTimeout(() => { | |
| pauseScrollHandlingRef.current = false; | |
| }, SCROLL_HANDLE_RESUME_DELAY); | |
| return () => { | |
| clearTimeout(resumeScrollHandlingTimeout); | |
| }; |
| const onPointerDown = useCallback( | ||
| (e: PointerEvent) => { | ||
| setDragStart([e.screenY, Number(scrollDate)]); | ||
| }, | ||
| [scrollDate], | ||
| ); | ||
|
|
||
| onMouseUp = () => { | ||
| this.setState({dragging: false}); | ||
| }; | ||
| const onPointerUp = useCallback((_e: PointerEvent) => { | ||
| setDragStart(null); | ||
| }, []); | ||
|
|
||
| onMouseMove = (e: MouseEvent) => { | ||
| this.props.onScroll(linearFunction(0, Number(this.props.scrollDate), yearScrollSpeed).y(e.movementY)); | ||
| }; | ||
| const onPointerMove = useCallback( | ||
| (e: PointerEvent) => { | ||
| scheduleScroll(() => { | ||
| if (!dragStart) return; | ||
| const [startY, startDate] = dragStart; | ||
|
|
||
| render() { | ||
| let year = subYears(startOfDay(this.props.scrollDate), 1); | ||
| const years = [year]; | ||
| for (let i = 0; i <= COVERYEARS; i++) { | ||
| year = addYears(year, 1); | ||
| years.push(year); | ||
| } | ||
| const yearFraction = (e.screenY - startY) / units.calHeight; | ||
| const startDatePlusOneYear = Number(addYears(new Date(startDate), 1)); | ||
| const newScrollDate = startDate + yearFraction * (startDatePlusOneYear - startDate); | ||
| onScroll(newScrollDate); | ||
| }); | ||
| }, | ||
| [onScroll, dragStart], | ||
| ); |
There was a problem hiding this comment.
Dragging is handled via onPointerMove/onPointerUp on the button, but there’s no setPointerCapture on pointer down (and no pointercancel handling). If the pointer leaves the button while dragging, move/up events may stop firing and dragStart can get stuck, especially on mobile. Capture the pointer on pointerdown and release it on pointerup/pointercancel (and consider handling pointercancel).
src/date-picker/months.tsx
Outdated
| const handleScroll = useCallback(() => { | ||
| scheduleScroll(() => { | ||
| if (pauseScrollHandlingRef.current) return; | ||
|
|
||
| return () => { | ||
| if (current) { | ||
| current.removeEventListener('wheel', handleWheel); | ||
| const scrollTop = componentRef.current?.scrollTop; | ||
| if (scrollTop == null) return; | ||
|
|
||
| const newScrollDate = getDateFromScrollPosition(months, scrollTop); | ||
| const newScrollDateNum = Number(newScrollDate); | ||
|
|
||
| onScroll(newScrollDateNum); | ||
|
|
||
| const newScrollDateIndex = months.findLastIndex(month => newScrollDateNum >= Number(month)); | ||
| if (newScrollDateIndex !== MONTHSBACK) { | ||
| const newMonths = getMonths(newScrollDate); | ||
| const newScrollTop = getScrollTopFromDate(newMonths, newScrollDate); | ||
|
|
||
| setMonths(newMonths); | ||
| setInitialScrollTop(newScrollTop); | ||
|
|
||
| pauseScrollHandlingRef.current = true; | ||
| } | ||
| }; | ||
| }, [handleWheel]); | ||
| }); | ||
| }, [months, onScroll]); |
There was a problem hiding this comment.
The new scroll-based calendar logic in Months and pointer-drag behavior in MonthSlider introduces non-trivial interaction paths (native scroll -> onScroll mapping, month buffer recentering, pointer dragging). There are existing DatePicker tests, but none appear to cover these interactions, so regressions (e.g., incorrect scrollDate updates or broken dragging) could go unnoticed. Consider adding tests that simulate scrolling the months container and dragging the month slider and assert that onScroll is called with expected values.
No description provided.