[NoJira][BpkSlider] - Fix native event handling for the Slider

packages/bpk-component-slider/README.md

@@ -28,3 +28,9 @@ const Slider = () => (
 ## Props
 
 Check out the full list of props on Skyscanner's [design system documentation website](https://www.skyscanner.design/latest/components/slider/web-aNXvlY7y#section-props-1e).
+
+
+## Native Events
+
+Just like a `input` `type="range"` the BpkSlider will fire a change event from the hidden `input` `type="number"` for each thumb. These behave similarly where user can drag the thumb and will fire a change event on `mouseup`/`click`.
+As for the keyboard events the change event will fire on `keyup` rather than on every keystroke registered like the `input` `type=range` does.

packages/bpk-component-slider/src/BpkSlider.tsx

@@ -20,11 +20,11 @@ import {
   useRef,
   useEffect,
   type ComponentPropsWithRef,
+  type RefObject,
 } from 'react';
 
 import { useComposedRefs } from '@radix-ui/react-compose-refs';
 import * as Slider from '@radix-ui/react-slider';
-import { usePrevious } from '@radix-ui/react-use-previous';
 
 import { cssModules, isRTL, setNativeValue } from '../../bpk-react-utils';
 
@@ -42,7 +42,9 @@ export type Props = {
   value: number[] | number;
   ariaLabel: string[];
   ariaValuetext?: string[];
-  inputProps?: [{ [key: string]: any }];
+  inputProps?:
+    | [{ [key: string]: any }]
+    | [{ [key: string]: any }, { [key: string]: any }];
   [rest: string]: any;
 };
 
@@ -72,6 +74,8 @@ const BpkSlider = ({
     }
   };
 
+  const thumbRefs = [useRef(null), useRef(null)];
+
   const handleOnChange = (sliderValues: number[]) => {
     processSliderValues(sliderValues, onChange);
   };
@@ -103,11 +107,16 @@ const BpkSlider = ({
           aria-valuetext={ariaValuetext ? ariaValuetext[index] : val.toString()}
           className={getClassName('bpk-slider__thumb')}
           aria-valuenow={currentValue[index]}
+          ref={thumbRefs[index]}
           asChild
         >
           {/* custom thumb with child input */}
           <span>
-            <BubbleInput value={currentValue[index]} {...(inputProps && inputProps[index])} />
+            <BubbleInput
+              value={currentValue[index]}
+              thumbRef={thumbRefs[index]}
+              {...(inputProps && inputProps[index])}
+            />
           </span>
         </Slider.Thumb>
       ))}
@@ -117,20 +126,67 @@ const BpkSlider = ({
 
 // Work around until radix-ui/react-slider is updated to accept an inputRef prop https://github.com/radix-ui/primitives/pull/3033
 const BubbleInput = forwardRef(
-  (props: ComponentPropsWithRef<'input'>, forwardedRef) => {
-    const { value, ...inputProps } = props;
+  (
+    props: ComponentPropsWithRef<'input'> & {
+      thumbRef: RefObject<HTMLElement>;
+    },
+    forwardedRef,
+  ) => {
+    const { thumbRef, value, ...inputProps } = props;
     const ref = useRef<HTMLInputElement>();
     const composedRefs = useComposedRefs(forwardedRef, ref);
 
-    const prevValue = usePrevious(value);
-
-    // Bubble value change to parents (e.g form change event)
+    // Provide native behaviour that the input range type provides.
     useEffect(() => {
-      const input = ref.current!;
-      if (prevValue !== value) {
-        setNativeValue(input, `${value}`);
+      const thumb = thumbRef.current;
+      const input = ref.current;
+      if (thumb) {
+        const interactionEndHandler = (prevValue: number) => {
+          if (input) {
+            const newValue = parseFloat(input.getAttribute('value') as string);
+            if (prevValue !== newValue) {
+              setNativeValue(input, newValue);
+            }
+          }
+        };
+
+        const focusInHandler = () => {
+          // Controller needed as we may click more than once when in focus. On focusout we can then abort the event listeners
+          const controller = new AbortController();
+          thumb.addEventListener('focusout', () => controller.abort(), {
+            once: true,
+          });
+          // needed on document as users can drag the thumb while out of the thumb elements mouse area
+          document.addEventListener(
+            'click',
+            () => interactionEndHandler(value as number),
+            {
+              signal: controller.signal,
+            },
+          );
+          thumb.addEventListener(
+            'keyup',
+            (e: KeyboardEvent) => {
+              ['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown'].includes(
+                e.key,
+              ) && interactionEndHandler(value as number);
+            },
+            {
+              signal: controller.signal,
+            },
+          );
+        };
+
+        thumb.addEventListener('focusin', focusInHandler);
+
+        return () => {
+          if (thumb) {
+            thumb.removeEventListener('focusin', focusInHandler);
+          }
+        };
       }
-    }, [prevValue, value]);
+      return () => {};
+    }, [thumbRef, ref, value]);
 
     /**
      * We purposefully do not use `type="hidden"` here otherwise forms that

packages/bpk-component-slider/src/form-test.tsx

@@ -16,7 +16,7 @@
  * limitations under the License.
  */
 
-import { useState } from 'react';
+import { useEffect, useState } from 'react';
 
 import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
@@ -34,7 +34,7 @@ window.ResizeObserver =
 describe('BpkSlider form-test for single-thumb slider', () => {
   it('should work as a form component in a form', async () => {
     const Wrap = () => {
-      const [sliderValue, setSliderValue] = useState<number>(50);  // single-thumb slider with only max value
+      const [sliderValue, setSliderValue] = useState<number>(50); // single-thumb slider with only max value
       return (
         <form data-testid="form">
           <BpkSlider
@@ -67,9 +67,12 @@ describe('BpkSlider form-test for single-thumb slider', () => {
   });
 
   it('should emit change event when both sides of slider value are changed for two-thumb slider', async () => {
-    const handleChange = jest.fn();
+    const formValidation = jest.fn();
     const Wrap = () => {
-      const [sliderValue, setSliderValue] = useState<number[]>([0, 100]);  // two-thumb slider with min and max value
+      const [sliderValue, setSliderValue] = useState<number[]>([0, 100]); // two-thumb slider with min and max value
+      useEffect(() => {
+        document.addEventListener('change', formValidation);
+      }, []);
       return (
         <form data-testid="form">
           <BpkSlider
@@ -80,8 +83,8 @@ describe('BpkSlider form-test for single-thumb slider', () => {
             ariaLabel={['min', 'max']}
             ariaValuetext={['0', '100']}
             value={sliderValue}
+            inputProps={[{ 'data-testid': 'sliderInputMin' }, { 'data-testid': 'sliderInputMax' }]}
             onChange={(value) => {
-              handleChange(value as number[]);
               setSliderValue(value as number[]);
             }}
           />
@@ -94,16 +97,20 @@ describe('BpkSlider form-test for single-thumb slider', () => {
     expect(sliderThumbMin).toBeInTheDocument();
 
     await sliderThumbMin.focus();
-    await userEvent.keyboard('{ArrowRight}');
-    expect(handleChange).toHaveBeenCalledTimes(1);
-    expect(handleChange).toHaveBeenCalledWith([1, 100]);
+    // Ideally we could use the mouse and drag but not currently possible in jsdom
+    // Moving the slider over multiple steps should result in one change event
+    await userEvent.keyboard('{ArrowRight>5}{/ArrowRight}');
+
+    expect(screen.getByTestId('sliderInputMin').getAttribute('value')).toBe('5');
+    expect(formValidation).toHaveBeenCalledTimes(1);
 
     const sliderThumbMax = screen.getByLabelText('max');
     expect(sliderThumbMax).toBeInTheDocument();
 
     await sliderThumbMax.focus();
-    await userEvent.keyboard('{ArrowLeft}');
-    expect(handleChange).toHaveBeenCalledTimes(2);
-    expect(handleChange).toHaveBeenCalledWith([1, 99]);
+    await userEvent.keyboard('{ArrowLeft>25}{/ArrowLeft}');
+
+    expect(screen.getByTestId('sliderInputMax').getAttribute('value')).toBe('75');
+    expect(formValidation).toHaveBeenCalledTimes(2);
   });
 });

packages/package.json

@@ -26,7 +26,6 @@
     "@popperjs/core": "^2.11.8",
     "@radix-ui/react-compose-refs": "^1.1.0",
     "@radix-ui/react-slider": "^1.1.2",
-    "@radix-ui/react-use-previous": "^1.1.0",
     "@react-google-maps/api": "^2.19.3",
     "@skyscanner/bpk-foundations-web": "^18.1.0",
     "@skyscanner/bpk-svgs": "^19.3.0",