Wp-calypso: Layouts: Replace Home Page Previews with static images

[ianstewart]

~We can see that they're too short for the space given to display them.~

Edit 6th March:

This issues has been changed from fixing issues with aspect ratios into replacing mshot previews with static images. We need to find a place where to store previews (should be the same as vertical layout screenshots) and we need to update our backend code to return those images instead of generating mshot link.

Images: p1582672574371100-slack-CJS75TX3R

Place to update code in backend - similar as D39359-code

Fixes #38064.

[obenland]

That is an mshot issue. We might be able to leverage the existing workaround for demoblog banners to not show those notifications on mshot requests

[ianstewart]

Are the other page templates using mShots?

[obenland]

To my knowledge, homepage templates are the only ones

[ianstewart]

Perhaps skip using mshots altogether? And just provide thumbnails similar to the other layouts? It might help the home page layouts from looking so much like … themes. cc @iamtakashi

[obenland]

We could use headstartdata for homepage templates as well. At the time we added them we only had a8ctm1 and wanted to avoid having assets all over the place.

[iamtakashi]

Perhaps skip using mshots altogether? And just provide thumbnails similar to the other layouts? It might help the home page layouts from looking so much like … themes

I have sent @apeatling static screenshots of the homepages with Maywood a while ago.

[ianstewart]

I think (for now, until we have dynamic previews or something better) that static screenshots will be best.

[obenland]

@iamtakashi Do you still have access to those static screenshots? Would you mind making them accessible here?

[marekhrabe]

This is two separate issues mixed together, I'll extract the visible notice into #39669 and let's solve the screen ratio here.

We can fix the thumbnail size ratio by properly requesting a screenshot from mshots. Reading through mshots source code, I can see it has parameters to configure the viewport width and height (vpw and vph), as well as the width and the height (width and height) of resulting image.

Example URL that does a square thumbnail of theme demo site: https://s.wordpress.com/mshots/v1/https%3A%2F%2Falvesdemo.wordpress.com%2F%3Ftheme_preview%3Dtrue?w=300&height=300&vpw=1300&vph=1300

Can somebody from design play with vpw and vph to make sure screenshots look their best for the selector? Once we have determined the right size, I can make a PR that updates it. 1300 looks ok to me but I'll leave that decision up to you to confirm

[obenland]

Nice find! Do the width/height and vpw/vph sets need to be in the same aspect ratio for this to work?

They seem to have at least some kind of limit, this request returns a 3x4 image again instead of a 4x3:
https://s.wordpress.com/mshots/v1/https%3A%2F%2Falvesdemo.wordpress.com%2F%3Ftheme_preview%3Dtrue?w=300&height=400&vpw=3000&vph=4000

[marekhrabe]

Do the width/height and vpw/vph sets need to be in the same aspect ratio for this to work?

They can be in different ratios and in such case, the screenshot will be cropped and some parts will end up being outside the canvas.

They seem to have at least some kind of limit

You have tried really large numbers (and it looks like I did too with 1300) - there is a cap:

https://github.com/Automattic/mShots/blob/c141f2058027c849a82c1e4792ff7baf8a0bb098/lib/worker.js#L18-L19

Since the max height is 1200, I think that makes our decision easier. Let's just go with 1200x1200 square for now. From few tests, demos like ok in that resolution to me. Agreed?

[marekhrabe]

I'm still getting some strange ratio issues as you've described even with lower sizes. Looking into it…

[marekhrabe]

960 ended up being the best number that worked without any unexpected glitches. There might be a bug in mshots that prevents anything higher than default (which is 960) but I think we should be good for now just using the number. Screenshots look good to me

D39359-code up for a review

[marekhrabe]

code updated, backend caches might take ~24hrs to clear

[marekhrabe]

All homepage layouts should now look nicer. We have updated their size ratios and removed cookie notices which were present in some templates. Now looking like this:

[ianstewart]

@iamtakashi Do you still have access to those static screenshots? Would you mind making them accessible here?

… or in a separate issue?

[iamtakashi]

I sent it to @obenland a while ago.

[obenland]

I shared it in slack, the file size was too big to upload here: https://a8c.slack.com/files/U029G7YEU/FUK7NH60N/homepage-thumbnails.zip

[iamtakashi]

What was the reason for keeping mshots? Not that I'm saying the static ones are 100 times better, but at least they don't include the headers and have consistency with other thumbnails.

[marekhrabe]

Since updating the current dynamic screenshots was a matter of only changing query arguments of the image URLs, it felt like the quickest solution we can get done and I went that way.

Are they not acceptable? I don't know how to update templates to use static images but somebody in Ajax definitely knows so we can make that happen if it is a requirement. Feel free to reopen and update the issue if that needs to be done

[iamtakashi]

I'm reopening this and add a note of what we discussed about static thumbnails vs mshots for homepage templates.

Although it's not the most urgent item to do, but we do prefer static thumbnails over mshots because of the consistency with other non-homepage layouts.

[marekhrabe]

Started in D39999-code.

I have used preview images that were uploaded to headstartdata site (somebody did that before me). They looked equal but I haven't checked all of them in detail. If some of them need to be replaced, we can do that.

[obenland]

Fixed in r205217-wpcom