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

Site Editor: Fix routing for Classic themes using block-based template parts #48343

Merged
merged 3 commits into from
Feb 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions lib/compat/wordpress-6.2/menu.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php
/**
* Admin menu overrides for WP 6.2.
*
* @package gutenberg
*/

/**
* Updates "Template Parts" menu URL to include `path` query argument.
*
* Note: Remove when the minimum required WP version is 6.2.
* No need to backport in core. Changes are applied in wp-admin/menu.php.
*/
function gutenberg_update_template_parts_menu_url() {
if ( wp_is_block_theme() ) {
return;
}

if ( ! current_theme_supports( 'block-template-parts' ) ) {
return;
}

global $submenu;
if ( ! isset( $submenu['themes.php'] ) ) {
return;
}

foreach ( $submenu['themes.php'] as $index => $menu_item ) {
if ( str_contains( $menu_item[2], 'site-editor.php?postType=wp_template_part' ) && ! str_contains( $menu_item[2], 'path=' ) ) {
$submenu['themes.php'][ $index ][2] = 'site-editor.php?postType=wp_template_part&path=/wp_template_part/all';
break;
}
}
}
add_action( 'admin_menu', 'gutenberg_update_template_parts_menu_url' );
1 change: 1 addition & 0 deletions lib/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ function gutenberg_is_experiment_enabled( $name ) {
require __DIR__ . '/compat/wordpress-6.2/block-editor-settings.php';
require __DIR__ . '/compat/wordpress-6.2/theme.php';
require __DIR__ . '/compat/wordpress-6.2/widgets.php';
require __DIR__ . '/compat/wordpress-6.2/menu.php';

if ( ! class_exists( 'WP_HTML_Tag_Processor' ) ) {
require __DIR__ . '/compat/wordpress-6.2/html-api/class-wp-html-attribute-token.php';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { useEntityRecords } from '@wordpress/core-data';
import { useSelect } from '@wordpress/data';
import { decodeEntities } from '@wordpress/html-entities';
import { useViewportMatch } from '@wordpress/compose';

Expand All @@ -18,6 +19,7 @@ import SidebarNavigationScreen from '../sidebar-navigation-screen';
import { useLink } from '../routes/link';
import SidebarNavigationItem from '../sidebar-navigation-item';
import AddNewTemplate from '../add-new-template';
import { store as editSiteStore } from '../../store';

const config = {
wp_template: {
Expand Down Expand Up @@ -51,6 +53,11 @@ export default function SidebarNavigationScreenTemplates() {
params: { postType },
} = useNavigator();
const isMobileViewport = useViewportMatch( 'medium', '<' );
const isTemplatePartsMode = useSelect( ( select ) => {
const settings = select( editSiteStore ).getSettings();

return !! settings.supportsTemplatePartsMode;
}, [] );

const { records: templates, isResolving: isLoading } = useEntityRecords(
'postType',
Expand All @@ -64,11 +71,14 @@ export default function SidebarNavigationScreenTemplates() {
path: '/' + postType + '/all',
} );

const canCreate = ! isMobileViewport && ! isTemplatePartsMode;

return (
<SidebarNavigationScreen
isRoot={ isTemplatePartsMode }
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit weird to be honest, I think the ideal solution for this "mode" is to probably have its own "init function" rather than rely on the default site editor initialization function. In the long term, I'm certain this "hack" is going to bite us. By "Hack" I mean the fact that we kind of try to load a specific version of the site editor for the "template parts" screen of classic themes.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's still the same site editor; we don't want some routes to be available for Classic themes.

But I agree this "hack" isn't the best way to solve this problem. I will try to explore different init methods for WP 6.3.

title={ config[ postType ].labels.title }
actions={
! isMobileViewport && (
canCreate && (
<AddNewTemplate
templateType={ postType }
toggleProps={ {
Expand Down
51 changes: 51 additions & 0 deletions test/e2e/specs/site-editor/template-parts-mode.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* WordPress dependencies
*/
const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' );

test.describe( 'Template Parts for Classic themes', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Great idea to have an e2e test for this, we don't check this flow often manually.

test.beforeAll( async ( { requestUtils } ) => {
await requestUtils.activateTheme( 'emptyhybrid' );
} );

test.afterAll( async ( { requestUtils } ) => {
await requestUtils.activateTheme( 'twentytwentyone' );
} );

test( 'can access template parts list page', async ( { admin, page } ) => {
await admin.visitAdminPage(
'site-editor.php',
'postType=wp_template_part&path=/wp_template_part/all'
);
kevin940726 marked this conversation as resolved.
Show resolved Hide resolved

await expect(
page.getByRole( 'table' ).getByRole( 'link', { name: 'header' } )
).toBeVisible();
} );

test( 'can view a template part', async ( { admin, editor, page } ) => {
await admin.visitAdminPage(
'site-editor.php',
'postType=wp_template_part&path=/wp_template_part/all'
);

const templatePart = page
.getByRole( 'table' )
Copy link
Member

Choose a reason for hiding this comment

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

Off-topic: We should probably fix this by giving the table an accessible name in another PR 😅 .

.getByRole( 'link', { name: 'header' } );

await expect( templatePart ).toBeVisible();
await templatePart.click();

await expect(
page.getByRole( 'region', { name: 'Editor content' } )
).toBeVisible();

await editor.canvas.click( 'body' );

await expect(
editor.canvas.getByRole( 'document', {
name: 'Block: Site Title',
} )
).toBeVisible();
} );
} );
25 changes: 25 additions & 0 deletions test/gutenberg-test-themes/emptyhybrid/functions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

if ( ! function_exists( 'emptyhybrid_support' ) ) :
function emptyhybrid_support() {

// Adding support for core block visual styles.
add_theme_support( 'wp-block-styles' );

// Enqueue editor styles.
add_editor_style( 'style.css' );

// Enable block-based template parts support.
add_theme_support( 'block-template-parts' );
}
add_action( 'after_setup_theme', 'emptyhybrid_support' );
endif;

/**
* Enqueue scripts and styles.
*/
function emptyhybrid_scripts() {
// Enqueue theme stylesheet.
wp_enqueue_style( 'emptyhybrid-style', get_template_directory_uri() . '/style.css', array(), wp_get_theme()->get( 'Version' ) );
}
add_action( 'wp_enqueue_scripts', 'emptyhybrid_scripts' );
15 changes: 15 additions & 0 deletions test/gutenberg-test-themes/emptyhybrid/index.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<!doctype html>
<html <?php language_attributes(); ?>>
<head>
<meta charset="<?php bloginfo( 'charset' ); ?>" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
<?php wp_head(); ?>
</head>

<body <?php body_class(); ?>>
<?php wp_body_open(); ?>
<div id="page" class="site">
<?php block_template_part( 'header' ); ?>
</div>
</body>
</html>
3 changes: 3 additions & 0 deletions test/gutenberg-test-themes/emptyhybrid/parts/header.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<!-- wp:site-title /-->

<!-- wp:site-tagline /-->
15 changes: 15 additions & 0 deletions test/gutenberg-test-themes/emptyhybrid/style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
Theme Name: Emptyhybrid
Theme URI: https://github.com/wordpress/gutenberg/
Author: the WordPress team
Description: The testing hybrid theme.
Requires at least: 6.0
Tested up to: 6.2
Requires PHP: 5.6
Version: 1.0
License: GNU General Public License v2 or later
License URI: http://www.gnu.org/licenses/gpl-2.0.html
Text Domain: Emptyhybrid
Emptytheme WordPress Theme, (C) 2021 WordPress.org
Emptytheme is distributed under the terms of the GNU GPL.
*/