SPT: preview into an iFrame element #39628

retrofox · 2020-02-24T14:54:00Z

Changes proposed in this Pull Request

This PR injects starter page template blocks into an iframe in order to more accurately preview what the templates look like.

A user wants the starter page template previews to match what they will see when they apply the template selection and are moved into the editor.

Related issues:

Fixes #39341.
Fixes #38779.
Related #39348, #39352, #39355.

Dev Setup For Testing

See existing documentation for setting up FSE plugin development. In addition, you will need these plugins active on your local environment, likely via a mounted volume:

Gutenberg
Jetpack (with WPCOM connection or running in Development Mode)
CoBlocks: https://wordpress.org/plugins/coblocks/
Block Experiments: https://github.com/automattic/block-experiments

Compile SPT plugin from apps/full-site-editing directory in calypso:

npx lerna run dev --scope='@automattic/full-site-editing' --stream --no-prefix

Have WPCOM sandbox running

Testing instructions

Once the env dev is set up:

Create a new page. The templates selector should appear.
Switch the template. See the template in the Large Preview.

WIP...

TODO

We need to test each starter template to see if the preview matches the editor view. (select the template, then apply the layout by selecting the top right "Use ___ Layout" button). If one of them is broken, please document:

which template it is,
which block is not rendering correctly,
and which theme you are using.
if this error is also in master or not.
if you consider this error blocking or not

Template Sections to Check

Other

Identify other bugs related to Starter Template preview that are not present in master
Fixing any identified bugs :)
core/cover block previewing
Post Title preview should respect theme settings for alignment (centered vs left aligned, etc)

matticbot · 2020-02-24T14:54:03Z

Test live: https://calypso.live/?branch=add/spt-dynamic-preview

matticbot · 2020-02-24T15:11:20Z

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.

simison · 2020-02-25T07:37:51Z

...ng-plugin/starter-page-templates/page-template-modal/components/template-selector-preview.js

+							content: __( 'Select a layout to preview.', 'full-site-editing' ),
+						} )
+						: [
+								createBlock( 'core/heading', {


Note that core/site-title is now available too, if it helps. :-)

makes a lot of sense. Thanks!

is it available?

I mean I'm not able to create a core/site-title block.

> wp.data .select( 'core/blocks' ) .getBlockTypes() .map( block => block.name ) .filter( name => name.match( /title/ ) );

it returns an empty [] array

Ah, you're right. We have Gutenberg 7.3.0 on wpcom currently (about to be updated to 7.6.0, p7DVsv-8c5-p2) and it likely doesn't have it yet.

Even once that's rolled out, I'd expect to have Atomic sites which are on an older versions of Gutenberg or have disabled it completely. I don't know if the whole FSE plugin with SPTs loads in that case tho.

@simison Can you spin out a separate Issue for this one so it can be tracked? I think it's unlikely that we'll tackle it here. Much appreciated.

noahtallen · 2020-02-25T21:31:07Z

It looks like the FSE plugin test failures are only because the behavior of FSE has changed :)

simison · 2020-02-26T21:35:12Z

...-plugin/starter-page-templates/page-template-modal/styles/starter-page-templates-editor.scss

+		}
+
+		// coblocks/gallery mock
+		.coblocks-gallery {


Could we please not include override-CSS like this if it's normally generated by JS? Let's solve the root problem instead.

It's going to be hugely problematic for keeping this CSS updated together with CoBlocks plugin version updates.

The JS also takes into account the viewport width, number, and size of images, so this CSS will need updating each time we change anything in the template or if we want to implement mobile previews, or just scale down the page in the browser window.

Yeah, it's a good point. It will be a tricky one to tackle.

I'm also nervous about handling Block specific overrides in this way.

Also, can I check this is definitely within the scope of this PR? If not then I'd recommend we split this out and handle separately in order to keep your PR super focused.

obenland · 2020-02-26T21:58:59Z

It's looking pretty good!

I noticed that in the Contact templates there are still some style issues (first, second, and fifth) and the Menu template is pretty broken.
The social media icons seem a bit off center and the linked-in one isn't always visible.
In the fifth contact page there is an empty paragraph block visible (Content…)

Could you rebase with master so the preview icons are back to being square etc?

getdave

I'm doing a first pass of this to try and get my head into it. As I did this I noticed some minor issues we could address together.

...editing-plugin/starter-page-templates/page-template-modal/components/block-iframe-preview.js

getdave · 2020-02-27T13:43:33Z

...-plugin/starter-page-templates/page-template-modal/styles/starter-page-templates-editor.scss

@@ -291,8 +281,7 @@ body.admin-bar:not( .is-fullscreen-mode ) .page-template-modal-screen-overlay {
 	right: $preview-right-margin;
 	background: $template-selector-empty-background;
 	border-radius: 2px;
-	overflow-x: hidden;
-	overflow-y: auto;
+	overflow: hidden;


Can you explain why this changed to a single value?

oh, it's not needed I think. Let me check it. Nice 👁

...-plugin/starter-page-templates/page-template-modal/styles/starter-page-templates-editor.scss

getdave · 2020-02-27T13:45:42Z

...-plugin/starter-page-templates/page-template-modal/styles/starter-page-templates-editor.scss

+		}
+
+		// coblocks/gallery mock
+		.coblocks-gallery {


I'm also nervous about handling Block specific overrides in this way.

Also, can I check this is definitely within the scope of this PR? If not then I'd recommend we split this out and handle separately in order to keep your PR super focused.

apps/full-site-editing/package.json

...editing-plugin/starter-page-templates/page-template-modal/components/block-iframe-preview.js

Resolves #39628

...editing-plugin/starter-page-templates/page-template-modal/components/block-iframe-preview.js

getdave · 2020-02-27T17:08:00Z

I'm concerned that this PR touches so many files that it could introduce regressions. For example I'm seeing the Contact Forms "placeholder" state showing up again in previews.

Just a note of caution as I know we're keen to land this one.

matticbot · 2020-02-27T19:45:23Z

Caution: This PR has changes that must be merged to WordPress.com
Hello retrofox! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D39555-code before merging this PR. Thank you!

This PR performs a refactoring in the templates previewing process, among other important changes. About the previewing, it moves the rendered blocks into an iFrame in order to apply CSS queries properly into the preview viewport. Other changes: remove the template title component and adding it as a template block for the preview, reduce CSS, etc.

matticbot · 2020-02-27T19:57:01Z

retrofox, Your synced wpcom patch D39555-code has been updated.

matticbot · 2020-02-27T20:33:51Z

retrofox, Your synced wpcom patch D39555-code has been updated.

matticbot · 2020-02-27T20:45:35Z

retrofox, Your synced wpcom patch D39555-code has been updated.

matticbot · 2020-02-27T21:38:04Z

retrofox, Your synced wpcom patch D39555-code has been updated.

simison · 2020-02-27T21:48:24Z

Aside, and no need in this PR, but you can now include many of these variables directly from a @wordpress/base-styles package:

wp-calypso/apps/full-site-editing/full-site-editing-plugin/starter-page-templates/page-template-modal/styles/starter-page-templates-editor.scss

Lines 14 to 38 in c034682

    
           $template-modal-background-color: #eeeeee; 
        
           $template-selector-border-color: #e2e4e7; 
        
           $template-selector-border-color-selected: #555d66; 
        
           $template-selector-border-color-active: #00a0d2; 
        
           $template-selector-border-color-hover: #c9c9ca; 
        
           $template-selector-empty-background: #fff; 
        
           $template-selector-modal-offset-bottom: 25px; 
        
           $template-selector-modal-offset-right: 32px; 
        
           $template-selector-blank-template-mobile-height: 70px; 
        
           $template-large-preview-title-height: 120px; 
        
           // Preview positioning 
        
           $preview-right-margin: 24px; 
        
           // Breakpoints 
        
           $breakpoint-mobile: 660px; 
        
           $breakpoint-tablet: 783px; 
        
           $breakpoint-desktop: 961px; 
        
           $breakpoint-huge: 1648px; 
        
           // WP.org sidebar and admin bar sizes 
        
           $wp-org-sidebar-full: 160px; 
        
           $wp-org-sidebar-collapsed: 36px; 
        
           $wp-org-admin-bar-full: 32px; 
        
           $wp-org-admin-bar-mobile: 46px;

matticbot · 2020-02-27T21:50:52Z

retrofox, Your synced wpcom patch D39555-code has been updated.

matticbot · 2020-02-28T16:33:58Z

retrofox, Your synced wpcom patch D39555-code has been updated.

matticbot · 2020-02-28T16:42:23Z

retrofox, Your synced wpcom patch D39555-code has been updated.

matticbot · 2020-02-28T17:03:54Z

retrofox, Your synced wpcom patch D39555-code has been updated.

matticbot · 2020-02-28T17:09:49Z

retrofox, Your synced wpcom patch D39555-code has been updated.

matticbot · 2020-02-28T17:16:42Z

retrofox, Your synced wpcom patch D39555-code has been updated.

matticbot · 2020-02-28T17:22:06Z

retrofox, Your synced wpcom patch D39555-code has been updated.

The component’s new behaviour means that the BlockPreview contents are moved into the iframe on render. This means the test cannot find it. Therefore we switch to assert on the prescence of the iframe.

matticbot · 2020-02-28T17:29:04Z

retrofox, Your synced wpcom patch D39555-code has been updated.

matticbot · 2020-02-28T17:31:46Z

retrofox, Your synced wpcom patch D39555-code has been updated.

obenland

Very nice job @retrofox! Thank you for your help @jeryj @getdave @frontdevde

getdave · 2020-02-28T17:39:52Z

Another enhancement idea would be to avoid those flickering images when we load into the preview. I think we could solve this by:

Preloading the images with rel="preload" - find all images in Blocks when we hover over the preview and then create <link rel="preload" ...> for each to preload the asset.
Adding a loading spinner whilst the Blocks are being populated into the preview. This isn't great but it's better than seeing flickering images and layout "jank" as the Blocks are loaded into the preview.

retrofox · 2020-02-28T17:40:50Z

Another enhancement idea would be to avoid those flickering images when we load into the preview. I think we could solve this by:

Preloading the images with rel="preload" - find all images in Blocks when we hover over the preview and then create <link rel="preload" ...> for each to preload the asset.

Adding a loading spinner whilst the Blocks are being populated into the preview. This isn't great but it's better than seeing flickering images and layout "jank" as the Blocks are loaded into the preview.

Among these changes, we should figure out how to reduce the images sizes

retrofox requested a review from a team as a code owner February 24, 2020 14:54

retrofox added the [Type] Enhancement label Feb 24, 2020

simison reviewed Feb 25, 2020

View reviewed changes

marekhrabe assigned retrofox Feb 25, 2020

retrofox requested a review from a team as a code owner February 25, 2020 16:52

obenland mentioned this pull request Feb 25, 2020

Layouts: Rockfield preview images not filling canvas #39351

Closed

simison reviewed Feb 26, 2020

View reviewed changes

getdave reviewed Feb 27, 2020

View reviewed changes

getdave added a commit that referenced this pull request Feb 27, 2020

Fix variable spelling

97739d8

Resolves #39628

getdave reviewed Feb 27, 2020

View reviewed changes

...editing-plugin/starter-page-templates/page-template-modal/components/block-iframe-preview.js Outdated Show resolved Hide resolved

obenland changed the title ~~[wip] SPT: preview into an iFrame element~~ SPT: preview into an iFrame element Feb 27, 2020

retrofox force-pushed the add/spt-dynamic-preview branch from ac40e9b to 03e9464 Compare February 27, 2020 19:42

retrofox force-pushed the add/spt-dynamic-preview branch from 03e9464 to 6a26bb5 Compare February 27, 2020 19:53

spt: clean code

765e3a7

spt: rename debounce constant

f804fe3

Restore square preview images and Safari fix

c034682

Clarify width attribute

8e7d478

Adds temporary CSS fixes for jetpack slider in the iframe preview

e4769dd

Make sure swiper pagination bullets are centered

fbf80e0

Removes margins on cover blocks in iframe preview

fe4582e

Removes empty paragraph placeholder text from iframe preview

6f96a92

getdave added 3 commits February 28, 2020 17:25

Guard against store not being available

9b600ce

Fix Template Preview test to account for iframe behaviour

ace2e50

The component’s new behaviour means that the BlockPreview contents are moved into the iframe on render. This means the test cannot find it. Therefore we switch to assert on the prescence of the iframe.

Remove old snapshot

4afe96a

obenland approved these changes Feb 28, 2020

View reviewed changes

retrofox requested review from getdave, obenland, jeryj, marekhrabe, frontdevde and a team February 28, 2020 17:34

retrofox merged commit 8159ada into master Feb 28, 2020

retrofox deleted the add/spt-dynamic-preview branch February 28, 2020 17:54

This was referenced Mar 2, 2020

SPT: Tweak template title. #39805

Closed

SPT: Re implement template post title. #39809

Closed

jeryj mentioned this pull request Mar 10, 2020

Fix Coutoire in the template selector preview #37685

Closed

obenland mentioned this pull request Mar 11, 2020

SPT: Menu page template is cropped and has an extra scrollbar #36901

Closed

SPT: preview into an iFrame element #39628

SPT: preview into an iFrame element #39628

Conversation

retrofox commented Feb 24, 2020 • edited Loading

Changes proposed in this Pull Request

Related issues:

Dev Setup For Testing

Testing instructions

TODO

matticbot commented Feb 24, 2020

matticbot commented Feb 24, 2020

simison Feb 25, 2020 • edited Loading

Choose a reason for hiding this comment

retrofox Feb 25, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noahtallen commented Feb 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

obenland commented Feb 26, 2020

getdave left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

getdave commented Feb 27, 2020

matticbot commented Feb 27, 2020

matticbot commented Feb 27, 2020

matticbot commented Feb 27, 2020

matticbot commented Feb 27, 2020

matticbot commented Feb 27, 2020

simison commented Feb 27, 2020

matticbot commented Feb 27, 2020

matticbot commented Feb 28, 2020

matticbot commented Feb 28, 2020

matticbot commented Feb 28, 2020

matticbot commented Feb 28, 2020

matticbot commented Feb 28, 2020

matticbot commented Feb 28, 2020

matticbot commented Feb 28, 2020

matticbot commented Feb 28, 2020

obenland left a comment

Choose a reason for hiding this comment

getdave commented Feb 28, 2020

retrofox commented Feb 28, 2020

retrofox commented Feb 24, 2020 •

edited

Loading

simison Feb 25, 2020 •

edited

Loading

retrofox Feb 25, 2020 •

edited

Loading