Skip to content

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

Closed
mischnic wants to merge 5 commits into
mainfrom
rtg-findDOMNode
Closed

react-transition-group: pass nodeRef so findDOMNode isn't used#1108
mischnic wants to merge 5 commits into
mainfrom
rtg-findDOMNode

Conversation

@mischnic
Copy link
Copy Markdown
Contributor

Part of #779

There are more problems with strict mode (which is why I didn't enable strict mode in tests or Storybook yet), this is just fixing the react-transition-group and findDOMNode problem.

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

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

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

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

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

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

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...

@adobe-bot
Copy link
Copy Markdown

Build successful! 🎉

Comment thread yarn.lock
@adobe-bot
Copy link
Copy Markdown

Build successful! 🎉

@adobe-bot
Copy link
Copy Markdown

Build successful! 🎉

@snowystinger
Copy link
Copy Markdown
Member

@mischnic got a conflict :(

@snowystinger
Copy link
Copy Markdown
Member

Closing due to CLA, I've reopened it here #1241

@dannify dannify deleted the rtg-findDOMNode branch March 26, 2021 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants