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.

Sign up for GitHub

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

Jump to bottom

Add default PortalContext if it's undefined #514

Merged
merged 4 commits into from
Dec 13, 2019
Merged

Add default PortalContext if it's undefined #514

merged 4 commits into from
Dec 13, 2019

Conversation

danielbartsch
Copy link
Contributor

@danielbartsch danielbartsch commented Dec 13, 2019 •
edited
Loading

Should fix below issue, when PortalContext is undefined. See the commit message for good description.

Closes #513

Does this PR introduce a breaking change?

Unfortunately, not sure.
I couldn't figure out when document would ever be undefined. But in that case, the outcome is different now.

@danielbartsch
use document.body as context, if it is undefined
71bf02f 
it can be undefined if the script that runs javascript in the htmlpage is called before the body was rendered. Eg. in the  element
@danielbartsch danielbartsch mentioned this pull request Dec 13, 2019
Closed
@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 13, 2019 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 35fb93a:

Sandbox Source
heuristic-johnson-opu2c Configuration

@ariakit-bot
Copy link

ariakit-bot commented Dec 13, 2019 •
edited
Loading

Deploy preview for reakit ready!

Built with commit 35fb93a

https://deploy-preview-514--reakit.netlify.com

diegohaz
diegohaz requested changes Dec 13, 2019
View reviewed changes
packages/reakit/src/Portal/Portal.tsx Outdated Show resolved Hide resolved
diegohaz
diegohaz approved these changes Dec 13, 2019
View reviewed changes
Copy link
Member

@diegohaz diegohaz left a comment

Choose a reason for hiding this comment

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

Thanks! :D

@diegohaz diegohaz merged commit 619adfd into ariakit:master Dec 13, 2019
@ariakit-bot
Copy link

Thanks a lot for contributing!

Based on our community guidelines, every person who has a PR of any kind merged is offered an invitation to the Reakit organization.

Should you accept, you'll get write access to the main repository and a fancy Reakit logo on your GitHub profile. You'll be able to label and close issues, create new branches etc. Make sure to read our contribution and community guidelines, which explains all of this in more detail.

If you have any questions, let me know!

@namjul namjul mentioned this pull request Dec 17, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diegohaz diegohaz diegohaz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Portal has null context
3 participants
@danielbartsch @ariakit-bot @diegohaz