Wp-calypso: FSE: incorrect preview for Reynolds layout in onboarding

[vindl]

Steps to reproduce

1. Start the Horizon FSE flow
2. Check out the look of Reynolds layout on "Choose Design" step
3. Notice the big gap before the cover block text starts that's not present on the front end.

This was working fine when we launched the flow but was changed in the meantime. I first noticed it during last week.

[jeyip]

TL;DR The preview is a generated screenshot. The screenshot is based on a demo view for Reynolds-FSE seen here, but the demo is pulling in incorrect theme styles (Rockfield instead of Seedlet).

Context

• The "Choose Design" step at https://horizon.wordpress.com/new/design uses a screenshot for the preview
• Other steps like "Pick a Font Pairing" at https://horizon.wordpress.com/new/style do not see the same problem because they do not use screenshots
• Code to generate the screenshot can be found at bin/generate-gutenboarding-design-thumbnails.js
  
  
  
  • The screenshot for Reynolds-FSE is captured from the v1 WordPress Rest API here
  
  
  • To emulate the problematic formatting, open the web page hosted by the v1 WordPress Rest API. Open the chrome dev console, press cmd + shift + p, type in full size screenshot, press enter, and view the sad, sad PNG ☹️
  
  

Cause

• In bin/generate-gutenboarding-design-thumbnails.js L68-L73, a comment identifies a similar issue the traditional (as opposed to block-based) Reynolds design had.
• The cause can be traced back to the Rockfield theme, which applies a min-height: 90vh that stretches the cover image in a full sized screenshot. The comment notes the temporary fix they implemented.
• While I was experimenting, the same temporary fix addressed the problem for Reynolds-FSE. This was strange to me because Reynolds-FSE uses the Seedlet theme, not Rockfield.
• It turns out that the demo view for the Reynolds-FSE design has the Rockfield theme styles applied.

Potential Solutions

• Quick and easy: Add Reynolds-FSE to the temporary fix written previously for Reynolds (super quick 1-liner) and regenerate the screenshots
• Involved and thorough: Apply Seedlet styles to the Reynolds-FSE demo view served by the v1 WordPress Rest API

[noahtallen]

Involved and thorough: Apply Seedlet styles to the Reynolds-FSE demo view served by the v1 WordPress Rest API

What makes this step more involved? Do we not have a way to set the theme demo site URL which should be used for generating some of these screenshots?

If this step will be eventually solved by patterns API work (cc @Automattic/ganon), then I vote the quick and easy fix :)

[jeyip]

Do we not have a way to set the theme demo site URL which should be used for generating some of these screenshots?

The demo site URLs are generated programmatically based on the design slug, template, and theme. If we want to point Reynolds-FSE at a different URL for screenshot generation, we definitely could, but it seems like we would need a conditional specifically for our case (maybe not the biggest deal).

We'd also need a demo view that is loading the Reynolds-FSE demo view with the correct Seedlet styling (which might already exist...I wasn't sure where that might be, but I can investigate) since the existing Reynolds-FSE demo is incorrectly using Rockfield.

[noahtallen]

it feels like this function should be encoding the right theme?? I think I must be missing something!

https://github.com/Automattic/wp-calypso/blob/e1f4361c647e8341a509ea2acca35e7a5c181327/bin/generate-gutenboarding-design-thumbnails.js#L39-L49

[jeyip]

it feels like this function should be encoding the right theme??

Maybe I can provide better clarification. I think the URL is correct. It's what is served at the demo view that is strange. If you visit https://public-api.wordpress.com/rest/v1/template/demo/seedlet-blocks/reynolds?font_base=Fira%20Sans&font_headings=Playfair%20Display&site_title=Reynolds%20%28FSE%29, the demo view used for the Reynolds-FSE screenshot, there are styles being pulled in from the Rockfield theme which are causing the stretched preview.

I was concluding that the demo view itself was applying the incorrect theme styling to the Reynolds-FSE design. If we examine the Reynolds-FSE design in the site editor frontend, and _not_ the demo view, Seedlet theme styles are used, and not Rockfield. Additionally, full sized screenshots in the site editor frontend _do not_ have problems with the big gap.

Let me know if that doesn't make sense, and thanks for sticking with me 🙂.

[noahtallen]

I was concluding that the demo view itself was applying the incorrect theme styling to the Reynolds-FSE design. If we examine the Reynolds-FSE design in the site editor frontend, and not the demo view, Seedlet theme styles are used, and not Rockfield

Ahhhh that makes sense! Thanks for an extra round of clarification.

So it sounds like there is a problem in the template demo API which is somehow not referencing the correct theme demo site or something like that 🤔

[jeyip]

So it sounds like there is a problem in the template demo API which is somehow not referencing the correct theme demo site or something like that 🤔

Yeah that's exactly what I was thinking 👍

[ramonjd]

So it sounds like there is a problem in the template demo API which is somehow not referencing the correct theme demo site or something like that

If you need reynolds-fse to reference a different theme demo site (or starter site), then you can specify that in the template property

Each design has its own unique starter site. Reynolds is based on the Rockfield theme, so that's why you're seeing Rockfield styles.

If you need a new starter site , with a different base theme for reynolds-fse then you can create a new starter site. See: PCYsg-r7w-p2

And then reference it in the backend. See: 261d1-pb

The process is intolerably manual and cumbersome - something we hope to rectify one day soon as mentioned above.

[jeyip]

@ramonjd Thanks for the clarification! Much appreciated. Your explanation looks like it should solve our problem, but I had another question.

@noahtallen had mentioned that this problem might eventually solved by patterns API work. I've read up a little bit on the topic, which appears to be focused on setting up an endpoint to serve translated design previews. It seems like this issue may be out of project scope because it involves creating a brand new starter site, but I wanted to double-check before moving forward.

[noahtallen]

Each design has its own unique starter site. Reynolds is based on the Rockfield theme, so that's why you're seeing Rockfield styles.

Ah, we were assuming that a "template" (page pattern, or whatever) would just specify starter content, not that it needed its own site.

seems like this issue may be out of project scope

I believe patterns API would make the process of specifying page patterns to use much easier. (Currently stuff is fairly hard-coded). So it would still require some work, I guess :)

So I think we should move forward with the fix @ramonjd mentions. I think you would need to create a site via the FSE test flow, and then maybe do an update on the backend similar to: D48207-code?

We should probably do the same thing for the other test-fse theme, just to be sure that each of our test-fse designs uses correct layout + theme for styles!

[ramonjd]

I believe patterns API would make the process of specifying page patterns to use much easier. (Currently stuff is fairly hard-coded). So it would still require some work, I guess :)

Yep! That's about right 👍

We'll still have to create a "starter site". After all, we'll need a place to store and edit the source material.

One of the core differences will be that we'll query the starter sites directly for content, and translate that content dynamically.

This means no more storing block patterns in JSON files. 🎉

The whole faff of generating a home page "template" JSON file on top of a headstart annotation is a vestige from Gutenboarding v1; it will be mercilessly ripped out of the site creation process as part of migrating over to the patterns API.

Work on streamlining headstarting sites with translated content is happening right now (cc @deBhal)

appears to be focused on setting up an endpoint to serve translated design previews

That's right in relation to the MVP, but the real objective is to serve translated block patterns. Any block pattern. Think Gutenberg patterns, entire layouts, even single blocks.

The implication is that we can maintain one block pattern, but offer it to our users in n languages.

[noahtallen]

One of the core differences will be that we'll query the starter sites directly for content

will we have to have a demo site for each different possible design? For example, we were just trying to reuse the "reynolds" template with a different theme. Do we have to recreate anything from the existing reynolds stuff, or just activate the reynolds layout on the homepage of a new site with the different theme?

[ramonjd]

will we have to have a demo site for each different possible design?

I don't know the answer to this yet. 🤷

For delivering translated blocks, I would say "no". All we care about is the raw block content.

For showing accurate design previews, especially where the base themes differ.... maybe? Unless we build a more modular preview system whereby we can swap theme styles in and out.

[noahtallen]

Unless we build a more modular preview system whereby we can swap theme styles in and out.

I feel like I'd seen something where you just have the theme demo site, and then you just change the content div to match the content of the "page pattern" via PHP 🤔 Maybe an older iteration?

[ramonjd]

I feel like I'd seen something where you just have the theme demo site, and then you just change the content div to match the content of the "page pattern" via PHP

I _think_ that's the way we're doing it now 🤔 So purely for post content I guess it should be fine.

Is there anything else we'd need to toggle? Theme-specific CSS/JS maybe?

I'm always thinking of the design sites as an entity, where the content structure, including menus, pages, post are part of that design (together with whatever arrangement of blocks they feature) 😛

[noahtallen]

Is there anything else we'd need to toggle? Theme-specific CSS/JS maybe?

Hm, not sure, I mean I'd think that the theme-specific CSS/JS would need to stay the same if the theme was to be activated, otherwise it wouldn't look like the end result 🤔

[jeyip]

That's right in relation to the MVP, but the real objective is to serve translated block patterns. Any block pattern. Think Gutenberg patterns, entire layouts, even single blocks.

The implication is that we can maintain one block pattern, but offer it to our users in n languages.

I see. Thanks for clarifying Ramon!

So I think we should move forward with the fix @ramonjd mentions.

I agree. 👍

I think you would need to create a site via the FSE test flow, and then maybe do an update on the backend similar to: D48207-code?

We should probably do the same thing for the other test-fse theme, just to be sure that each of our test-fse designs uses correct layout + theme for styles!

I'll go ahead and create an FSE site for Reynolds and follow up with Vesta in a different PR.

[ramonjd]

I'll go ahead and create an FSE site for Reynolds and follow up with Vesta in a different PR.

Let us know if you need a review/testing! Or add @Automattic/ganon to your review list 👍 🙇

[jeyip]

Question about the Reynolds FSE design thumbnail. Do we want the default header and footer template parts to be displayed in the preview? In the past, they've been hidden.

It makes sense to me to provide a true-to-life sneak-peek, but I'm curious what others think @noahtallen @vindl

[noahtallen]

@jeyip could you clarify this?

Do we want the default header and footer template parts to be displayed in the preview? In the past, they've been hidden.

I don't totally follow where they are being hidden :)

[noahtallen]

When I look at the existing design in Gutenboarding:

it shows the header associated with the theme it was using. I think we want the same behavior here, but the headers will be from seedlet-blocks instead.

Maybe I'm missing something?

[vindl]

It makes sense to me to provide a true-to-life sneak-peek, but I'm curious what others think @noahtallen @vindl

We were showing them in the test flow initially so we should continue doing that.

[jeyip]

When I look at the existing design in Gutenboarding:
it shows the header associated with the theme it was using.

We were showing them in the test flow initially so we should continue doing that.

Oh I see now! It was a misunderstanding on my part.

1. I thought that the original Reynolds(FSE) thumbnail didn't have a header, but it does. (The black bar at the top)
2. Because I thought the original thumbnail didn't have a header, I was wondering if the updated thumbnail should follow the same design pattern. The original thumbnail, however, _does_ have a header, so we _should_ display the header from seedlet blocks.

Thanks for the clarification everyone! 🙏