fix: Ref was not being passed through for Picker (#2122)

- Pass through ref to SpectrumPicker correctly
- Added a `useMultiRef` hook for handling multiple refs being passed in

Author: mofojed

packages/components/src/spectrum/comboBox/ComboBox.tsx

@@ -3,26 +3,32 @@ import {
   ComboBox as SpectrumComboBox,
   SpectrumComboBoxProps,
 } from '@adobe/react-spectrum';
-import type { FocusableRef } from '@react-types/shared';
+import type { DOMRef } from '@react-types/shared';
 import cl from 'classnames';
 import type { NormalizedItem } from '../utils';
 import { PickerPropsT, usePickerProps } from '../picker';
+import useMultiRef from '../picker/useMultiRef';
 
 export type ComboBoxProps = PickerPropsT<SpectrumComboBoxProps<NormalizedItem>>;
 
 export const ComboBox = React.forwardRef(function ComboBox(
   { UNSAFE_className, ...props }: ComboBoxProps,
-  ref: FocusableRef<HTMLElement>
+  ref: DOMRef<HTMLDivElement>
 ): JSX.Element {
-  const { defaultSelectedKey, disabledKeys, selectedKey, ...comboBoxProps } =
-    usePickerProps(props);
-
+  const {
+    defaultSelectedKey,
+    disabledKeys,
+    selectedKey,
+    ref: scrollRef,
+    ...comboBoxProps
+  } = usePickerProps(props);
+  const pickerRef = useMultiRef(ref, scrollRef);
   return (
     <SpectrumComboBox
       // eslint-disable-next-line react/jsx-props-no-spreading
       {...comboBoxProps}
       UNSAFE_className={cl('dh-combobox', UNSAFE_className)}
-      ref={ref}
+      ref={pickerRef}
       // Type assertions are necessary here since Spectrum types don't account
       // for number and boolean key values even though they are valid runtime
       // values.

packages/components/src/spectrum/picker/Picker.tsx

@@ -2,9 +2,12 @@ import {
   Picker as SpectrumPicker,
   SpectrumPickerProps,
 } from '@adobe/react-spectrum';
+import type { DOMRef } from '@react-types/shared';
 import cl from 'classnames';
+import React from 'react';
 import type { NormalizedItem } from '../utils';
 import type { PickerProps } from './PickerProps';
+import useMultiRef from './useMultiRef';
 import { usePickerProps } from './usePickerProps';
 
 /**
@@ -14,17 +17,23 @@ import { usePickerProps } from './usePickerProps';
  * for the Spectrum Picker component.
  * See https://react-spectrum.adobe.com/react-spectrum/Picker.html
  */
-export function Picker({
-  UNSAFE_className,
-  ...props
-}: PickerProps): JSX.Element {
-  const { defaultSelectedKey, disabledKeys, selectedKey, ...pickerProps } =
-    usePickerProps<PickerProps, HTMLDivElement>(props);
-
+export const Picker = React.forwardRef(function Picker(
+  { UNSAFE_className, ...props }: PickerProps,
+  ref: DOMRef<HTMLDivElement>
+): JSX.Element {
+  const {
+    defaultSelectedKey,
+    disabledKeys,
+    selectedKey,
+    ref: scrollRef,
+    ...pickerProps
+  } = usePickerProps(props);
+  const pickerRef = useMultiRef(ref, scrollRef);
   return (
     <SpectrumPicker
       // eslint-disable-next-line react/jsx-props-no-spreading
       {...pickerProps}
+      ref={pickerRef}
       UNSAFE_className={cl('dh-picker', UNSAFE_className)}
       // Type assertions are necessary here since Spectrum types don't account
       // for number and boolean key values even though they are valid runtime
@@ -40,6 +49,6 @@ export function Picker({
       }
     />
   );
-}
+});
 
 export default Picker;

packages/components/src/spectrum/picker/useMultiRef.test.ts

@@ -0,0 +1,59 @@
+import { renderHook } from '@testing-library/react-hooks';
+import useMultiRef from './useMultiRef';
+
+describe('useMultiRef', () => {
+  it('should assign the ref to all refs passed in', () => {
+    const ref1 = jest.fn();
+    const ref2 = jest.fn();
+    const ref3 = jest.fn();
+    const { result } = renderHook(() => useMultiRef(ref1, ref2, ref3));
+    const multiRef = result.current;
+    const element = document.createElement('div');
+    multiRef(element);
+    expect(ref1).toHaveBeenCalledWith(element);
+    expect(ref2).toHaveBeenCalledWith(element);
+    expect(ref3).toHaveBeenCalledWith(element);
+  });
+
+  it('should assign the ref to all refs passed in with null', () => {
+    const ref1 = jest.fn();
+    const ref2 = jest.fn();
+    const ref3 = jest.fn();
+    const { result } = renderHook(() => useMultiRef(ref1, ref2, ref3));
+    const multiRef = result.current;
+    multiRef(null);
+    expect(ref1).toHaveBeenCalledWith(null);
+    expect(ref2).toHaveBeenCalledWith(null);
+    expect(ref3).toHaveBeenCalledWith(null);
+  });
+
+  it('should work with non-function refs', () => {
+    const ref1 = { current: null };
+    const ref2 = { current: null };
+    const ref3 = { current: null };
+    const { result } = renderHook(() =>
+      useMultiRef<HTMLDivElement | null>(ref1, ref2, ref3)
+    );
+    const multiRef = result.current;
+    const element = document.createElement('div');
+    multiRef(element);
+    expect(ref1.current).toBe(element);
+    expect(ref2.current).toBe(element);
+    expect(ref3.current).toBe(element);
+  });
+
+  it('should handle a mix of function and non-function refs', () => {
+    const ref1 = jest.fn();
+    const ref2 = { current: null };
+    const ref3 = jest.fn();
+    const { result } = renderHook(() =>
+      useMultiRef<HTMLDivElement | null>(ref1, ref2, ref3)
+    );
+    const multiRef = result.current;
+    const element = document.createElement('div');
+    multiRef(element);
+    expect(ref1).toHaveBeenCalledWith(element);
+    expect(ref2.current).toBe(element);
+    expect(ref3).toHaveBeenCalledWith(element);
+  });
+});

packages/components/src/spectrum/picker/useMultiRef.ts

@@ -0,0 +1,22 @@
+import { MutableRefObject, Ref, RefCallback, useCallback } from 'react';
+
+/**
+ * Takes in multiple refs and then returns one ref that can be assigned to the component.
+ * In turn all the refs passed in will be assigned when the ref returned is assigned.
+ * @param refs The refs to assign
+ */
+function useMultiRef<T>(...refs: readonly Ref<T>[]): RefCallback<T> {
+  return useCallback(newRef => {
+    refs.forEach(ref => {
+      if (typeof ref === 'function') {
+        ref(newRef);
+      } else if (ref != null) {
+        // eslint-disable-next-line no-param-reassign
+        (ref as MutableRefObject<T | null>).current = newRef;
+      }
+    });
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, refs);
+}
+
+export default useMultiRef;