Skip to content

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

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
mischnic wants to merge 5 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 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
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
4 changes: 1 addition & 3 deletions packages/@react-spectrum/overlays/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@
"@react-aria/utils": "^3.4.0",
"@react-spectrum/utils": "^3.4.0",
"@react-stately/overlays": "^3.1.1",
"@react-types/overlays": "^3.3.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 @@ -51,7 +51,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 @@ -35,9 +35,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
7 changes: 5 additions & 2 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 @@ -18,8 +18,11 @@ interface UnderlayProps {
isOpen?: boolean
}

export function Underlay({isOpen}: UnderlayProps) {
function Underlay({isOpen}: UnderlayProps, ref: React.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};
17 changes: 15 additions & 2 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 @@ -30,8 +31,19 @@ function TooltipTrigger(props: SpectrumTooltipTriggerProps) {
offset = DEFAULT_OFFSET
} = props;

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

// @ts-ignore
let tooltipWithRef = React.cloneElement(tooltip, {ref: node => {
tooltipRef.current = node;
// @ts-ignore
if (tooltip.ref) {
// @ts-ignore
tooltip.ref.current = node;
}
}});
Comment on lines +37 to +45
Copy link
Copy Markdown
Contributor Author

@mischnic mischnic Sep 29, 2020

Choose a reason for hiding this comment

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

I couldn't find a better way to do this

Copy link
Copy Markdown
Member

@snowystinger snowystinger Oct 1, 2020

Choose a reason for hiding this comment

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

i'm guessing merging them in a useEffect is too late?

Copy link
Copy Markdown
Contributor Author

@mischnic mischnic Oct 1, 2020

Choose a reason for hiding this comment

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

What do you have in mind?

Copy link
Copy Markdown
Member

@snowystinger snowystinger Oct 1, 2020

Choose a reason for hiding this comment

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

we have some ref merging logic around the repo that takes place in a useEffect, trying to find some examples
it uses context usually though if i recall correctly

Copy link
Copy Markdown
Member

@snowystinger snowystinger Oct 2, 2020

Choose a reason for hiding this comment

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

here's the one i was thinking of https://github.com/adobe/react-spectrum/blob/main/packages/@react-aria/interactions/src/DOMPropsContext.ts#L37
I've no idea if it'll help you out

Copy link
Copy Markdown
Contributor Author

@mischnic mischnic Oct 2, 2020

Choose a reason for hiding this comment

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

I don't think so, it could still be the case that the tooltip doesn't have a ref yet and I still need to do cloneElement then...


let state = useTooltipTriggerState(props);

let tooltipTriggerRef = useRef<HTMLElement>();
Expand Down Expand Up @@ -63,8 +75,9 @@ function TooltipTrigger(props: SpectrumTooltipTriggerProps) {
arrowProps,
...tooltipProps
}}>
<Overlay isOpen={state.isOpen}>
{tooltip}
{/* @ts-ignore */}
<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 React, {HTMLAttributes, 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: React.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 @@ -92,7 +93,7 @@ export interface PopoverProps extends StyleProps, OverlayProps {
shouldCloseOnBlur?: 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:
Comment thread
mischnic marked this conversation as resolved.
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