-
Notifications
You must be signed in to change notification settings - Fork 658
Revert "Replace useRefObjectAsForwardedRef with useMergedRefs internally"
#7737
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||||
| import React, {forwardRef, type JSX} from 'react' | ||||||||||||||||
| import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' | ||||||||||||||||
| import type {ButtonProps} from './types' | ||||||||||||||||
| import {useMergedRefs} from '../hooks/useMergedRefs' | ||||||||||||||||
| import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' | ||||||||||||||||
| import {VisuallyHidden} from '../VisuallyHidden' | ||||||||||||||||
| import Spinner from '../Spinner' | ||||||||||||||||
| import CounterLabel from '../CounterLabel' | ||||||||||||||||
|
|
@@ -51,7 +51,7 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f | |||||||||||||||
| } = props | ||||||||||||||||
|
|
||||||||||||||||
| const innerRef = React.useRef<HTMLButtonElement>(null) | ||||||||||||||||
| const combinedRefs = useMergedRefs(forwardedRef, innerRef) | ||||||||||||||||
| useRefObjectAsForwardedRef(forwardedRef, innerRef) | ||||||||||||||||
|
|
||||||||||||||||
| const uuid = useId(id) | ||||||||||||||||
| const loadingAnnouncementID = `${uuid}-loading-announcement` | ||||||||||||||||
|
|
@@ -87,7 +87,8 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f | |||||||||||||||
| <Component | ||||||||||||||||
| aria-disabled={loading ? true : undefined} | ||||||||||||||||
| {...rest} | ||||||||||||||||
| ref={combinedRefs} | ||||||||||||||||
| // @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent | ||||||||||||||||
| ref={innerRef} | ||||||||||||||||
|
Comment on lines
+90
to
+91
|
||||||||||||||||
| // @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent | |
| ref={innerRef} | |
| ref={ | |
| Component === 'a' | |
| ? (innerRef as React.Ref<HTMLAnchorElement>) | |
| : (innerRef as React.Ref<HTMLButtonElement>) | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import {clsx} from 'clsx' | ||
| import React, {useEffect, type ForwardedRef, type ElementRef} from 'react' | ||
| import {useMergedRefs} from '../hooks' | ||
| import {useRefObjectAsForwardedRef} from '../hooks' | ||
| import classes from './Link.module.css' | ||
| import type {ComponentProps} from '../utils/types' | ||
| import {type PolymorphicProps, fixedForwardRef} from '../utils/modern-polymorphic' | ||
|
|
@@ -20,7 +20,7 @@ export const UnwrappedLink = <As extends React.ElementType = 'a'>( | |
| ) => { | ||
| const {as: Component = 'a', className, inline, muted, hoverColor, ...restProps} = props | ||
| const innerRef = React.useRef<ElementRef<As>>(null) | ||
| const mergedRef = useMergedRefs(ref, innerRef) | ||
| useRefObjectAsForwardedRef(ref, innerRef) | ||
|
|
||
| if (__DEV__) { | ||
| /** | ||
|
|
@@ -53,7 +53,8 @@ export const UnwrappedLink = <As extends React.ElementType = 'a'>( | |
| data-inline={inline} | ||
| data-hover-color={hoverColor} | ||
| {...restProps} | ||
| ref={mergedRef} | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| ref={innerRef as any} | ||
|
Comment on lines
53
to
+57
|
||
| /> | ||
| ) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ import type {AriaRole, Merge} from '../utils/types' | |
| import type {TouchOrMouseEvent} from '../hooks' | ||
| import {useOverlay} from '../hooks' | ||
| import Portal from '../Portal' | ||
| import {useMergedRefs} from '../hooks/useMergedRefs' | ||
| import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' | ||
| import type {AnchorSide} from '@primer/behaviors' | ||
| import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' | ||
| import classes from './Overlay.module.css' | ||
|
|
@@ -194,7 +194,7 @@ const Overlay = React.forwardRef<HTMLDivElement, internalOverlayProps>( | |
| ): ReactElement<any> => { | ||
| const featureFlagMaxHeightClampToViewport = useFeatureFlag('primer_react_overlay_max_height_clamp_to_viewport') | ||
| const overlayRef = useRef<HTMLDivElement>(null) | ||
| const mergedRef = useMergedRefs(forwardedRef, overlayRef) | ||
| useRefObjectAsForwardedRef(forwardedRef, overlayRef) | ||
| const slideAnimationDistance = 8 // var(--base-size-8), hardcoded to do some math | ||
|
Comment on lines
195
to
198
|
||
| const slideAnimationEasing = 'cubic-bezier(0.33, 1, 0.68, 1)' | ||
| const cssAnchorPositioning = useFeatureFlag('primer_react_css_anchor_positioning') | ||
|
|
@@ -239,7 +239,7 @@ const Overlay = React.forwardRef<HTMLDivElement, internalOverlayProps>( | |
| role={role} | ||
| width={width} | ||
| data-reflow-container={!preventOverflow ? true : undefined} | ||
| ref={mergedRef} | ||
| ref={overlayRef} | ||
| left={leftPosition} | ||
| right={right} | ||
| height={height} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing
inputNode.focus()means this test now relies onfireEvent.click(inputNode)to focus/open the menu, which can make it pass even if the intended “ArrowDown when focused opens menu” behavior regresses (click may open the menu by itself). To keep the assertion targeted, focus the input (or assert the menu remains closed after click) before the ArrowDown keyDown.