Skip to content
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

Use render prop in withModalHandlers for target #748

Merged
merged 5 commits into from
Mar 1, 2019

Conversation

emmatown
Copy link
Member

@emmatown emmatown commented Mar 1, 2019

I'm working on code splitting things with suspense and things are breaking because of the findDOMNode calls in react-node-resolver

Slightly related to this: @jossmac, i think you'll like this, reactjs/rfcs#97

I'm working on code splitting things with suspense and things are breaking because of the findDOMNode calls in react-node-resolver

Slightly related to this: @jossmac, i think you'll like this, reactjs/rfcs#97
@emmatown emmatown changed the title Use render prop in with modal handlers for target Use render prop in withModalHandlers for target Mar 1, 2019
Copy link
Member

@jossmac jossmac left a comment

Choose a reason for hiding this comment

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

LGTM. NodeResolver was a shitty solution to a shitty problem...

@jossmac
Copy link
Member

jossmac commented Mar 1, 2019

Slightly related to this: @jossmac, i think you'll like this, reactjs/rfcs#97

Oh god, yes --> that's gonna be sweet! Coupled with the "native" focus management RFC; things are looking good 🙂

@emmatown emmatown merged commit bd06ddb into master Mar 1, 2019
@emmatown emmatown deleted the use-render-prop-in-with-modal-handlers-for-target branch March 1, 2019 12:04
@jossmac
Copy link
Member

jossmac commented Mar 1, 2019

I thought this disclaimer might be helpful for the node resolver package.

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.

2 participants