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

Fix sandbox refresh bug #20176

Merged
merged 5 commits into from
Feb 13, 2020
Merged

Fix sandbox refresh bug #20176

merged 5 commits into from
Feb 13, 2020

Conversation

glendaviesnz
Copy link
Contributor

@glendaviesnz glendaviesnz commented Feb 12, 2020

Description

Fixes #16831 - currently passing in new iframe html to the sandbox does not cause the component to rerender. This forces a rerender if the html prop has changed.

How has this been tested?

Tested manually using new google calendar block at Automattic/jetpack#13999 - without this patch, and with the temporary key prop to fix this issue removed updating the calendar embed url does not affect the iframe preview, with this patch the iframe preview is reloaded when embed url is changed

Types of changes

Sets the forceRerender value to false by default and sets to true if component updates and html prop has changed

Glen Davies added 2 commits February 12, 2020 14:29
…n order to fix bug with sandbox not refreshing when passed in html changes
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Confirmed the fix by programmatically updating the content attribute of an HTML block in preview mode.

Do you know if we can unit test this somehow.

@glendaviesnz
Copy link
Contributor Author

Do you know if we can unit test this somehow.

Good question, will take a look.

@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Feb 13, 2020

Do you know if we can unit test this somehow.

@youknowriad Took a while to get it rendering in a testable way - couldn't do it with mount or shallow, but act() from 'react-dom/test-utils' did the business, so a test committed using this - let me know if you know of a simpler way.

@youknowriad
Copy link
Contributor

The test is looking good for me. Thanks :)

@glendaviesnz glendaviesnz merged commit 5a5564b into master Feb 13, 2020
@glendaviesnz glendaviesnz deleted the fix/sandbox-refresh-bug branch February 13, 2020 18:53
@github-actions github-actions bot added this to the Gutenberg 7.6 milestone Feb 13, 2020
@mcsf
Copy link
Contributor

mcsf commented Mar 27, 2020

Regression: #20614

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.

SandBox not re-rendered after HTML changes
3 participants