Skip to content

[Enhancement] Support nodes within other frames #156

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

Closed
wants to merge 3 commits into from

Conversation

Craga89
Copy link

@Craga89 Craga89 commented May 17, 2016

Added a getOwnerDocument utility that resolves the correct document based on the DraggableCores associated DOMElement. This enables usage of react-draggable on nodes that are rendered within an <iframe />.

@STRML
Copy link
Collaborator

STRML commented May 17, 2016

Hey, nicely done!

Could you add a quick test case for it?

@Craga89
Copy link
Author

Craga89 commented May 17, 2016

Hmmm, I'm not too up-to-date with the TestUtils! Currently using react-frame-component to render into an Iframe, so we'd probably need to do that, but since it uses the unstable_renderSubtreeIntoContainer ReactDOM utility not sure if it would work within the tests?

@Craga89
Copy link
Author

Craga89 commented May 17, 2016

Actually, maybe I just need to test that getOwnerDocument works as expected? That should be easy enough.

@STRML
Copy link
Collaborator

STRML commented May 17, 2016

Yeah - rendering iframed components is a little crazy IMO and breaks all sorts of assumptions. To test it properly I don't mind installing react-frame-component as a devDep.

@Craga89
Copy link
Author

Craga89 commented May 17, 2016

👍 I'll give it a shot when I get some time

@STRML
Copy link
Collaborator

STRML commented Jul 19, 2016

Closing this in favor of #177 which also handles window.getComputedStyle(), which has problems with iframes.

@STRML STRML closed this Jul 19, 2016
@STRML
Copy link
Collaborator

STRML commented Jul 19, 2016

Thanks anyhow for the contribution and I apologize that it got left here for this long.

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