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

appendTo prop #2802

Merged
merged 3 commits into from Sep 4, 2019
Merged

appendTo prop #2802

merged 3 commits into from Sep 4, 2019

Conversation

@jschuler
Copy link
Collaborator

jschuler commented Aug 29, 2019

Closes: #2551

Adds appendTo prop to the Modal which makes it more flexible to where the DOM element can get appended to

Examples:
1.

<Modal
          appendTo={document.getElementById('my-target')} />
<Modal
          appendTo={() => {
            const innerFrame = document.getElementById('my_iframe').contentDocument;
            return innerFrame.getElementsByTagName('body')[0];
          }} />
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Aug 29, 2019

PatternFly-React preview: https://patternfly-react-pr-2802.surge.sh

Copy link
Contributor

tlabaj left a comment

can you link related issue please. Thanks.

Copy link
Contributor

tlabaj left a comment

Should similar changes be made to the AboutModal? If so can you open an issue for that please.

@kmcfaul
kmcfaul approved these changes Sep 4, 2019
@redallen redallen merged commit 704a5f7 into patternfly:master Sep 4, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Sep 4, 2019

Your changes have been released in:

  • @patternfly/react-core@3.95.6
  • @patternfly/react-docs@4.10.41
  • @patternfly/react-inline-edit-extension@2.11.16
  • demo-app-ts@2.21.20
  • @patternfly/react-table@2.19.16
  • @patternfly/react-topology@2.8.16
  • @patternfly/react-virtualized-extension@1.2.5

Thanks for your contribution! 🎉

@sjd78

This comment has been minimized.

Copy link
Member

sjd78 commented Sep 7, 2019

@jschuler - This looks pretty good, and I know it's merged, but I do have a question.

Doesn't the appendTo still really need to be a document.body for the backdrop styles to be applied correctly and for the ESC key events to be handled properly? In other words, will your example 1 actually work?

In the case of my PR #2574, and Issue #2551, the situation is the react app running in an iframe and needing to render the component up in the frame's parent...

<Modal appendTo={window.top.document.body} />

...so I don't see an immediate issue for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.