Combine the useMove frequently appears with useKeyboard (#2508)

* Combine the useMove frequently appears with useKeyboard

* fix merge mistake

* Remove isPage from useMove, add modifier keys

* continue propagation for keys we don't handle

* have aria pass through the step size

Co-authored-by: Rob Snow <[email protected]>

Author: majornista

packages/@react-aria/color/src/useColorArea.ts

@@ -68,44 +68,34 @@ export function useColorArea(props: AriaColorAreaProps, state: ColorAreaState, i
   let {keyboardProps} = useKeyboard({
     onKeyDown(e) {
       // these are the cases that useMove doesn't handle
-      if (!((e.shiftKey && /^Arrow(?:Right|Left|Up|Down)$/.test(e.key)) || /^(PageUp|PageDown|Home|End)$/.test(e.key))) {
+      if (!/^(PageUp|PageDown|Home|End)$/.test(e.key)) {
+        e.continuePropagation();
         return;
       }
+      // same handling as useMove, don't need to stop propagation, useKeyboard will do that for us
+      e.preventDefault();
+      // remember to set this and unset it so that onChangeEnd is fired
       stateRef.current.setDragging(true);
-      let isPage = e.shiftKey;
       switch (e.key) {
         case 'PageUp':
-          isPage = true;
-        case 'ArrowUp':
-        case 'Up':
-          stateRef.current.incrementY(isPage);
+          stateRef.current.incrementY(stateRef.current.yChannelPageStep);
           focusedInputRef.current = inputYRef.current;
           break;
         case 'PageDown':
-          isPage = true;
-        case 'ArrowDown':
-        case 'Down':
-          stateRef.current.decrementY(isPage);
+          stateRef.current.decrementY(stateRef.current.yChannelPageStep);
           focusedInputRef.current = inputYRef.current;
           break;
         case 'Home':
-          isPage = true;
-        case 'ArrowLeft':
-        case 'Left':
-          direction === 'rtl' ? stateRef.current.incrementX(isPage) : stateRef.current.decrementX(isPage);
+          direction === 'rtl' ? stateRef.current.incrementX(stateRef.current.xChannelPageStep) : stateRef.current.decrementX(stateRef.current.xChannelPageStep);
           focusedInputRef.current = inputXRef.current;
           break;
         case 'End':
-          isPage = true;
-        case 'ArrowRight':
-        case 'Right':
-          direction === 'rtl' ? stateRef.current.decrementX(isPage) : stateRef.current.incrementX(isPage);
+          direction === 'rtl' ? stateRef.current.decrementX(stateRef.current.xChannelPageStep) : stateRef.current.incrementX(stateRef.current.xChannelPageStep);
           focusedInputRef.current = inputXRef.current;
           break;
       }
       stateRef.current.setDragging(false);
       if (focusedInputRef.current) {
-        e.preventDefault();
         focusInput(focusedInputRef.current ? focusedInputRef : inputXRef);
         focusedInputRef.current = undefined;
       }
@@ -117,24 +107,26 @@ export function useColorArea(props: AriaColorAreaProps, state: ColorAreaState, i
       currentPosition.current = null;
       stateRef.current.setDragging(true);
     },
-    onMove({deltaX, deltaY, pointerType}) {
+    onMove({deltaX, deltaY, pointerType, shiftKey}) {
       if (currentPosition.current == null) {
         currentPosition.current = stateRef.current.getThumbPosition();
       }
       let {width, height} = containerRef.current.getBoundingClientRect();
       if (pointerType === 'keyboard') {
-        if (deltaX > 0 || deltaX < 0) {
-          stateRef.current[`${deltaX > 0 ? 'increment' : 'decrement'}X`]();
-        }
-        if (deltaY > 0 || deltaY < 0) {
-          stateRef.current[`${deltaY < 0 ? 'increment' : 'decrement'}Y`]();
+        if (deltaX > 0) {
+          stateRef.current.incrementX(shiftKey ? stateRef.current.xChannelPageStep : stateRef.current.xChannelStep);
+        } else if (deltaX < 0) {
+          stateRef.current.decrementX(shiftKey ? stateRef.current.xChannelPageStep : stateRef.current.xChannelStep);
+        } else if (deltaY > 0) {
+          stateRef.current.decrementY(shiftKey ? stateRef.current.yChannelPageStep : stateRef.current.yChannelStep);
+        } else if (deltaY < 0) {
+          stateRef.current.incrementY(shiftKey ? stateRef.current.yChannelPageStep : stateRef.current.yChannelStep);
         }
         // set the focused input based on which axis has the greater delta
         focusedInputRef.current = (deltaX !== 0 || deltaY !== 0) && Math.abs(deltaY) > Math.abs(deltaX) ? inputYRef.current : inputXRef.current;
-      }
-      currentPosition.current.x += (direction === 'rtl' ? -1 : 1) * deltaX / width ;
-      currentPosition.current.y += deltaY / height;
-      if (pointerType !== 'keyboard') {
+      } else {
+        currentPosition.current.x += (direction === 'rtl' ? -1 : 1) * deltaX / width ;
+        currentPosition.current.y += deltaY / height;
         stateRef.current.setColorFromPoint(currentPosition.current.x, currentPosition.current.y);
       }
     },
@@ -279,8 +271,6 @@ export function useColorArea(props: AriaColorAreaProps, state: ColorAreaState, i
         }
       })
   }, keyboardProps, movePropsThumb);
-  // order matters, keyboard need to finish before move so that onChangeEnd is fired last
-  // after valueRef in stately has been updated
 
 
   let xInputLabellingProps = useLabels({

packages/@react-aria/color/src/useColorWheel.ts

@@ -33,8 +33,6 @@ interface ColorWheelAria {
   inputProps: InputHTMLAttributes<HTMLInputElement>
 }
 
-const PAGE_MIN_STEP_SIZE = 6;
-
 /**
  * Provides the behavior and accessibility implementation for a color wheel component.
  * Color wheels allow users to adjust the hue of an HSL or HSB color value on a circular track.
@@ -62,22 +60,48 @@ export function useColorWheel(props: ColorWheelAriaProps, state: ColorWheelState
   stateRef.current = state;
 
   let currentPosition = useRef<{x: number, y: number}>(null);
+
+  let {keyboardProps} = useKeyboard({
+    onKeyDown(e) {
+      // these are the cases that useMove doesn't handle
+      if (!/^(PageUp|PageDown)$/.test(e.key)) {
+        e.continuePropagation();
+        return;
+      }
+      // same handling as useMove, don't need to stop propagation, useKeyboard will do that for us
+      e.preventDefault();
+      // remember to set this and unset it so that onChangeEnd is fired
+      stateRef.current.setDragging(true);
+      switch (e.key) {
+        case 'PageUp':
+          e.preventDefault();
+          state.increment(stateRef.current.pageStep);
+          break;
+        case 'PageDown':
+          e.preventDefault();
+          state.decrement(stateRef.current.pageStep);
+          break;
+      }
+      stateRef.current.setDragging(false);
+    }
+  });
+
   let moveHandler = {
     onMoveStart() {
       currentPosition.current = null;
       state.setDragging(true);
     },
-    onMove({deltaX, deltaY, pointerType}) {
+    onMove({deltaX, deltaY, pointerType, shiftKey}) {
       if (currentPosition.current == null) {
         currentPosition.current = stateRef.current.getThumbPosition(thumbRadius);
       }
       currentPosition.current.x += deltaX;
       currentPosition.current.y += deltaY;
       if (pointerType === 'keyboard') {
         if (deltaX > 0 || deltaY < 0) {
-          state.increment();
+          state.increment(shiftKey ? stateRef.current.pageStep : stateRef.current.step);
         } else if (deltaX < 0 || deltaY > 0) {
-          state.decrement();
+          state.decrement(shiftKey ? stateRef.current.pageStep : stateRef.current.step);
         }
       } else {
         stateRef.current.setHueFromPoint(currentPosition.current.x, currentPosition.current.y, thumbRadius);
@@ -169,21 +193,6 @@ export function useColorWheel(props: ColorWheelAriaProps, state: ColorWheelState
     }
   };
 
-  let {keyboardProps} = useKeyboard({
-    onKeyDown(e) {
-      switch (e.key) {
-        case 'PageUp':
-          e.preventDefault();
-          state.increment(PAGE_MIN_STEP_SIZE);
-          break;
-        case 'PageDown':
-          e.preventDefault();
-          state.decrement(PAGE_MIN_STEP_SIZE);
-          break;
-      }
-    }
-  });
-
   let trackInteractions = isDisabled ? {} : mergeProps({
     onMouseDown: (e: React.MouseEvent) => {
       if (e.button !== 0 || e.altKey || e.ctrlKey || e.metaKey) {
@@ -218,7 +227,7 @@ export function useColorWheel(props: ColorWheelAriaProps, state: ColorWheelState
     onTouchStart: (e: React.TouchEvent) => {
       onThumbDown(e.changedTouches[0].identifier);
     }
-  }, movePropsThumb, keyboardProps);
+  }, keyboardProps, movePropsThumb);
   let {x, y} = state.getThumbPosition(thumbRadius);
 
   // Provide a default aria-label if none is given

packages/@react-aria/color/test/useColorWheel.test.tsx

@@ -52,24 +52,18 @@ function ColorWheel(props: ColorWheelProps) {
 describe('useColorWheel', () => {
   let onChangeSpy = jest.fn();
 
-  afterEach(() => {
-    onChangeSpy.mockClear();
-  });
-
   beforeAll(() => {
     // @ts-ignore
-    jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => cb());
-    jest.useFakeTimers();
+    jest.useFakeTimers('modern');
   });
   afterAll(() => {
     jest.useRealTimers();
-    // @ts-ignore
-    window.requestAnimationFrame.mockRestore();
   });
 
   afterEach(() => {
     // for restoreTextSelection
     jest.runAllTimers();
+    onChangeSpy.mockClear();
   });
 
   it('sets input props', () => {

packages/@react-aria/interactions/src/useMove.ts

@@ -20,6 +20,13 @@ interface MoveResult {
   moveProps: HTMLAttributes<HTMLElement>
 }
 
+interface EventBase {
+  shiftKey: boolean,
+  ctrlKey: boolean,
+  metaKey: boolean,
+  altKey: boolean
+}
+
 /**
  * Handles move interactions across mouse, touch, and keyboard, including dragging with
  * the mouse or touch, and using the arrow keys. Normalizes behavior across browsers and
@@ -43,7 +50,7 @@ export function useMove(props: MoveEvents): MoveResult {
       disableTextSelection();
       state.current.didMove = false;
     };
-    let move = (pointerType: PointerType, deltaX: number, deltaY: number) => {
+    let move = (originalEvent: EventBase, pointerType: PointerType, deltaX: number, deltaY: number) => {
       if (deltaX === 0 && deltaY === 0) {
         return;
       }
@@ -52,36 +59,48 @@ export function useMove(props: MoveEvents): MoveResult {
         state.current.didMove = true;
         onMoveStart?.({
           type: 'movestart',
-          pointerType
+          pointerType,
+          shiftKey: originalEvent.shiftKey,
+          metaKey: originalEvent.metaKey,
+          ctrlKey: originalEvent.ctrlKey,
+          altKey: originalEvent.altKey
         });
       }
       onMove({
         type: 'move',
         pointerType,
         deltaX: deltaX,
-        deltaY: deltaY
+        deltaY: deltaY,
+        shiftKey: originalEvent.shiftKey,
+        metaKey: originalEvent.metaKey,
+        ctrlKey: originalEvent.ctrlKey,
+        altKey: originalEvent.altKey
       });
     };
-    let end = (pointerType: PointerType) => {
+    let end = (originalEvent: EventBase, pointerType: PointerType) => {
       restoreTextSelection();
       if (state.current.didMove) {
         onMoveEnd?.({
           type: 'moveend',
-          pointerType
+          pointerType,
+          shiftKey: originalEvent.shiftKey,
+          metaKey: originalEvent.metaKey,
+          ctrlKey: originalEvent.ctrlKey,
+          altKey: originalEvent.altKey
         });
       }
     };
 
     if (typeof PointerEvent === 'undefined') {
       let onMouseMove = (e: MouseEvent) => {
         if (e.button === 0) {
-          move('mouse', e.pageX - state.current.lastPosition.pageX, e.pageY - state.current.lastPosition.pageY);
+          move(e, 'mouse', e.pageX - state.current.lastPosition.pageX, e.pageY - state.current.lastPosition.pageY);
           state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY};
         }
       };
       let onMouseUp = (e: MouseEvent) => {
         if (e.button === 0) {
-          end('mouse');
+          end(e, 'mouse');
           removeGlobalListener(window, 'mousemove', onMouseMove, false);
           removeGlobalListener(window, 'mouseup', onMouseUp, false);
         }
@@ -98,19 +117,17 @@ export function useMove(props: MoveEvents): MoveResult {
       };
 
       let onTouchMove = (e: TouchEvent) => {
-        // @ts-ignore
         let touch = [...e.changedTouches].findIndex(({identifier}) => identifier === state.current.id);
         if (touch >= 0) {
           let {pageX, pageY} = e.changedTouches[touch];
-          move('touch', pageX - state.current.lastPosition.pageX, pageY - state.current.lastPosition.pageY);
+          move(e, 'touch', pageX - state.current.lastPosition.pageX, pageY - state.current.lastPosition.pageY);
           state.current.lastPosition = {pageX, pageY};
         }
       };
       let onTouchEnd = (e: TouchEvent) => {
-        // @ts-ignore
         let touch = [...e.changedTouches].findIndex(({identifier}) => identifier === state.current.id);
         if (touch >= 0) {
-          end('touch');
+          end(e, 'touch');
           state.current.id = null;
           removeGlobalListener(window, 'touchmove', onTouchMove);
           removeGlobalListener(window, 'touchend', onTouchEnd);
@@ -135,22 +152,20 @@ export function useMove(props: MoveEvents): MoveResult {
     } else {
       let onPointerMove = (e: PointerEvent) => {
         if (e.pointerId === state.current.id) {
-          // @ts-ignore
-          let pointerType: PointerType = e.pointerType || 'mouse';
+          let pointerType = (e.pointerType || 'mouse') as PointerType;
 
           // Problems with PointerEvent#movementX/movementY:
           // 1. it is always 0 on macOS Safari.
           // 2. On Chrome Android, it's scaled by devicePixelRatio, but not on Chrome macOS
-          move(pointerType, e.pageX - state.current.lastPosition.pageX, e.pageY - state.current.lastPosition.pageY);
+          move(e, pointerType, e.pageX - state.current.lastPosition.pageX, e.pageY - state.current.lastPosition.pageY);
           state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY};
         }
       };
 
       let onPointerUp = (e: PointerEvent) => {
         if (e.pointerId === state.current.id) {
-            // @ts-ignore
-          let pointerType: PointerType = e.pointerType || 'mouse';
-          end(pointerType);
+          let pointerType = (e.pointerType || 'mouse') as PointerType;
+          end(e, pointerType);
           state.current.id = null;
           removeGlobalListener(window, 'pointermove', onPointerMove, false);
           removeGlobalListener(window, 'pointerup', onPointerUp, false);
@@ -172,41 +187,37 @@ export function useMove(props: MoveEvents): MoveResult {
       };
     }
 
-    let triggerKeyboardMove = (deltaX: number, deltaY: number) => {
+    let triggerKeyboardMove = (e: EventBase, deltaX: number, deltaY: number) => {
       start();
-      move('keyboard', deltaX, deltaY);
-      end('keyboard');
+      move(e, 'keyboard', deltaX, deltaY);
+      end(e, 'keyboard');
     };
 
     moveProps.onKeyDown = (e) => {
-      // don't want useMove to handle shift key + arrow events because it doesn't do anything
-      if (e.shiftKey) {
-        return;
-      }
       switch (e.key) {
         case 'Left':
         case 'ArrowLeft':
           e.preventDefault();
           e.stopPropagation();
-          triggerKeyboardMove(-1, 0);
+          triggerKeyboardMove(e, -1, 0);
           break;
         case 'Right':
         case 'ArrowRight':
           e.preventDefault();
           e.stopPropagation();
-          triggerKeyboardMove(1, 0);
+          triggerKeyboardMove(e, 1, 0);
           break;
         case 'Up':
         case 'ArrowUp':
           e.preventDefault();
           e.stopPropagation();
-          triggerKeyboardMove(0, -1);
+          triggerKeyboardMove(e, 0, -1);
           break;
         case 'Down':
         case 'ArrowDown':
           e.preventDefault();
           e.stopPropagation();
-          triggerKeyboardMove(0, 1);
+          triggerKeyboardMove(e, 0, 1);
           break;
       }
     };