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

Page layout picker: Fix broken web fonts in page layout picker large preview #50253

Merged
merged 2 commits into from
Feb 19, 2021

Conversation

andrewserong
Copy link
Member

@andrewserong andrewserong commented Feb 19, 2021

In the page layout picker, the existing code that looks to copy over styles to the iframe no longer targets the correct style tags, causing the preview to miss crucial style tags for rendering the web fonts from Global Styles correctly.

This change targets getting the styles from where they're inserted in the visual editor in Gutenberg, which is as children of the .edit-post-visual-editor node (the relevant line in Gutenberg is here).

This fixes the rendering issue reported in #49742.

Changes proposed in this Pull Request

  • In the page layout picker, target only the style tags within the post body that we require to adequately style the preview, to avoid CSS conflicts that broke web fonts in the preview.
  • Note: the whitespace change in the readme.txt file is just to trigger an ETK build. I'll remove the whitespace change before merging.

Screenshot

Before:

image

After:

image

Testing instructions

  • Apply this change in your sandbox (D57313-code) and set your sandboxed test site to use a theme that uses custom fonts / Global styles, e.g. Balasana, Maywood, or Barnsbury
  • Go to add a new page, and browse through the layouts and compare against the reported issue (Starter Page Templates: large iFrame preview is not rendering Google fonts #49742) — the styles should look much closer to how the layout looks when inserted in the editor.
  • Insert a layout.
  • From the top right in the editor, go to Global Styles and update the fonts for the site, and hit the publish button within the Global Styles plugin.
  • Go to add a new page and ensure the preview renders with the global styles correctly.
  • Switch your site to a theme that doesn't use Global Styles, e.g. Twenty Twenty or Twenty Twenty One, and ensure the preview renders correctly (or at least better than in the reported issue)

Related to #49742

@matticbot
Copy link
Contributor

@andrewserong andrewserong self-assigned this Feb 19, 2021
@andrewserong andrewserong requested a review from a team February 19, 2021 00:49
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 19, 2021
@andrewserong andrewserong requested a review from a team February 19, 2021 00:49
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@andrewserong andrewserong added the [Type] Bug When a feature is broken and / or not performing as intended label Feb 19, 2021
@matticbot
Copy link
Contributor

Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D57313-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2

Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

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

LGTM. Everything tested well. 🚢

  • Tested with Balasana & Maywood
  • New page layout previews look much closer to how they look after inserting into the editor.
  • Inspecting styles within the previews showed correct Google fonts being applied
  • Previews also honoured prior selections for custom fonts via the Customizer
  • After applying Global Styles, the previews faithfully reflected those choices
  • Switching back to a non Global Styles theme, the previews reflected that themes fonts

@ramonjd
Copy link
Member

ramonjd commented Feb 19, 2021

Before:
Screen Shot 2021-02-19 at 12 47 34 pm

After:
Screen Shot 2021-02-19 at 12 53 35 pm

👍

@andrewserong andrewserong merged commit 8172b29 into trunk Feb 19, 2021
@andrewserong andrewserong deleted the try/fixes-for-page-layout-picker branch February 19, 2021 03:15
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants