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

Legacy widget's preview functionality is broken when the page is moved #34384

Merged
merged 23 commits into from
Aug 31, 2021
Merged

Legacy widget's preview functionality is broken when the page is moved #34384

merged 23 commits into from
Aug 31, 2021

Conversation

anton-vlasenko
Copy link
Contributor

Description

Legacy Widget block has a preview function. However, it uses a relative URL.
iframe cannot load the widgets.php file when the code is no longer running from the /wp-admin/ URL.
This PR fixes that by using a REST API endpoint instead of the widgets.php file to fetch preview HTML. It doesn't depend on the admin URL.
Fixes #30049

Based on #34230

How has this been tested?

  1. Go to Appearance -> Widgets page.
  2. Click on the "plus sign" button at the very bottom of the page.
  3. Add a Legacy Widget block.
  4. Select Meta widget from the dropdown.
  5. Click somewhere on the background of the page (not on the Meta widget itself). You should see a preview of the Meta widget now.
  6. Open your browser's developer tools and find an <iframe /> element with the wp-block-legacy-widget__edit-preview-iframe class. Check its srcdoc attribute.

Expected result:

The srcdoc attribute of the <iframe /> should contain html code of the preview.
Please see the screenshots section for reference.

Screenshots

Screenshot 2021-08-12 at 20 00 39
Screenshot 2021-08-19 at 21 59 05

Types of changes

Bug fix.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

…iew was loaded.

2. Refactor the function. Use return early approach.
2. Add error handling.
3. Abort request if the iframe is unmounted.
4. Use useEffect to make a request (per kevin940726's comment).
5. Use states instead of manipulating DOM values.
…dability.

2. Remove isPreviewFetched state (useEffect is enough).
3. Add idBase and instance as dependencies to useEffect method. We have to fetch the preview html again if any of those parameter gets changed.
4. Move abortController and fetchPreviewHTML inside the useEffect callback to avoid recreating them on each component redraw.
We have to include id_base parameter.
Copy link
Member

@kevin940726 kevin940726 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@anton-vlasenko anton-vlasenko added this to the Gutenberg 11.5 milestone Aug 31, 2021
@anton-vlasenko anton-vlasenko added the [Block] Legacy Widget Affects the Legacy Widget Block - used for displaying Classic Widgets label Aug 31, 2021
@anton-vlasenko
Copy link
Contributor Author

@kevin940726 Could you please merge it if you think it's fine? 🙏
Thank you.

@kevin940726 kevin940726 merged commit 521dbbb into WordPress:trunk Aug 31, 2021
@noisysocks
Copy link
Member

Nice work 👏

@getsource getsource added the [Type] Bug An existing feature does not function as intended label Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Legacy Widget Affects the Legacy Widget Block - used for displaying Classic Widgets [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Widgets preview uses a relative URL that breaks when the page is moved
4 participants