FSE: Add a default logo placeholder image

[glendaviesnz]

There is no code to review for this issue as the header template has been updated in the fsemodernbusiness site.

Steps to test

1. Set up a new FSE site either locally or via the Horizon FSE CfT steps
2. Check that default modern business logo is displaying in the header instead of the image placeholder

before

after

[gwwar]

@apeatling the site logo placeholder is pretty prominent. Should we maybe populate the design with a logo to start with to avoid these screens? cc @iamtakashi @shaunandrews

[apeatling]

Yes we should, this has come up a few times in feedback. I think we should actually update this issue to reflect that.

@iamtakashi Should we use the placeholder logo from the modern business proofs?

[apeatling]

I probably should have opened a new issue instead, but this will work for now.

[iamtakashi]

I agree with starting with an actual logo image rather than a placeholder. But, would FSE themes header always start with having a logo? Or does it depend on demo site?

[apeatling]

It will depend on the header template being used, so it could be different depending on what the theme needs.

[iamtakashi]

Ok, and we'll define the header template in the demo site, correct?

[apeatling]

I think we can do that, but it's not really in scope for this discussion, we just need to decide what the best default logo is in the context of Modern Business.

[iamtakashi]

Sure, we can use the logo in the demo site instead of the placeholder.

[glendaviesnz]

@apeatling Can you please point me to the proofs so I can get a copy of the logo mentioned?

[apeatling]

@iamtakashi The logo in the demo site looks fuzzy on a retina display, do we have a 2x version of it? Is this going to be different in Maywood?

@glendaviesnz You can start with the logo at the top of https://modernbusinessdemo.wordpress.com

[gwwar]

What's left on this one and is this blocking for 10% testing?

[glendaviesnz]

As far as I can tell there are no code changes required for this. We just need to update the header template content and change the image block from

<!-- wp:image {"className":"fse-site-logo"} -->
<figure class="wp-block-image fse-site-logo"><img alt=""/></figure>
<!-- /wp:image -->

to

<!-- wp:image {"className":"fse-site-logo"} -->
<figure class="wp-block-image size-large fse-site-logo"><img src="https://modernbusinessdemo.files.wordpress.com/2019/06/logo-modern-3.png" alt=""/></figure>
<!-- /wp:image -->

Just need to hunt down that code for snapshooting the template data ...

[glendaviesnz]

@qwwar I can't access the templates any more at fsemodernbusiness/wp-admin/edit.php?post_type=wp_template - gives a console error - did we move these to the main modernbusiness demo site already?

For future reference details about how to snapshot the new header with default image at D31195-code

[glendaviesnz]

So for some reason the patch D31361-code had broken access to the fse demo site for me, I think maybe because I had classic editor set for my user from past edits of the templates. I can get in and edit them my temporarily reverting that on my sandbox - @gwwar - do you want me to go ahead and add that logo to the header template on fsemodernbusiness?

I have added and issue for the problem of forcing gutenberg for users that have chosen classic - #35804

[gwwar]

do you want me to go ahead and add that logo to the header template on fsemodernbusiness?

Sure let's do that for now, though we'll also need to make sure we reapply changes to Maywood when we switch over.

[glendaviesnz]

This is now done and the default modern business logo now displays instead of the placeholder if you go through the FSE setup flow

[noahtallen]

Just went through the CfT flow again, and this is definitely working as expected!

For follow up, I guess we just need to make sure the templates for maywood/etc also do this.

I guess I'll close this since there's no merging to do :)

[gwwar]

@glendaviesnz can we update the header block property on the image to add an explicit align center?

<!-- wp:image {"align":"center","className":"size-large fse-site-logo"} -->
<div class="wp-block-image size-large fse-site-logo"><figure class="aligncenter"><img src="https://modernbusinessdemo.files.wordpress.com/2019/06/logo-modern-3.png" alt=""/></figure></div>
<!-- /wp:image -->

I'm seeing the following currently while testing Automattic/themes#1341

It also looks like we're hotlinking. Not sure if we need to tidy before the 10% test, but we should for 100%. @obenland any ideas for what we'd need to do for side-loading? At worst, this looks small enough where even a base64 encoded image would be preferable.

[apeatling]

I've re-pinged Takashi on this one, it needs to be a 2x version of this image.

[iamtakashi]

@iamtakashi The logo in the demo site looks fuzzy on a retina display, do we have a 2x version of it? Is this going to be different in Maywood?

Sorry for the delay in response. We could use a different logo for Maywood. It's not a big deal if that's not convenient though. Here is a big version of a new logo. Let me know if there is any issue.

[glendaviesnz]

It also looks like we're hotlinking. Not sure if we need to tidy before the 10% test, but we should for 100%. @obenland any ideas for what we'd need to do for side-loading?

There is code at /full-site-editing/full-site-editing-plugin/starter-page-templates/class-wp-rest-sideload-image-controller.php for side-loading images, but would be a bit of work to get this working in tandem with the fse template creation api - I would suggest we defer for the 100%

At worst, this looks small enough where even a base64 encoded image would be preferable.

Have tried base64 encoded data: version of the new logo but on saving the base64 data is stripped - I am guessing the the KSES filters remove this.

So, any thoughts on whether we just make do with a hotlinked image for the 10% trial, or is there another way to get base64 encoded version accepted?

[apeatling]

Hot linking seems fine for the test, since we are still hot linking other images in templates right now (until we figure out side loading).

[apeatling]

@glendaviesnz Let's update this to use the high res logo Takashi provided.

[obenland]

Hot linking seems fine for the test, since we are still hot linking other images in templates right now

I'd strongly advise against hotlinking more images. We'll never be able to change or remove those images once we start doing that, even for a small amount of sites. It's part of the reason why I've held back updating those template images with optimized versions so far.

[obenland]

#35588 added the utility functions to communicate with the endpoint, #34839 has the code that integrates the side-loading endpoint in the templates selector. We've had a a fair bit of performance troubles with it, especially for multiple images though.

[apeatling]

Thanks @obenland, that's a good point. I'm concerned about the performance issues of side loading as a blocker for the test.

[obenland]

If it's just this one image with a size of 35kb it should be fine. We've not had many problems with the blog template so far, Professional with its 12+ images is where it becomes pretty bad. I do realize though that implementing that sideloading approach might delay the test.

[gwwar]

I'll create a new issue for the hotlinking fix, since that sounds like a decent amount of work.

[glendaviesnz]

@gwwar have added the explicit centre alignment now.

[gwwar]

Thank @glendaviesnz verified that this populates with the attribute on a new site:

Closing this out and will add a second one for the hotlinking

[apeatling]

@gwwar @glendaviesnz Based on the above, it doesn't look like the new logo Takashi linked? The hotlink URL needs to be updated to:

https://maywooddemo.files.wordpress.com/2019/08/maywood-logo.png

With the appropriate size set.