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

Gutenboarding: Display FSE design using seedlet-blocks #43349

Merged
merged 14 commits into from
Jun 19, 2020

Conversation

noahtallen
Copy link
Contributor

@noahtallen noahtallen commented Jun 16, 2020

This resolves #43288, since you will be able to activate seedlet-blocks internally via Gutenboarding in staging environments. This also resolves #43285, which adds a design which when chosen, will activate FSE on the new site.

This diff is the companion on wpcom and has already been deployed: D45075-code.

Changes proposed in this Pull Request

  • Add is_fse flag and seedlet-blocks theme to a design
  • Fix issue where selecting a premium plan did not redirect you to the site editor
  • Added selector for FSE site now that there are multiple ways to enter the flow
  • Instead of signup flow option, use new enabled_fse option to create the site. This way, the backend still considers the ultimate flow to be gutenboarding. (see code-D45075 as well.)
  • When the gutenboarding/site-editor flag is set, convert every design to use is_fse and seedlet-blocks as the theme.

This is currently blocked on #43415, which needs deployed so that the seedlet demo site begins working again. Once that is working, we can correctly generate previews. Currently previews do not work because the demo site does not render a post content block: (Previews are now working.)

Second, we need the is_edge config option in #42354 to restrict the design to certain environents. Otherwise this will start working in prod. (This has been completed.)

Known issues:

  • Font options do not work with the seedlet-blocks theme.
  • When loading the site editor for the first time, the incorrect template is used for the homepage, meaning that you do not see the post content of your selected design. This is fixed by FSE: Move initial template fetch to client WordPress/gutenberg#23186, which will arrive on dotcom in the coming weeks.

Testing instructions

  • (if you are sandboxed, make sure it is up to date. being sandboxed is no longer a requirement)
  • Go through http://calypso.localhost:3000/new
  • Select the first design option (named "FSE Test Design")
  • Select the premium plan
  • Verify that you are redirected to the site editor after getting the premium plan
  • Verify that the seedlet theme is active on your site.
  • Verify that the gutenberg-edge sticker is applied to your site.
  • Verify that the homepage post_content matches the Cassel page pattern (it should not match the seedlet-blocks default homepage content.)

Ålso test:

@noahtallen noahtallen added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Goal] Full Site Editing [Goal] New Onboarding previously called Gutenboarding labels Jun 16, 2020
@noahtallen noahtallen requested a review from a team as a code owner June 16, 2020 18:42
@noahtallen noahtallen self-assigned this Jun 16, 2020
@matticbot
Copy link
Contributor

@noahtallen noahtallen requested a review from a team June 16, 2020 18:42
@simison simison requested a review from a team June 16, 2020 18:48
@matticbot
Copy link
Contributor

matticbot commented Jun 16, 2020

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~171 bytes added 📈 [gzipped])

name                 parsed_size           gzip_size
entry-gutenboarding       +850 B  (+0.0%)     +171 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@simison
Copy link
Member

simison commented Jun 16, 2020

What do you think about overriding this at here:

const availableDesigns: Readonly< AvailableDesigns > = availableDesignsConfig;

And using script to generate new thumbnails with Seedlet theme:

https://github.com/Automattic/wp-calypso/blob/master/bin/generate-gutenboarding-design-thumbnails.js

Thumbnail filenames contain theme name so you won't end up overriding existing ones.

That way everything should fall into place nicely, including site creation with correct theme.

You'll need theme demo site for the preview mechanism to work tho!

@noahtallen
Copy link
Contributor Author

I definitely like that idea! I think theme demo site is the missing part. if that is available, will it work for template API and for mshots?

@simison
Copy link
Member

simison commented Jun 16, 2020

Yep!

@noahtallen noahtallen changed the title Use seedlet-blocks all the time if FSE is enabled Gutenboarding: Create FSE design config using seedlet-blocks Jun 16, 2020
@ockham
Copy link
Contributor

ockham commented Jun 16, 2020

I definitely like that idea! I think theme demo site is the missing part. if that is available, will it work for template API and for mshots?

@ianstewart Can you help us set up a demo site for seedlets-blocks?

@noahtallen
Copy link
Contributor Author

What do you think about overriding this at here:

done. I copied the existing design file and changed the theme property to seedlet-blocks and also changed the src to reference seedlet instead of the theme it had set.

@ianstewart
Copy link
Contributor

Can you help us set up a demo site for seedlets-blocks?

Does it need to 1) involve the showcase in any way? Or 2) does it just need to be any site on WP.com with any theme that has Headstart annotations.

If it's number one … we might need a way to hide the theme from non-automatticians since it's not ready for launch. If it's number two … for sure.

@noahtallen
Copy link
Contributor Author

@ianstewart I don't know... it needs to work for Gutenboarding. So with mshots and the template API.

The theme activates correctly via the API already :) The only time the showcase appears to be broken right now is if the theme is already active on the site (because its preview image doesn't show up)

does it just need to be any site on WP.com with any theme that has Headstart annotations.

This sounds true? I'm not sure what this distinction is.

@ianstewart
Copy link
Contributor

I set up a temporary demo site and generated temporary Headstart annotations as well.

https://seedletblocksdemo.wordpress.com/

The front-end doesn't look as expected currently but I believe that's just a known issue with our demo templates in the site editor on WP.com.

@noahtallen noahtallen requested a review from a team as a code owner June 16, 2020 21:31
@noahtallen
Copy link
Contributor Author

TODO: integrate with changes in #42354

@ramonjd
Copy link
Member

ramonjd commented Jun 17, 2020

TODO: integrate with changes in #42354

You might get there first :P

@noahtallen
Copy link
Contributor Author

The problem with previews in this PR is related to two issues:

The seedlet demo site has some kind of template issue and doesn't render a static homepage. To fix:

a) deploy #43334
b) possibly wait for new Gutenberg plugin version to match latest bug fixes from yesterday
c) update the demo site via site editor to fix the template issue whatever it is
d) run preview update script again here
e) cross fingers and hope it all works

Additionally, it seems that the seedlet blocks theme is not using gutenboarding headstart content or whatever shenanigans occur there, so all the content is started in classic editor content mode 🕺

@noahtallen
Copy link
Contributor Author

You might get there first

it looks so close! :D

@ramonjd
Copy link
Member

ramonjd commented Jun 17, 2020

seedlet blocks theme is not using gutenboarding headstart content or whatever shenanigans occur there, so all the content is started in classic editor content mode

Looking at the seedletblocksdemo site, it appears seedlet blocks is its own theme. I suspect that there are no headstart annotation for headstart to use so it's using some old fallback. (?)

You could try tricking headstart into using a different annotation by passing it a vertical slug. I haven't tested it, but I'm talking about making it use m1.json etc that you'll find under nux-helpers.

@noahtallen
Copy link
Contributor Author

I suspect that there are no headstart annotation for headstart to use

I think @ianstewart was able to fix this! 💙

@ianstewart
Copy link
Contributor

ianstewart commented Jun 17, 2020

I think @ianstewart was able to fix this! 💙

Yep. I made and deployed one. If anyone runs into an issue just ping me in Slack or here. I'm familiar enough with Headstart commands to make updates quickly.

@noahtallen
Copy link
Contributor Author

noahtallen commented Jun 17, 2020

btw, generating working FSE design previews is blocked on #43415

@noahtallen noahtallen force-pushed the try/gutenboarding-seedlet-blocks branch from d456c3c to 817e448 Compare June 17, 2020 23:27
@ockham
Copy link
Contributor

ockham commented Jun 18, 2020

Thanks! This is great!

  • Instead of signup flow option, use new enabled_fse option to create the site. This way, the backend still considers the ultimate flow to be gutenboarding. (see code-D45075 as well.)

I ❤️ this in particular -- your choice to use site options is much better (as it preserves other Gutenboarding-specific customizations) than mine was.

  • When the gutenboarding/site-editor flag is set, convert every design to use is_fse and seedlet-blocks as the theme.

I'm a bit skeptical about this -- IIUC, it will also require re-running the thumbnails generation script? I think that's easy to miss -- I'd rather make the available designs JSON the complete source of truth; for a given available designs JSON (and no changes to the designs themselves), running the generator script should reproducibly yield the same result. If we want seedlet-based themes, we'll need to add them to the JSON -- I think that's a valid requirement.

@ockham
Copy link
Contributor

ockham commented Jun 18, 2020

I'm a bit skeptical about this -- IIUC, it will also require re-running the thumbnails generation script? I think that's easy to miss -- I'd rather make the available designs JSON the complete source of truth; for a given available designs JSON (and no changes to the designs themselves), running the generator script should reproducibly yield the same result. If we want seedlet-based themes, we'll need to add them to the JSON -- I think that's a valid requirement.

(The level of indirection and number of wrapped generator scripts is IMO a symptom of this.)

@noahtallen
Copy link
Contributor Author

Though I do agree with you about the level of indirection, I think it practically works:

it will also require re-running the thumbnails generation script

Ah I see what you mean. I fixed this issue in, c5f3f2d. So the generation script will work with all possible design options.

Anyways, the motivation for this is that if you have the flag set, you'll end up with a site editor, but no block based theme and no demo templates. So in that scenario, I think you'll have a broken site.

We could get rid of the flag? 😅

@ockham
Copy link
Contributor

ockham commented Jun 18, 2020

Though I do agree with you about the level of indirection, I think it practically works:

it will also require re-running the thumbnails generation script

Ah I see what you mean. I fixed this issue in, c5f3f2d. So the generation script will work with all possible design options.

Anyways, the motivation for this is that if you have the flag set, you'll end up with a site editor, but no block based theme and no demo templates. So in that scenario, I think you'll have a broken site.

We could get rid of the flag?

Oh, okay, that rationale was lost on me. Well could we hide the non-block based themes if the flag is set?

@noahtallen noahtallen force-pushed the try/gutenboarding-seedlet-blocks branch from c5f3f2d to f9f2972 Compare June 18, 2020 23:19
@noahtallen
Copy link
Contributor Author

Well could we hide the non-block based themes if the flag is set?

Based on this, I've changed it so that enabling the flag filters the designs to be only the FSE ones. The FSE designs still show up normal horizon/wpcalypso, but the flag just removes the other ones. That way, any option the user chooses with the flag set will get them to an FSE site.

@ockham
Copy link
Contributor

ockham commented Jun 18, 2020

Thanks a lot for the latest round of changes that removed the forceFSE option, as discussed in Slack (p1592521647367900-slack-cylon)!

Code-wise, this is looking great now. One more question, can we remove some of the regenerated thumbnails that are part of this PR? I assume we only need the reynolds-fse and vesta-fse ones now?

@noahtallen
Copy link
Contributor Author

can we remove some of the regenerated thumbnails that are part of this PR? I assume we only need the reynolds-fse and vesta-fse ones now

sure :)

@noahtallen noahtallen force-pushed the try/gutenboarding-seedlet-blocks branch from 8fb6f9a to 4cd4e98 Compare June 18, 2020 23:54
@noahtallen
Copy link
Contributor Author

I assume we only need the reynolds-fse and vesta-fse ones now?

done.

@ockham
Copy link
Contributor

ockham commented Jun 19, 2020

I just tested this locally without any of the new feature flags, but was shown the new FSE designs in the design selector:

image

@noahtallen
Copy link
Contributor Author

Yes, that's intentional :) We can change that behavior if we want to. Note that these designs are also "alpha" which means they won't show up on prod ;)

@noahtallen
Copy link
Contributor Author

noahtallen commented Jun 19, 2020

Yes, that's intentional

I'm walking back on this now. Since the site editor isn't totally usable right now due to a bug in Gutenberg (the wrong template is used for the homepage in the editor), I'm fine hiding a way into that flow by default for the time being.

Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

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

Tested this and it works well! Awesome stuff!

Both creation flows worked as expected (with and without the flag). And now that the flag is required the designs do not show up without it.

@noahtallen noahtallen changed the title Gutenboarding: Create FSE design using seedlet-blocks Gutenboarding: Display FSE design using seedlet-blocks Jun 19, 2020
@noahtallen noahtallen merged commit d9526a6 into master Jun 19, 2020
@noahtallen noahtallen deleted the try/gutenboarding-seedlet-blocks branch June 19, 2020 00:49
@noahtallen
Copy link
Contributor Author

Feel free to continue improving this flow if needed :) Props to the gutenboarding team for making it so easy to work with!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Goal] Full Site Editing [Goal] New Onboarding previously called Gutenboarding [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
8 participants