improve ref handling

microsoft · Dec 8, 2023 · 3636fef · 3636fef
1 parent 3631af8
commit 3636fef
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 18 deletions.
diff --git a/packages/react-components/react-aria/src/activedescendant/useActiveDescendant.ts b/packages/react-components/react-aria/src/activedescendant/useActiveDescendant.ts
@@ -1,14 +1,16 @@
 import * as React from 'react';
 import { useOptionWalker } from './useOptionWalker';
-import type { ActiveDescendantOptions } from './types';
+import type { ActiveDescendantImperativeRef, ActiveDescendantOptions } from './types';
 import { ACTIVEDESCENDANT_ATTRIBUTE } from './constants';
+import { useMergedRefs } from '@fluentui/react-utilities';
 
 export function useActiveDescendant<TActiveParentElement extends HTMLElement, TListboxElement extends HTMLElement>(
   options: ActiveDescendantOptions,
 ) {
-  const { imperativeRef, matchOption } = options;
+  const { imperativeRef: imperativeRefProp, matchOption } = options;
   const activeParentRef = React.useRef<TActiveParentElement>(null);
   const { listboxRef, optionWalker } = useOptionWalker<TListboxElement>({ matchOption });
+  const imperativeRef = React.useRef<ActiveDescendantImperativeRef>();
   const getActiveDescendant = () => {
     return listboxRef.current?.querySelector<HTMLElement>(`[${ACTIVEDESCENDANT_ATTRIBUTE}]`);
   };
@@ -54,7 +56,7 @@ export function useActiveDescendant<TActiveParentElement extends HTMLElement, TL
     }
   };
 
-  React.useImperativeHandle(imperativeRef, () => ({
+  React.useImperativeHandle(useMergedRefs(imperativeRef, imperativeRefProp), () => ({
     first: () => {
       if (!listboxRef.current || !activeParentRef.current) {
         return;
@@ -134,5 +136,5 @@ export function useActiveDescendant<TActiveParentElement extends HTMLElement, TL
     },
   }));
 
-  return { listboxRef, activeParentRef };
+  return { listboxRef, activeParentRef, imperativeRef };
 }
diff --git a/packages/react-components/react-combobox/src/components/Combobox/useCombobox.tsx b/packages/react-components/react-combobox/src/components/Combobox/useCombobox.tsx
@@ -63,13 +63,18 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLIn
   };
 
   const triggerRef = React.useRef<HTMLInputElement>(null);
-  let listbox: Slot<typeof Listbox> | undefined = slot.optional(props.listbox, {
+  let listbox = slot.optional(props.listbox, {
     renderByDefault: true,
     defaultProps: { children: props.children },
     elementType: Listbox,
   });
-  listbox = useListboxSlot(props, listbox, comboboxPopupRef, triggerRef);
-  listbox = open ? listbox : undefined;
+
+  const listboxRef = useMergedRefs(listbox?.ref, comboboxPopupRef);
+  if (listbox) {
+    listbox.ref = listboxRef;
+  }
+
+  listbox = useListboxSlot(props, listbox, triggerRef) as typeof listbox;
 
   let triggerSlot: Slot<'input'> = slot.always(props.input, {
     defaultProps: {
@@ -84,7 +89,7 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLIn
 
   const rootSlot = slot.always(props.root, {
     defaultProps: {
-      'aria-owns': !inlinePopup ? listbox?.id : undefined,
+      'aria-owns': !inlinePopup && open ? listbox?.id : undefined,
       ...rootNativeProps,
     },
     elementType: 'div',
@@ -95,7 +100,7 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLIn
     components: { root: 'div', input: 'input', expandIcon: 'span', listbox: Listbox },
     root: rootSlot,
     input: triggerSlot,
-    listbox,
+    listbox: open ? listbox : undefined,
     expandIcon: slot.optional(props.expandIcon, {
       renderByDefault: true,
       defaultProps: {

diff --git a/packages/react-components/react-combobox/src/components/Dropdown/useDropdown.tsx b/packages/react-components/react-combobox/src/components/Dropdown/useDropdown.tsx
@@ -35,13 +35,16 @@ export const useDropdown_unstable = (props: DropdownProps, ref: React.Ref<HTMLBu
   const [comboboxPopupRef, comboboxTargetRef] = useComboboxPositioning(props);
 
   const triggerRef = React.useRef<HTMLButtonElement>(null);
-  let listbox: Slot<typeof Listbox> | undefined = slot.optional(props.listbox, {
+  let listbox = slot.optional(props.listbox, {
     renderByDefault: true,
     defaultProps: { children: props.children },
     elementType: Listbox,
   });
-  listbox = useListboxSlot(props, listbox, comboboxPopupRef, triggerRef);
-  listbox = open ? listbox : undefined;
+  const listboxRef = useMergedRefs(listbox?.ref, comboboxPopupRef);
+  if (listbox) {
+    listbox.ref = listboxRef;
+  }
+  listbox = useListboxSlot(props, listbox, triggerRef) as typeof listbox;
 
   let trigger: Slot<'button'> = slot.always(props.button, {
     defaultProps: {
@@ -57,7 +60,7 @@ export const useDropdown_unstable = (props: DropdownProps, ref: React.Ref<HTMLBu
 
   const rootSlot = slot.always(props.root, {
     defaultProps: {
-      'aria-owns': !props.inlinePopup ? listbox?.id : undefined,
+      'aria-owns': !props.inlinePopup && open ? listbox?.id : undefined,
       children: props.children,
       ...rootNativeProps,
     },
@@ -69,7 +72,7 @@ export const useDropdown_unstable = (props: DropdownProps, ref: React.Ref<HTMLBu
     components: { root: 'div', button: 'button', expandIcon: 'span', listbox: Listbox },
     root: rootSlot,
     button: trigger,
-    listbox,
+    listbox: open ? listbox : undefined,
     expandIcon: slot.optional(props.expandIcon, {
       renderByDefault: true,
       defaultProps: {

diff --git a/packages/react-components/react-combobox/src/utils/useListboxSlot.ts b/packages/react-components/react-combobox/src/utils/useListboxSlot.ts
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { mergeCallbacks, useId, useEventCallback, useMergedRefs } from '@fluentui/react-utilities';
+import { mergeCallbacks, useId, useEventCallback } from '@fluentui/react-utilities';
 import type { ExtractSlotProps, Slot } from '@fluentui/react-utilities';
 import { Listbox } from '../components/Listbox/Listbox';
 import type { ComboboxBaseProps } from './ComboboxBase.types';
@@ -10,9 +10,8 @@ import type { ComboboxBaseProps } from './ComboboxBase.types';
 export function useListboxSlot(
   props: ComboboxBaseProps,
   listboxSlot: ExtractSlotProps<Slot<typeof Listbox>> | undefined,
-  ref: React.Ref<HTMLDivElement>,
   triggerRef: React.RefObject<HTMLInputElement> | React.RefObject<HTMLButtonElement>,
-): ExtractSlotProps<Slot<typeof Listbox>> | undefined {
+): ExtractSlotProps<Slot<typeof Listbox>> {
   const { multiselect } = props;
 
   /**
@@ -41,7 +40,6 @@ export function useListboxSlot(
     ...listboxSlot,
     onMouseDown,
     onClick,
-    ref: useMergedRefs(listboxSlot?.ref, ref),
   };
 
   return listbox;