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

reset InPortal nodeProps on OutPortal node update or unmount #38

Merged
merged 2 commits into from
Jul 19, 2022

Conversation

adri1wald
Copy link
Contributor

When the <OutPortal /> node prop changes or it unmounts, we need to reset the relevant node's portal props by calling node.setPortalProps({}).

@CLAassistant
Copy link

CLAassistant commented Jul 11, 2022

CLA assistant check
All committers have signed the CLA.

@pimterry
Copy link
Member

Thanks for this! Sorry I haven't reviewed this yet, it's been a busy week. It looks good at a glance, I just need to do some careful testing to check this for the various edge cases, but I should be able to get that sorted early next week 👍

@adri1wald
Copy link
Contributor Author

adri1wald commented Jul 15, 2022

Hey no worries we're running off of a fork for now. I actually rewrote it using hooks as well as I find it easier to reason about the logic. Would you be interested in accepting a hooks-based rewrite in a different PR?

Edit: also I imagine this fix will be a breaking change for people relying on the current behavior. Probably want to doing a major version release.

Copy link
Member

@pimterry pimterry left a comment

Choose a reason for hiding this comment

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

All works great, thanks!

@pimterry pimterry merged commit 0eca495 into httptoolkit:master Jul 19, 2022
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