fix: Range Slider Mobile UX (#1909)

Author: dbchristopher

packages/components/src/Form/Inputs/RangeSlider/RangeSlider.test.tsx

@@ -30,6 +30,7 @@ import { renderWithTheme } from '@looker/components-test-utils'
 import { RangeSlider } from './RangeSlider'
 
 const globalConsole = global.console
+const globalRequestAnimationFrame = global.requestAnimationFrame
 /* eslint-disable-next-line @typescript-eslint/unbound-method */
 const globalGetBoundingClientRect = Element.prototype.getBoundingClientRect
 
@@ -39,6 +40,9 @@ beforeEach(() => {
     error: jest.fn(),
     warn: jest.fn(),
   }
+  global.requestAnimationFrame = jest
+    .fn()
+    .mockImplementation((cb: Function) => cb())
   /* eslint-disable-next-line @typescript-eslint/unbound-method */
   Element.prototype.getBoundingClientRect = jest.fn(() => {
     return {
@@ -58,6 +62,7 @@ beforeEach(() => {
 afterEach(() => {
   jest.resetAllMocks()
   global.console = globalConsole
+  global.requestAnimationFrame = globalRequestAnimationFrame
   /* eslint-disable-next-line @typescript-eslint/unbound-method */
   Element.prototype.getBoundingClientRect = globalGetBoundingClientRect
 })
@@ -85,7 +90,7 @@ test('warns the developer if value prop falls outside of possible min/max range'
   expect(handleChange).toHaveBeenCalledWith([10, 20])
 })
 
-test('fires onChange callback when on mouseMove', () => {
+test('fires onChange callback on mouseMove', () => {
   const handleChange = jest.fn()
   const { getByTestId } = renderWithTheme(
     <RangeSlider onChange={handleChange} />
@@ -94,6 +99,22 @@ test('fires onChange callback when on mouseMove', () => {
   const wrapper = getByTestId('range-slider-wrapper')
   fireEvent.mouseDown(wrapper)
   fireEvent.mouseMove(wrapper, { clientX: 100, clientY: 10 })
+  fireEvent.mouseUp(wrapper)
+
+  expect(handleChange).toHaveBeenLastCalledWith([3, 10])
+})
+
+test('fires onChange callback on touchMove', () => {
+  const handleChange = jest.fn()
+  const { getByTestId } = renderWithTheme(
+    <RangeSlider onChange={handleChange} />
+  )
+
+  const wrapper = getByTestId('range-slider-wrapper')
+  fireEvent.touchStart(wrapper)
+  fireEvent.touchMove(wrapper, { touches: [{ clientX: 100, clientY: 10 }] })
+  fireEvent.touchEnd(wrapper)
+
   expect(handleChange).toHaveBeenLastCalledWith([3, 10])
 })
 
@@ -192,4 +213,20 @@ describe('disabled prop', () => {
     expect(handleChange).toHaveBeenLastCalledWith([0, 10]) // unchanged
     expect(handleChange).toHaveBeenCalledTimes(1)
   })
+
+  test('disabled component does not respond to TOUCH input', () => {
+    const handleChange = jest.fn()
+    const { getByTestId } = renderWithTheme(
+      <RangeSlider onChange={handleChange} disabled />
+    )
+
+    expect(handleChange).toHaveBeenCalledWith([0, 10]) // initial render
+
+    const wrapper = getByTestId('range-slider-wrapper')
+    fireEvent.touchStart(wrapper)
+    fireEvent.touchMove(wrapper, { touches: [{ clientX: 100, clientY: 10 }] })
+
+    expect(handleChange).toHaveBeenLastCalledWith([0, 10]) // unchanged
+    expect(handleChange).toHaveBeenCalledTimes(1)
+  })
 })

packages/components/src/Form/Inputs/RangeSlider/RangeSlider.tsx

@@ -47,10 +47,10 @@ import startsWith from 'lodash/startsWith'
 import partial from 'lodash/partial'
 import map from 'lodash/map'
 import isEqual from 'lodash/isEqual'
+
 import {
   useMeasuredElement,
   useMouseDragPosition,
-  usePreviousValue,
   useReadOnlyWarn,
 } from '../../../utils'
 import { ValidationType } from '../../ValidationMessage'
@@ -202,7 +202,6 @@ export const InternalRangeSlider = forwardRef(
     const containerRect = useMeasuredElement(containerRef)
 
     const { mousePos, isMouseDown } = useMouseDragPosition(containerRef)
-    const prevMouseDown = usePreviousValue(isMouseDown)
 
     const minThumbRef = useRef<HTMLDivElement>(null)
     const maxThumbRef = useRef<HTMLDivElement>(null)
@@ -285,6 +284,7 @@ export const InternalRangeSlider = forwardRef(
           newPoint,
           maintainFocus ? focusedThumb : undefined
         )
+
         focusChangedPoint(newValue, newPoint)
         setValue(newValue)
       }
@@ -298,7 +298,7 @@ export const InternalRangeSlider = forwardRef(
      */
     useEffect(
       () => {
-        if (isMouseDown && prevMouseDown) {
+        if (isMouseDown) {
           handleMouseDrag()
         }
       },
@@ -336,6 +336,8 @@ export const InternalRangeSlider = forwardRef(
       <div
         data-testid="range-slider-wrapper"
         onMouseDown={handleMouseDown}
+        onTouchStart={handleMouseDown}
+        onTouchEnd={handleBlur}
         className={className}
         id={id}
         ref={setContainerRef}
@@ -412,6 +414,8 @@ export const RangeSlider = styled(InternalRangeSlider).attrs<RangeSliderProps>(
   ${space}
   ${typography}
   padding: 1.5rem 0 0.5rem;
+  touch-action: none;
+  user-select: none;
 `
 
 const SliderTrack = styled.div`

packages/components/src/utils/useMouseDragPosition.ts

@@ -26,6 +26,7 @@
 
 import { useState, useEffect } from 'react'
 import throttle from 'lodash/throttle'
+import get from 'lodash/get'
 
 interface MouseState {
   mousePos: { x: number; y: number }
@@ -42,43 +43,56 @@ export function useMouseDragPosition(
   const [isMouseDown, setIsMouseDown] = useState(false)
   const [mousePos, setMousePos] = useState({ x: 0, y: 0 })
 
-  const handleMouseDown = (e: globalThis.MouseEvent) => {
-    setMousePos({ x: e.pageX, y: e.pageY })
-    setIsMouseDown(true)
+  const updateMousePos = (e: globalThis.TouchEvent | globalThis.MouseEvent) => {
+    // use e.touches[0] for touch events, or simply e for mouse events
+    const event = get(e, 'touches[0]', e)
+    // e.clientX and e.clientY fallbacks are included for testing purposes. jsDOM doesn't support pageX/pageY attributes
+    const { pageX, clientX, pageY, clientY } = event
+    setMousePos({ x: pageX || clientX, y: pageY || clientY })
   }
 
-  const handleMouseUp = () => {
-    setIsMouseDown(false)
+  const handleStart = (e: globalThis.TouchEvent | globalThis.MouseEvent) => {
+    // update mouse down state AFTER mouse position state is updated
+    requestAnimationFrame(() => {
+      setIsMouseDown(true)
+    })
+    updateMousePos(e)
   }
 
-  const handleMouseMove = throttle((e: globalThis.MouseEvent) => {
+  const handleMove = throttle(updateMousePos, 50)
+
+  const handleEnd = () => {
+    requestAnimationFrame(() => {
+      setIsMouseDown(false)
+    })
+  }
+
+  useEffect(() => {
+    targetRef && targetRef.addEventListener('mousedown', handleStart)
+    targetRef && targetRef.addEventListener('touchstart', handleStart)
+    window.addEventListener('mouseup', handleEnd)
+    window.addEventListener('touchend', handleEnd)
+
     if (isMouseDown) {
-      // e.clientX and e.clientY fallbacks are included for testing purposes. jsDOM doesn't support pageX/pageY attributes
-      setMousePos({ x: e.pageX || e.clientX, y: e.pageY || e.clientY })
+      window.addEventListener('touchmove', handleMove)
+      window.addEventListener('mousemove', handleMove)
+      window.addEventListener('mouseleave', handleEnd)
     }
-  }, 50)
 
-  const handleMouseLeave = () => {
-    setIsMouseDown(false)
-  }
+    return () => {
+      targetRef && targetRef.removeEventListener('mousedown', handleStart)
+      targetRef && targetRef.removeEventListener('touchstart', handleStart)
+      window.removeEventListener('mouseup', handleEnd)
+      window.removeEventListener('touchend', handleEnd)
 
-  useEffect(
-    () => {
-      targetRef && targetRef.addEventListener('mousedown', handleMouseDown)
-      window.addEventListener('mouseup', handleMouseUp)
-      window.addEventListener('mousemove', handleMouseMove)
-      window.addEventListener('handleMouseLeave', handleMouseLeave)
-
-      return () => {
-        targetRef && targetRef.removeEventListener('mousedown', handleMouseDown)
-        window.removeEventListener('mouseup', handleMouseUp)
-        window.removeEventListener('mousemove', handleMouseMove)
-        window.removeEventListener('handleMouseLeave', handleMouseLeave)
+      if (isMouseDown) {
+        window.removeEventListener('touchmove', handleMove)
+        window.removeEventListener('mousemove', handleMove)
+        window.removeEventListener('mouseleave', handleEnd)
       }
-    },
+    }
     // eslint-disable-next-line react-hooks/exhaustive-deps
-    [isMouseDown, targetRef]
-  )
+  }, [isMouseDown, targetRef])
 
   return { isMouseDown, mousePos: mousePos }
 }