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

Added support for a custom container (via getContainer prop) #37

Merged
merged 2 commits into from
Jul 27, 2016

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Jul 27, 2016

I've also added a new Storybook section demonstrating how this property can be used to integrate with more complex 3rd party components such as FlexTable.

This PR also updates the version of 'react-virtualized' used in 'devDependencies' to 7.15 (so that a FlexTable example could be added using the new getContainer prop) and adds 'react' to 'devDependencies' since it was missing.

This PR also updates the version of 'react-virtualized' used in 'devDependencies' to 7.15 so that a FlexTable example could be added using the new getContainer prop.
The PR also adds 'react' to 'devDependencies' since it was missing.
@bvaughn
Copy link
Contributor Author

bvaughn commented Jul 27, 2016

Hey @clauderic :)

I tried to keep the change set size minimal and stick with something close to what we discussed earlier via Gitter. Happy to make changes or adjustments!

@bvaughn
Copy link
Contributor Author

bvaughn commented Jul 27, 2016

I should probably go ahead and do a quick react-virtualized bump to change the refs to .Grid instead of ._grid since I'm basically committing to making that part of the Api going forward. Give me 5 minutes to do this. :)

Edit: Done via react-virtualized release 7.15.1.

This renames ._grid ref to .Grid (since it's now an official part of the API).

this.container = ReactDOM.findDOMNode(this);
this.container = (typeof getContainer == 'function') ? getContainer(this.getWrappedInstance()) : ReactDOM.findDOMNode(this);
Copy link
Contributor Author

@bvaughn bvaughn Jul 27, 2016

Choose a reason for hiding this comment

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

I was on the fence about whether the getContainer prop should return a React element or a DOM node. Thoughts?

Copy link
Owner

Choose a reason for hiding this comment

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

I think I'd lean towards DOM node, simply because it would enable the use of elements outside of React's scope, for instance, document.body

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I thought also. (So that's what it currently does.)

@clauderic clauderic merged commit 2bedfb2 into clauderic:master Jul 27, 2016
@joaoreynolds
Copy link

I'm trying to use this new feature (getContainer) and I'm having trouble making it work. Can you provide an example? I seem to be returning the wrong object in this function. I've tried sending the ref object, as well as the dom object returned by findDOMNode

@bvaughn
Copy link
Contributor Author

bvaughn commented Jul 30, 2016

Hey @joaoreynolds

You can see an example here:
https://github.com/clauderic/react-sortable-hoc/blob/master/src/.stories/index.js#L97-L143

DimitarNestorov pushed a commit to codemotionapps/react-sortable-hoc that referenced this pull request Feb 4, 2019
Added support for a custom container (via getContainer prop)
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.

3 participants