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

Page Layouts: Preview in the selector not using current color scheme. #42679

Open
alaczek opened this issue May 27, 2020 · 23 comments
Open

Page Layouts: Preview in the selector not using current color scheme. #42679

alaczek opened this issue May 27, 2020 · 23 comments
Labels
Page Layouts [Source] User testing Issues uncovered in user testing Triaged To be used when issues have been triaged. [Type] Enhancement

Comments

@alaczek
Copy link
Contributor

alaczek commented May 27, 2020

The Theme team is working on bringing additional color palettes to Varia child themes (see pNEWy-cQp-p2 for details). While testing those, I noticed that when I switched the color palette in the Customizer, those colors were not reflected in the previews in page layout selector.

Steps to reproduce

  1. Read pNEWy-cQp-p2 and apply relevant patches to your sandbox.
  2. Activate one of Varia child themes (be sure to apply the patch for that theme too).
  3. Navigate to Customzier > Colors & Backgrounds and switch to the dark color palette (although this happens with all of them).

color_palettes

  1. Save and exit Customizer.
  2. Add a new page.
  3. Notice how in page layout selector all the previews are using the default color scheme.

What I expected

For the layout previews to use the current color scheme. In contrast, the previews for block patterns are using the selected color palette.

Screenshot / Video

See video: https://cloudup.com/c8fRLK5cP9d

cc @johnHackworth @ianstewart @mendezcode

@deBhal deBhal added the [Pri] Low Address when resources are available. label Oct 26, 2020
@simison
Copy link
Member

simison commented Oct 26, 2020

if you need to apply patches to see the problem

Since the issue is from May and color schemes have shipped since then, this probably isn't the case anymore.

This is part of a bigger problem with that UI: just because how that page picker is implemented, previews don't reflect site’s theme either. Previews are static images.

@ianstewart
Copy link
Contributor

ianstewart commented Oct 26, 2020

I believe @alaczek is referring not the static thumbnail previews but the large editor preview when you've selected a page layout. Like the following example …

image

On the left is my example site with the Balasana theme active and a dark colour scheme published in the Customizer. On the right is the Add New Page Screen and the preview of one of those Page Layouts.

Fortunately, the Colour Scheme appears to now be loading here. Unfortunately, it appears to be broken-looking with much of the text not displaying correctly.

This appears to be either a theme or template issue.

@ianstewart
Copy link
Contributor

Here's the Editor Styles of the post relative to the front-end of the published page.

image

It may be an editor style issue in the theme. I'll report it in the themes repo and we can take a look there as well.

@simison
Copy link
Member

simison commented Jan 21, 2021

We're likely re-implementing this view soon so since it's pretty big task, it might make sense to wait a bit.

@ramonjd et co curious for your thoughts!

@simison simison removed the [Pri] Low Address when resources are available. label Jan 21, 2021
@ianstewart
Copy link
Contributor

I think the designs I've seen for this don't have the "big preview" so that might just take care of the issue?

@worldomonation worldomonation added the [Source] User testing Issues uncovered in user testing label Jan 21, 2021
@simison
Copy link
Member

simison commented Jan 21, 2021

@ianstewart well all the thumbnails will be new previews. :-)

So in future, the theme that's applied to site, as well theme settings such as colors, will be reflected in thumbnail images.

@ramonjd
Copy link
Member

ramonjd commented Jan 21, 2021

Unfortunately, it appears to be broken-looking with much of the text not displaying correctly.

What's the underlying theme for the above examples?

We're likely re-implementing this view soon so since it's pretty big task, it might make sense to wait a bit.

As far as I understand it, the latest designs for the page picker will show image-based previews (e.g., mshots or otherwise). We're having some success generating the onboarding preview images.

In that case, we'd be taking shots of the original sites and not relying on the demo endpoint, so CSS/JS cross-contamination won't be an issue, at least in the editor.

The demo endpoint will remain on onboarding for now I guess, but there haven't been any reported anomalies with that so far.

@ianstewart
Copy link
Contributor

@ramonjd looks like the underlying theme was Balasana in my comments.

Automattic/themes#2690

@ramonjd
Copy link
Member

ramonjd commented Jan 21, 2021

looks like the underlying theme was Balasana in my comments.

Ah, er I didn't have my reading glasses on 😇 Thanks!

I just wanted to double check the demo endpoint for that theme: https://public-api.wordpress.com/rest/v1/template/demo/balasana/balasana/?language=ko

@davipontesblog
Copy link
Contributor

@p-jackson is this still an issue with the new layout picker launched today?

@p-jackson
Copy link
Member

p-jackson commented Mar 17, 2021

Yes and no. The large preview area no longer exists, so that's no longer a problem. The thumbnails do not however reflect the site's current colour scheme.

Screenshot 2021-03-17 at 7 59 08 PM

The above screenshot is from a site with the Balasana theme and a dark colour scheme enabled in the customiser. However the thumbnails don't have a dark colour scheme (although they are showing what Balasana would look like at least).

After selecting the layout the page is rendered in the editor using the correct colour scheme.

Screenshot 2021-03-17 at 8 01 00 PM

So yes I think some of the issues in this thread are fixed now. The preview no longer looks broken like in some of the screenshots @ianstewart shared. But on the other hand the original issue in the title still exists. The preview does not honour the colour scheme.

Not sure what sort of priority this should be @simison @autumnfjeld. Feels like a new feature for the pattern toolkit would be needed. It is one of the things that would be needed to make the preview more like the real thing, as was discussed here: p7DVsv-aRA-p2#comment-35255

@davipontesblog
Copy link
Contributor

I'd say let's leave it as a feature request then.

@davipontesblog davipontesblog added the [Type] Feature Request Feature requests label Mar 17, 2021
@autumnfjeld
Copy link
Contributor

The preview does not honour the colour scheme.

Not sure what sort of priority this should be @simison @autumnfjeld. Feels like a new feature for the pattern toolkit would be needed. It is one of the things that would be needed to make the preview more like the real thing, as was discussed here: p7DVsv-aRA-p2#comment-35255

@p-jackson @davipontesblog
I have a memory of @andrewserong addressing this, such that the preview to matches the theme. Is this update the same thing that is being discussed here?

@p-jackson
Copy link
Member

p-jackson commented Mar 24, 2021

such that the preview to matches the theme
@autumnfjeld yep the previews reflect the theme, but not the current colour scheme, like what's been selected in the customiser.

Screen Shot on 2021-03-24 at 16:02:52

Screen Shot on 2021-03-24 at 16:03:32

Some themes have colour palettes (like the example in the issue description) and some themes just let you pick a colour. The pattern toolkit api has a query parameter to specify the theme, but not one to specify any of these custom colour options.

@andrewserong
Copy link
Member

I imagine it should be possible to add an additional param to pass in colour information to then apply to the preview in the template/demo endpoint. I guess the thing to be careful with for then passing into mShots is that the more specific the preview, then the more screenshots that get generated and we lose the benefit of caching between different sites, which could result in a longer load time to display screenshots for each user/site?

@p-jackson
Copy link
Member

Was just thinking that. And then with the advent of global styles, does that mean we have to pass the entire jsonified global styles config as a query param? May as well say goodbye to caching at that point.

@andrewserong
Copy link
Member

May as well say goodbye to caching at that point.

Exactly. At some point, maybe it'd be worth trying out live previews? Though that then comes with an entirely different set of performance penalties. Ah, trade-offs! 😅

@autumnfjeld
Copy link
Contributor

autumnfjeld commented Mar 24, 2021

Thanks for educating me @p-jackson 😊. I hadn't thought about / realized how custom colors (or other site customization) would also impact the thumbnail previews being non-accurate.

@simison
Copy link
Member

simison commented Mar 24, 2021

@andrewserong yeah, the performance hit of live-previews is annoyance here indeed. This will be an issue in core as well so it has to be worked out eventually. Here's one recent discussion about using grid format in Gutenberg: WordPress/gutenberg#20477 (comment)

I would imagine using the live rendering some day again, but as it is today, it's not ready for it. If you folks think the work/complexity required to get thumbnails match for colors isn't worth the effort, that's fair!

The design picker API sorts this out by switching to the site where the template is then rendered, so it includes the theme, any custom CSS from that site, color styles, etc. Would it make sense for patterns API to render patterns preview in the customer site's context?

@andrewserong
Copy link
Member

Would it make sense for patterns API to render patterns preview in the customer site's context?

It’s an interesting idea. We guarded the existing code a little bit by only rendering within a known set of sites rather than passing in an arbitrary site, where it might not be guaranteed to work properly (e.g. calling out to Atomic sites is probably a no-go).

Before venturing further into making screenshots unique to the site, we might want to look at the math on number of individual sites x requests to add a new page x number of page layouts on offer, as we could wind up generating a very large number of screenshots and mshots is currently set up to expect a fair bit of caching. So there could be server-performance considerations to take into account there, too.

Definitely interesting ideas worth exploring, though.

@simison
Copy link
Member

simison commented Mar 26, 2021

@andrewserong Right, fair points! Lots of potential complexity with theme switching, Atomic, and whatnot.

At the same time, here we are talking just about 2 options that matter for templates (footer/header color are not relevant in our case) so it's best not to overcomplicate it too early considering global styles. We'll figure it out when the time comes.

For caching we still have most sites having the same defaults initially, and once those are changed, the cached images stay valid for the same site across requests.

At any rate, this is all stuff more for the team to figure out, and I merely wanted to remind to keep things simple. :-)

@andrewserong
Copy link
Member

At any rate, this is all stuff more for the team to figure out, and I merely wanted to remind to keep things simple. :-)

Very good point, thanks for clarifying! Yes, in the shorter term it’d be good for us to look into whether we can pass in the selected colour scheme in an extra param in a simple way to cover the main existing case for a site having different colours :)

@inaikem
Copy link
Contributor

inaikem commented Apr 29, 2021

Sweeping feature requests for relevance. Looks good, keeping open.

@inaikem inaikem added the Triaged To be used when issues have been triaged. label Apr 29, 2021
@inaikem inaikem added Triaged To be used when issues have been triaged. and removed Triaged To be used when issues have been triaged. labels Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Page Layouts [Source] User testing Issues uncovered in user testing Triaged To be used when issues have been triaged. [Type] Enhancement
Projects
None yet
Development

No branches or pull requests