Skip to content

react-transition-group: pass nodeRef so findDOMNode isn't used #1241

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.

Sign up for GitHub

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

Closed
snowystinger wants to merge 11 commits into from
Closed

react-transition-group: pass nodeRef so findDOMNode isn't used #1241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9d68bf5
Update react-transition-group
mischnic Sep 29, 2020
a9b6874
Pass nodeRef into Transition
mischnic Sep 29, 2020
577d2c5
WIP
mischnic Sep 29, 2020
fd558ad
Merge branch 'main' into rtg-findDOMNode
dannify Oct 13, 2020
de418cb
Merge branch 'main' into rtg-findDOMNode
snowystinger Nov 6, 2020
aae65b1
Removed too many deps
snowystinger Nov 6, 2020
4048775
and merged a bad number
snowystinger Nov 6, 2020
9c7216a
fix docs build
snowystinger Nov 6, 2020
ec81f5c
Merge branch 'main' into rtg-findDOMNode-try2
snowystinger Nov 6, 2020
a115f31
Remove a few ignores and explain the last remaining one
snowystinger Nov 17, 2020
18ca0f2
Merge branch 'main' into rtg-findDOMNode-try2
snowystinger Dec 23, 2020
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
2 changes: 1 addition & 1 deletion packages/@react-spectrum/overlays/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"@react-stately/overlays": "^3.1.1",
"@react-types/overlays": "^3.4.0",
"@react-types/shared": "^3.3.0",
"react-transition-group": "^2.2.0"
"react-transition-group": "^4.4.1"
},
"devDependencies": {
"@adobe/spectrum-css-temp": "^3.0.0-alpha.1"
Expand Down
8 changes: 5 additions & 3 deletions packages/@react-spectrum/overlays/src/Modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {ModalProps} from '@react-types/overlays';
import modalStyles from '@adobe/spectrum-css-temp/components/modal/vars.css';
import {Overlay} from './Overlay';
import overrideStyles from './overlays.css';
import React, {forwardRef, HTMLAttributes, ReactNode, RefObject} from 'react';
import React, {forwardRef, HTMLAttributes, ReactNode, RefObject, useRef} from 'react';
import {Underlay} from './Underlay';
import {useModal, useOverlay, usePreventScroll} from '@react-aria/overlays';

Expand All @@ -35,9 +35,11 @@ function Modal(props: ModalProps, ref: DOMRef<HTMLDivElement>) {
let domRef = useDOMRef(ref);
let {styleProps} = useStyleProps(props);

let underlayRef = useRef(null);

return (
<Overlay {...otherProps}>
<Underlay />
<Overlay {...otherProps} nodeRef={underlayRef}>
<Underlay ref={underlayRef} />
<ModalWrapper
{...styleProps}
onClose={onClose}
Expand Down
5 changes: 3 additions & 2 deletions packages/@react-spectrum/overlays/src/Overlay.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import React, {useCallback, useState} from 'react';
import ReactDOM from 'react-dom';

function Overlay(props: OverlayProps, ref: DOMRef<HTMLDivElement>) {
let {children, isOpen, container, onEnter, onEntering, onEntered, onExit, onExiting, onExited} = props;
let {children, isOpen, container, onEnter, onEntering, onEntered, onExit, onExiting, onExited, nodeRef} = props;
let [exited, setExited] = useState(!isOpen);

let handleEntered = useCallback(() => {
Expand Down Expand Up @@ -52,7 +52,8 @@ function Overlay(props: OverlayProps, ref: DOMRef<HTMLDivElement>) {
onExited={handleExited}
onEnter={onEnter}
onEntering={onEntering}
onEntered={handleEntered}>
onEntered={handleEntered}
nodeRef={nodeRef}>
{children}
</OpenTransition>
</Provider>
Expand Down
2 changes: 1 addition & 1 deletion packages/@react-spectrum/overlays/src/Popover.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function Popover(props: PopoverProps, ref: DOMRef<HTMLDivElement>) {
let {styleProps} = useStyleProps(props);

return (
<Overlay {...otherProps}>
<Overlay {...otherProps} nodeRef={domRef}>
<PopoverWrapper
{...styleProps}
ref={domRef}
Expand Down
6 changes: 4 additions & 2 deletions packages/@react-spectrum/overlays/src/Tray.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ function Tray(props: TrayProps, ref: DOMRef<HTMLDivElement>) {
let domRef = useDOMRef(ref);
let {styleProps} = useStyleProps(props);

let underlayRef = useRef(null);

return (
<Overlay {...otherProps}>
<Underlay />
<Overlay {...otherProps} nodeRef={underlayRef}>
<Underlay ref={underlayRef} />
<TrayWrapper
{...styleProps}
onClose={onClose}
Expand Down
9 changes: 6 additions & 3 deletions packages/@react-spectrum/overlays/src/Underlay.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,18 @@
*/

import {classNames} from '@react-spectrum/utils';
import React from 'react';
import React, {Ref} from 'react';
import underlayStyles from '@adobe/spectrum-css-temp/components/underlay/vars.css';

interface UnderlayProps {
isOpen?: boolean
}

export function Underlay({isOpen}: UnderlayProps) {
function Underlay({isOpen}: UnderlayProps, ref: Ref<HTMLDivElement>) {
return (
<div className={classNames(underlayStyles, 'spectrum-Underlay', {'is-open': isOpen})} />
<div ref={ref} className={classNames(underlayStyles, 'spectrum-Underlay', {'is-open': isOpen})} />
);
}

const _Underlay = React.forwardRef(Underlay);
export {_Underlay as Underlay};
22 changes: 19 additions & 3 deletions packages/@react-spectrum/tooltip/src/TooltipTrigger.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {Overlay} from '@react-spectrum/overlays';
import React, {ReactElement, useRef} from 'react';
import {SpectrumTooltipTriggerProps} from '@react-types/tooltip';
import {TooltipContext} from './context';
import {unwrapDOMRef} from '@react-spectrum/utils';
import {useOverlayPosition} from '@react-aria/overlays';
import {useTooltipTrigger} from '@react-aria/tooltip';
import {useTooltipTriggerState} from '@react-stately/tooltip';
Expand All @@ -31,7 +32,22 @@ function TooltipTrigger(props: SpectrumTooltipTriggerProps) {
trigger: triggerAction
} = props;

let [trigger, tooltip] = React.Children.toArray(children);
let tooltipRef = useRef();

let [trigger, tooltip] = React.Children.toArray(children) as [ReactElement, ReactElement];

// this does mean that we can't have some element wrapping the Tooltip
// we'd need to do it through context if we wanted to allow for that
let tooltipWithRef = React.cloneElement(tooltip, {
Copy link
Copy Markdown
Member

@devongovett devongovett Dec 23, 2020

Choose a reason for hiding this comment

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

Yeah we should avoid cloneElement. Someone might do something like <MyFancyTooltip> that wraps <Tooltip> and not pass through the ref...

ref: node => {
tooltipRef.current = node;
// @ts-ignore react forwardRef doesn't get typed quite right, so we ignore these two warnings about accessing ref
if (tooltip.ref) {
// @ts-ignore
tooltip.ref.current = node;
}
}
});

let state = useTooltipTriggerState(props);

Expand Down Expand Up @@ -65,8 +81,8 @@ function TooltipTrigger(props: SpectrumTooltipTriggerProps) {
arrowProps,
...tooltipProps
}}>
<Overlay isOpen={state.isOpen}>
{tooltip}
<Overlay isOpen={state.isOpen} nodeRef={unwrapDOMRef(tooltipRef)}>
{tooltipWithRef}
</Overlay>
</TooltipContext.Provider>
</FocusableProvider>
Expand Down
11 changes: 6 additions & 5 deletions packages/@react-types/overlays/src/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* governing permissions and limitations under the License.
*/

import {HTMLAttributes, ReactElement, ReactNode} from 'react';
import {HTMLAttributes, MutableRefObject, ReactElement, ReactNode} from 'react';
import {StyleProps} from '@react-types/shared';

export type Placement = 'bottom' | 'bottom left' | 'bottom right' | 'bottom start' | 'bottom end' |
Expand Down Expand Up @@ -71,18 +71,19 @@ export interface OverlayProps {
onEntered?: () => void,
onExit?: () => void,
onExiting?: () => void,
onExited?: () => void
onExited?: () => void,
nodeRef: MutableRefObject<HTMLElement>
}

export interface ModalProps extends StyleProps, OverlayProps {
export interface ModalProps extends StyleProps, Omit<OverlayProps, 'nodeRef'> {
children: ReactElement,
isOpen?: boolean,
onClose?: () => void,
type?: 'modal' | 'fullscreen' | 'fullscreenTakeover',
isDismissable?: boolean
}

export interface PopoverProps extends StyleProps, OverlayProps {
export interface PopoverProps extends StyleProps, Omit<OverlayProps, 'nodeRef'> {
children: ReactNode,
placement?: PlacementAxis,
arrowProps?: HTMLAttributes<HTMLElement>,
Expand All @@ -93,7 +94,7 @@ export interface PopoverProps extends StyleProps, OverlayProps {
isNonModal?: boolean
}

export interface TrayProps extends StyleProps, OverlayProps {
export interface TrayProps extends StyleProps, Omit<OverlayProps, 'nodeRef'> {
children: ReactElement,
isOpen?: boolean,
onClose?: () => void,
Expand Down
20 changes: 19 additions & 1 deletion yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1402,7 +1402,7 @@
core-js-pure "^3.0.0"
regenerator-runtime "^0.13.4"

"@babel/runtime@7.9.0", "@babel/runtime@^7.0.0", "@babel/runtime@^7.1.2", "@babel/runtime@^7.10.2", "@babel/runtime@^7.10.3", "@babel/runtime@^7.11.2", "@babel/runtime@^7.3.1", "@babel/runtime@^7.3.4", "@babel/runtime@^7.4.2", "@babel/runtime@^7.4.4", "@babel/runtime@^7.5.0", "@babel/runtime@^7.5.4", "@babel/runtime@^7.5.5", "@babel/runtime@^7.6.2", "@babel/runtime@^7.6.3", "@babel/runtime@^7.7.2", "@babel/runtime@^7.7.4", "@babel/runtime@^7.7.6", "@babel/runtime@^7.8.4", "@babel/runtime@^7.9.2":
"@babel/runtime@7.9.0", "@babel/runtime@^7.0.0", "@babel/runtime@^7.1.2", "@babel/runtime@^7.10.2", "@babel/runtime@^7.10.3", "@babel/runtime@^7.11.2", "@babel/runtime@^7.3.1", "@babel/runtime@^7.3.4", "@babel/runtime@^7.4.2", "@babel/runtime@^7.4.4", "@babel/runtime@^7.5.0", "@babel/runtime@^7.5.4", "@babel/runtime@^7.5.5", "@babel/runtime@^7.6.2", "@babel/runtime@^7.6.3", "@babel/runtime@^7.7.2", "@babel/runtime@^7.7.4", "@babel/runtime@^7.7.6", "@babel/runtime@^7.8.4", "@babel/runtime@^7.8.7", "@babel/runtime@^7.9.2":
version "7.9.0"
resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.9.0.tgz#337eda67401f5b066a6f205a3113d4ac18ba495b"
integrity sha512-cTIudHnzuWLS56ik4DnRnqqNf8MkdUzV4iFFI1h7Jo9xvrpQROYaAnaSd2mHLQAzzZAPfATynX5ord6YlNYNMA==
Expand Down Expand Up @@ -8955,6 +8955,14 @@ dom-helpers@^3.2.1, dom-helpers@^3.3.1, dom-helpers@^3.4.0:
dependencies:
"@babel/runtime" "^7.1.2"

dom-helpers@^5.0.1:
version "5.2.0"
resolved "https://registry.yarnpkg.com/dom-helpers/-/dom-helpers-5.2.0.tgz#57fd054c5f8f34c52a3eeffdb7e7e93cd357d95b"
integrity sha512-Ru5o9+V8CpunKnz5LGgWXkmrH/20cGKwcHwS4m73zIvs54CN9epEmT/HLqFJW3kXpakAFkEdzgy1hzlJe3E4OQ==
dependencies:
"@babel/runtime" "^7.8.7"
csstype "^3.0.2"

dom-serializer@0:
version "0.2.2"
resolved "https://registry.yarnpkg.com/dom-serializer/-/dom-serializer-0.2.2.tgz#1afb81f533717175d478655debc5e332d9f9bb51"
Expand Down Expand Up @@ -19017,6 +19025,16 @@ react-transition-group@^2.2.0, react-transition-group@^2.2.1:
prop-types "^15.6.2"
react-lifecycles-compat "^3.0.4"

react-transition-group@^4.4.1:
version "4.4.1"
resolved "https://registry.yarnpkg.com/react-transition-group/-/react-transition-group-4.4.1.tgz#63868f9325a38ea5ee9535d828327f85773345c9"
integrity sha512-Djqr7OQ2aPUiYurhPalTrVy9ddmFCCzwhqQmtN+J3+3DzLO209Fdr70QrN8Z3DsglWql6iY1lDWAfpFiBtuKGw==
dependencies:
"@babel/runtime" "^7.5.5"
dom-helpers "^5.0.1"
loose-envify "^1.4.0"
prop-types "^15.6.2"

react@^16.7.0, "react@^16.8.0 || ^17.0.0-rc.1":
version "16.13.1"
resolved "https://registry.yarnpkg.com/react/-/react-16.13.1.tgz#2e818822f1a9743122c063d6410d85c1e3afe48e"
Expand Down