Backport: Block theme previews #4627
Conversation
There was a problem hiding this comment.
@MaggieCabrera There are a few small but critical permission related bits to fix here, other than that this looks good.
Which Trac ticket is this PR for? Would be great if you could add it to the PR description to link the two.
|
@felixarntz @audrasjb would you please have another look? |
|
During testing, I'm seeing an error when the active theme is a classic theme
The reverse case is unaffected: it's still possible to preview a classic theme while a block theme is active.
|
I suspect this is due to the npm packages not being updated yet in core. Can you confirm @MaggieCabrera ? |
This is in fact not happening when testing that flow using Gutenberg trunk and core 6.2.2. I can't say of the top of my head what's causing it, I'm going to double-check, but most likely we are just missing the npm packages like you said. |
| $preview_stylesheet = ! empty( $_GET['wp_theme_preview'] ) ? $_GET['wp_theme_preview'] : null; | ||
| $wp_theme = wp_get_theme( $preview_stylesheet ); | ||
| if ( ! is_wp_error( $wp_theme->errors() ) ) { | ||
| if ( current_filter() === 'template' ) { |
There was a problem hiding this comment.
What is happening here? Is this detecting if the theme has a parent? If so, I remember using $wp_theme->parent().
There was a problem hiding this comment.
Thanks for looking at this Johnny. This function (wp_get_theme_preview_path) is applied on two different filters - stylesheet and template. The return value of the function needs to be different depending on whether we are running on the stylesheet or template filter. The call to current_filter is determining which filter we are on so we know which value to return.
This is not the reason why it was failing - we found the cause was an incorrect GET param on the button. Having said that we are still finding that the feature isn't working correctly on this branch because the npm packages aren't updated yet. I think we'll need to wait for this to be done before we can confirm that this is working as expected. |
There was a problem hiding this comment.
@MaggieCabrera This looks good to me now, though based on one of your comments we can probably make one improvement here so that customize isn't unnecessarily checked for block themes 👇
|
Thanks for the pointer, I tried running |
Co-authored-by: Andrei Draganescu <[email protected]>
Co-authored-by: Andrei Draganescu <[email protected]>
This reverts commit ab55297.
Co-authored-by: Ben Dwyer <[email protected]>
Co-authored-by: Peter Wilson <[email protected]>
Co-authored-by: Peter Wilson <[email protected]>
Co-authored-by: Felix Arntz <[email protected]>
MaggieCabrera
force-pushed
the
theme-previews-backport
branch
from
ce9a41f
to
9971a7a
Compare
Trac ticket: https://core.trac.wordpress.org/ticket/58561
This PR backports the changes made to the block theme previews. I grouped them together instead of having one PR for each change because they would all depend on each other, which made more sense.
Backports:
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.