Skip to content

Use render prop in withModalHandlers for target#748

Merged
emmatown merged 5 commits into
masterfrom
use-render-prop-in-with-modal-handlers-for-target
Mar 1, 2019
Merged

Use render prop in withModalHandlers for target#748
emmatown merged 5 commits into
masterfrom
use-render-prop-in-with-modal-handlers-for-target

Conversation

@emmatown

@emmatown emmatown commented Mar 1, 2019

Copy link
Copy Markdown
Member

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 added 2 commits March 1, 2019 19:54
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 requested a review from jossmac March 1, 2019 10:06
@emmatown emmatown changed the title Use render prop in with modal handlers for target Use render prop in withModalHandlers for target Mar 1, 2019

@jossmac jossmac left a comment

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.

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

@jossmac

jossmac commented Mar 1, 2019

Copy link
Copy Markdown
Member

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

jossmac commented Mar 1, 2019

Copy link
Copy Markdown
Member

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