Skip to content

Replace useCombinedRefs with useRefObjectAsForwardedRef #2204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/red-wombats-whisper.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Replace `useCombinedRefs` with `useRefObjectAsForwardedRef`
11 changes: 5 additions & 6 deletions src/Autocomplete/AutocompleteInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React, {
ChangeEventHandler,
FocusEventHandler,
KeyboardEventHandler,
MutableRefObject,
useCallback,
useContext,
useEffect,
Expand All @@ -11,7 +10,7 @@ import React, {
import {ForwardRefComponent as PolymorphicForwardRefComponent} from '@radix-ui/react-polymorphic'
import {AutocompleteContext} from './AutocompleteContext'
import TextInput from '../TextInput'
import {useCombinedRefs} from '../hooks/useCombinedRefs'
import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef'
import {ComponentProps} from '../utils/types'

type InternalAutocompleteInputProps = {
Expand Down Expand Up @@ -39,7 +38,7 @@ const AutocompleteInput = React.forwardRef(
setShowMenu,
showMenu
} = autocompleteContext
const combinedInputRef = useCombinedRefs(inputRef, forwardedRef)
useRefObjectAsForwardedRef(forwardedRef, inputRef)
const [highlightRemainingText, setHighlightRemainingText] = useState<boolean>(true)

const handleInputFocus: FocusEventHandler<HTMLInputElement> = useCallback(
Expand All @@ -58,12 +57,12 @@ const AutocompleteInput = React.forwardRef(
// this prevents the menu from hiding when the user is clicking an option in the Autoselect.Menu,
// but still hides the menu when the user blurs the input by tabbing out or clicking somewhere else on the page
setTimeout(() => {
if (document.activeElement !== combinedInputRef.current) {
if (document.activeElement !== inputRef.current) {
setShowMenu(false)
}
}, 0)
},
[onBlur, setShowMenu, combinedInputRef]
[onBlur, setShowMenu, inputRef]
)

const handleInputChange: ChangeEventHandler<HTMLInputElement> = useCallback(
Expand Down Expand Up @@ -157,7 +156,7 @@ const AutocompleteInput = React.forwardRef(
onKeyDown={handleInputKeyDown}
onKeyPress={onInputKeyPress}
onKeyUp={handleInputKeyUp}
ref={combinedInputRef as MutableRefObject<HTMLInputElement>}
ref={inputRef}
aria-controls={`${id}-listbox`}
aria-autocomplete="both"
role="combobox"
Expand Down
6 changes: 3 additions & 3 deletions src/Autocomplete/AutocompleteOverlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {useAnchoredPosition} from '../hooks'
import Overlay, {OverlayProps} from '../Overlay'
import {ComponentProps} from '../utils/types'
import {AutocompleteContext} from './AutocompleteContext'
import {useCombinedRefs} from '../hooks/useCombinedRefs'
import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef'

type AutocompleteOverlayInternalProps = {
/**
Expand Down Expand Up @@ -39,7 +39,7 @@ function AutocompleteOverlay({
[showMenu, selectedItemLength]
)

const combinedOverlayRef = useCombinedRefs(scrollContainerRef, floatingElementRef)
useRefObjectAsForwardedRef(scrollContainerRef, floatingElementRef)

const closeOptionList = useCallback(() => {
setShowMenu(false)
Expand All @@ -55,7 +55,7 @@ function AutocompleteOverlay({
preventFocusOnOpen={true}
onClickOutside={closeOptionList}
onEscape={closeOptionList}
ref={combinedOverlayRef as React.RefObject<HTMLDivElement>}
ref={floatingElementRef as React.RefObject<HTMLDivElement>}
top={position?.top}
left={position?.left}
visibility={showMenu ? 'visible' : 'hidden'}
Expand Down
5 changes: 3 additions & 2 deletions src/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import useDialog from './hooks/useDialog'
import sx, {SxProp} from './sx'
import Text from './Text'
import {ComponentProps} from './utils/types'
import {useCombinedRefs} from './hooks/useCombinedRefs'
import {useRefObjectAsForwardedRef} from './hooks/useRefObjectAsForwardedRef'

const noop = () => null

Expand Down Expand Up @@ -95,7 +95,8 @@ type InternalDialogProps = {
const Dialog = forwardRef<HTMLDivElement, InternalDialogProps>(
({children, onDismiss = noop, isOpen, initialFocusRef, returnFocusRef, ...props}, forwardedRef) => {
const overlayRef = useRef(null)
const modalRef = useCombinedRefs(forwardedRef)
const modalRef = useRef<HTMLDivElement>(null)
useRefObjectAsForwardedRef(forwardedRef, modalRef)
const closeButtonRef = useRef(null)

const onCloseClick = () => {
Expand Down
6 changes: 3 additions & 3 deletions src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {XIcon} from '@primer/octicons-react'
import {useFocusZone} from '../hooks/useFocusZone'
import {FocusKeys} from '@primer/behaviors'
import Portal from '../Portal'
import {useCombinedRefs} from '../hooks/useCombinedRefs'
import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef'
import {useSSRSafeId} from '@react-aria/ssr'

const ANIMATION_DURATION = '200ms'
Expand Down Expand Up @@ -274,7 +274,7 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
const defaultedProps = {...props, title, subtitle, role, dialogLabelId, dialogDescriptionId}

const dialogRef = useRef<HTMLDivElement>(null)
const combinedRef = useCombinedRefs(dialogRef, forwardedRef)
useRefObjectAsForwardedRef(forwardedRef, dialogRef)
const backdropRef = useRef<HTMLDivElement>(null)
useFocusTrap({containerRef: dialogRef, restoreFocusOnCleanUp: true, initialFocusRef: autoFocusedFooterButtonRef})

Expand All @@ -297,7 +297,7 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
<StyledDialog
width={width}
height={height}
ref={combinedRef}
ref={dialogRef}
role={role}
aria-labelledby={dialogLabelId}
aria-describedby={dialogDescriptionId}
Expand Down
12 changes: 6 additions & 6 deletions src/Overlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {AriaRole, Merge} from './utils/types'
import {useOverlay, TouchOrMouseEvent} from './hooks'
import Portal from './Portal'
import sx, {SxProp} from './sx'
import {useCombinedRefs} from './hooks/useCombinedRefs'
import {useRefObjectAsForwardedRef} from './hooks/useRefObjectAsForwardedRef'
import type {AnchorSide} from '@primer/behaviors'
import {useTheme} from './ThemeProvider'
import {ForwardRefComponent as PolymorphicForwardRefComponent} from '@radix-ui/react-polymorphic'
Expand Down Expand Up @@ -142,7 +142,7 @@ const Overlay = React.forwardRef<HTMLDivElement, OwnOverlayProps>(
forwardedRef
): ReactElement => {
const overlayRef = useRef<HTMLDivElement>(null)
const combinedRef = useCombinedRefs(overlayRef, forwardedRef)
useRefObjectAsForwardedRef(forwardedRef, overlayRef)
const {theme} = useTheme()
const slideAnimationDistance = parseInt(get('space.2')(theme).replace('px', ''))
const slideAnimationEasing = get('animation.easeOutCubic')(theme)
Expand All @@ -158,10 +158,10 @@ const Overlay = React.forwardRef<HTMLDivElement, OwnOverlayProps>(
})

useEffect(() => {
if (height === 'initial' && combinedRef.current?.clientHeight) {
combinedRef.current.style.height = `${combinedRef.current.clientHeight}px`
if (height === 'initial' && overlayRef.current?.clientHeight) {
overlayRef.current.style.height = `${overlayRef.current.clientHeight}px`
}
}, [height, combinedRef])
}, [height])

useLayoutEffect(() => {
const {x, y} = getSlideAnimationStartingVector(anchorSide)
Expand All @@ -185,7 +185,7 @@ const Overlay = React.forwardRef<HTMLDivElement, OwnOverlayProps>(
height={height}
role={role}
{...rest}
ref={combinedRef}
ref={overlayRef}
style={
{
top: `${top || 0}px`,
Expand Down
16 changes: 7 additions & 9 deletions src/TextInputWithTokens.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import {isFocusable} from '@primer/behaviors/utils'
import {omit} from '@styled-system/props'
import React, {FocusEventHandler, KeyboardEventHandler, MouseEventHandler, RefObject, useRef, useState} from 'react'
import Box from './Box'
import {useProvidedRefOrCreate} from './hooks'
import {useCombinedRefs} from './hooks/useCombinedRefs'
import {useRefObjectAsForwardedRef} from './hooks/useRefObjectAsForwardedRef'
import {useFocusZone} from './hooks/useFocusZone'
import Text from './Text'
import {TextInputProps} from './TextInput'
Expand Down Expand Up @@ -93,12 +92,11 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo
visibleTokenCount,
...rest
}: TextInputWithTokensProps<TokenComponentType>,
externalRef: React.ForwardedRef<HTMLInputElement>
forwardedRef: React.ForwardedRef<HTMLInputElement>
) {
const {onBlur, onFocus, onKeyDown, ...inputPropsRest} = omit(rest)
const ref = useProvidedRefOrCreate<HTMLInputElement>(externalRef as React.RefObject<HTMLInputElement>)
const localInputRef = useRef<HTMLInputElement>(null)
const combinedInputRef = useCombinedRefs(localInputRef, ref)
const ref = useRef<HTMLInputElement>(null)
useRefObjectAsForwardedRef(forwardedRef, ref)
const [selectedTokenIndex, setSelectedTokenIndex] = useState<number | undefined>()
const [tokensAreTruncated, setTokensAreTruncated] = useState<boolean>(Boolean(visibleTokenCount))
const {containerRef} = useFocusZone(
Expand All @@ -124,7 +122,7 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo
}

if (nextIndex > tokens.length || nextIndex < 1) {
return combinedInputRef.current || undefined
return ref.current || undefined
}

return containerRef.current?.children[nextIndex] as HTMLElement
Expand Down Expand Up @@ -230,7 +228,7 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo
}

const focusInput: MouseEventHandler = () => {
combinedInputRef.current?.focus()
ref.current?.focus()
}

const preventTokenClickPropagation: MouseEventHandler = event => {
Expand Down Expand Up @@ -323,7 +321,7 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo
}}
>
<UnstyledTextInput
ref={combinedInputRef}
ref={ref}
disabled={disabled}
onFocus={handleInputFocus}
onBlur={handleInputBlur}
Expand Down
6 changes: 4 additions & 2 deletions src/drafts/InlineAutocomplete/InlineAutocomplete.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, {cloneElement, useRef} from 'react'
import Box from '../../Box'
import {useCombinedRefs} from '../../hooks/useCombinedRefs'
import {useSyntheticChange} from '../hooks/useSyntheticChange'
import Portal from '../../Portal'
import {BetterSystemStyleObject} from '../../sx'
Expand All @@ -14,6 +13,7 @@ import {
requireChildrenToBeInput
} from './utils'
import AutocompleteSuggestions from './_AutocompleteSuggestions'
import {useRefObjectAsForwardedRef} from '../../hooks'

export type InlineAutocompleteProps = {
/** Register the triggers that can cause suggestions to appear. */
Expand Down Expand Up @@ -86,7 +86,9 @@ const InlineAutocomplete = ({
// Forward accessibility props so it works with FormControl
...forwardProps
}: InlineAutocompleteProps & React.ComponentProps<'textarea' | 'input'>) => {
const inputRef = useCombinedRefs(children.ref)
const inputRef = useRef<HTMLInputElement & HTMLTextAreaElement>(null)
useRefObjectAsForwardedRef(children.ref ?? noop, inputRef)

const externalInput = requireChildrenToBeInput(children, inputRef)

const emitSyntheticChange = useSyntheticChange({
Expand Down
1 change: 1 addition & 0 deletions src/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ export {useProvidedStateOrCreate} from './useProvidedStateOrCreate'
export {useMenuInitialFocus} from './useMenuInitialFocus'
export {useMenuKeyboardNavigation} from './useMenuKeyboardNavigation'
export {useMnemonics} from './useMnemonics'
export {useRefObjectAsForwardedRef} from './useRefObjectAsForwardedRef'
40 changes: 0 additions & 40 deletions src/hooks/useCombinedRefs.ts

This file was deleted.

12 changes: 12 additions & 0 deletions src/hooks/useRefObjectAsForwardedRef.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import {ForwardedRef, RefObject, useImperativeHandle} from 'react'

/**
* Use a ref object as the imperative handle for a forwarded ref. This can be used to
* synchronize the ref object with the forwarded ref and allow local access the reference
* instance with `.current`.
*
* **NOTE**: The `refObject` should be passed to the underlying element, NOT the `forwardedRef`.
*/
export function useRefObjectAsForwardedRef<T>(forwardedRef: ForwardedRef<T>, refObject: RefObject<T>): void {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to suggestions for naming this. I think this name works but it could maybe be clearer. Other options I considered include:

  • useForwardedRefObject
  • useForwardedRefAsRefObject
  • useRefObjectAsHandle

useImperativeHandle<T | null, T | null>(forwardedRef, () => refObject.current)
}