Site Editor: Allow all classic themes to access Patterns page #54066
Conversation
t-hamano
added
[Feature] Patterns
|
Size Change: -48 B (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
t-hamano
force-pushed
the
site-editor/patterns-page-classic-theme
branch
from
ea2656f to
d92175f
Compare
t-hamano
changed the title
t-hamano
force-pushed
the
site-editor/patterns-page-classic-theme
branch
from
d92175f to
211d276
Compare
t-hamano
changed the title
There was a problem hiding this comment.
Thanks for your efforts in improving access to the Patterns site editor page for non-block-themes @t-hamano.
It appears we have an issue we with the pattern commands available in the command palette. While testing this I noticed two patterns related commands.
It looks like #53496 introduced the more recent command when it probably needed to just update the existing one that took into account whether or not the user could access the site editor's pattern page.
I imagine we need to consolidate those commands before applying the new logic here.
cc/ @glendaviesnz & @richtabor
I'm also not sure what the current thinking is around allowing classic themes to access the patterns page just yet. If we proceed with this PR, I get the feeling we should also be cleaning up the access to the template parts list page etc as well.
Alternatively, perhaps a tracking issue to make sure we don't leave any loose ends here would help.
|
Thanks for the review, @aaronrobertshaw!
I agree. I have submitted #54133 which fixes this issue.
The issue of compatibility between the classic theme and the command palette or site editor has often been addressed. Here is an example, which I believe is compatible enough for now, except for the duplication of pattern commands you reported.
|
I think it would be good to get some design input into this to see if we can define exactly what level of compatibility we want to provide to classic/hybrid themes. I have added the needs design input flag - but feel free to remove if you don't think this is needed. |
t-hamano
force-pushed
the
site-editor/patterns-page-classic-theme
branch
from
211d276 to
93824f6
Compare
|
I have Rebase this PR with the latest trunk, which will also fix the duplicate Pattarns command. By the way, here is what I have in mind as a patch to submit to the core: Detailsdiff --git a/src/wp-admin/menu.php b/src/wp-admin/menu.php
index 17317f9daa..460cd8d787 100644
--- a/src/wp-admin/menu.php
+++ b/src/wp-admin/menu.php
@@ -204,6 +204,8 @@ if ( ! is_multisite() && current_user_can( 'update_themes' ) ) {
if ( wp_is_block_theme() ) {
$submenu['themes.php'][6] = array( _x( 'Editor', 'site editor menu item' ), 'edit_theme_options', 'site-editor.php' );
+} else {
+ $submenu['themes.php'][6] = array( __( 'Patterns', 'site editor menu item' ), 'edit_theme_options', 'site-editor.php?path=/patterns' );
}
if ( ! wp_is_block_theme() && current_theme_supports( 'block-template-parts' ) ) {
@@ -219,9 +221,7 @@ $customize_url = add_query_arg( 'return', urlencode( remove_query_arg( wp_remova
// Hide Customize link on block themes unless a plugin or theme
// is using 'customize_register' to add a setting.
if ( ! wp_is_block_theme() || has_action( 'customize_register' ) ) {
- $position = ( wp_is_block_theme() || current_theme_supports( 'block-template-parts' ) ) ? 7 : 6;
-
- $submenu['themes.php'][ $position ] = array( __( 'Customize' ), 'customize', esc_url( $customize_url ), '', 'hide-if-no-customize' );
+ $submenu['themes.php'][7] = array( __( 'Customize' ), 'customize', esc_url( $customize_url ), '', 'hide-if-no-customize' );
}
}
-if ( ! ( current_theme_supports( 'block-template-parts' ) || wp_is_block_theme() ) ) {
- wp_die( __( 'The theme you are currently using is not compatible with the Site Editor.' ) );
-}
-
-$is_template_part = isset( $_GET['postType'] ) && 'wp_template_part' === sanitize_key( $_GET['postType'] );
+$is_template_part = isset( $_GET['postType'] ) && 'wp_template_part' === sanitize_key( $_GET['postType'] );
$is_template_part_path = isset( $_GET['path'] ) && 'wp_template_partall' === sanitize_key( $_GET['path'] );
+$is_patterns = isset( $_GET['postType'] ) && 'wp_block' === sanitize_key( $_GET['postType'] );
+$is_patterns_path = isset( $_GET['path'] ) && 'patterns' === sanitize_key( $_GET['path'] );
+
$is_template_part_editor = $is_template_part || $is_template_part_path;
+$is_patterns_editor = $is_patterns || $is_patterns_path;
-if ( ! wp_is_block_theme() && ! $is_template_part_editor ) {
- wp_die( __( 'The theme you are currently using is not compatible with the Site Editor.' ) );
+if ( ! wp_is_block_theme() ) {
+ if ( ! current_theme_supports( 'block-template-parts' ) && $is_template_part_editor ) {
+ wp_die( __( 'The theme you are currently using is not compatible with the Site Editor.' ) );
+ }
+ if ( ! $is_template_part_editor && ! $is_patterns_editor ) {
+ wp_die( __( 'The theme you are currently using is not compatible with the Site Editor.' ) );
+ }
} |
t-hamano
force-pushed
the
site-editor/patterns-page-classic-theme
branch
from
93824f6 to
494985e
Compare
There was a problem hiding this comment.
Apologies for the delays on getting back to this one @t-hamano, I've been wrestling with exactly what access classic patterns should have to the site editor's pattern page.
However, I believe the Patterns page in the Site Editor is designed to be available regardless of the type of theme.
I'm not one of the design team but I'm not sure I agree with this 100%. Which, I believe, is why we were waiting on some design feedback.
After some further discussions, I think there's some middle ground for classic themes that we could achieve for the 6.4 release.
- Only classic themes that also adopt
block-template-partssupport can access the pattern page. - Without template parts support, a classic theme should be directed to the
wp_blockpost-type index page and editor (as they do on trunk now in commands etc).
This would help prevent adding additional complexity to the patterns pages and screens. Make the page more consistent for all users that can access it etc.
It seems the primary issue for classic themes is the ability to easily access the post-type index page for patterns. The links via the block toolbar or command palette aren't easily discoverable.
The main proposal floated to address that was adding a link under Appearance > Patterns in wp-admin. Doing that only for classic themes without template part support would probably get my vote.
cc/ @SaxonF @jameskoster @glendaviesnz any thoughts given the limited time until the next beta?
|
Thank you for the suggestion @aaronrobertshaw!
Does this mean that when I press the back button on the "All Template Parts" page, I will be redirected to the Patterns page instead of the dashboard? Or do you mean that in addition to those, the classic theme with opt-in block-template-parts will show 'Patterns' menu item instead of 'Template Parts' in the Appearance menu? |
|
Sorry for the confusion, I could have been clearer in my comment.
That's a good question. It might depend on whether the Appearance > Template Parts link continues to go to the All Template Parts index page or to the main Patterns page with a default template part category selected. If it was the latter, back from the Patterns page would go back to wp-admin for both patterns and template parts (for classic themes with
Eventually, Template Parts will be rolled into Patterns, so this will possibly be what it is long term. I'm not sure that will happen now though. Which, is why we might need some further thought on these links.
To bring the discussion back to what I think is the primary issue, classic themes need an easy way to access patterns and edit them. That could be solved by linking to |
|
I also created #54342 as an alternative approach so that we can play with both and decide which is better for the users. Good thing is that seems like there's no blocker in terms of code and implementation, so we just need come to a consensus 👍 . |
|
Thanks for trying a different approach, @kevin940726! What I would like to see is whether it makes sense to allow all themes to access the Pattern page, ultimately including classic themes that have not opted into What do you think? |
Patterns are still useful to sites with classic themes, no? I think it makes sense to allow access even if they don't support template parts. |
|
If we want to allow all themes to access |
|
Thanks for weighing in @jameskoster 👍
The consensus is definitely classic themes should have easier access to patterns. The question is whether that access should be the site editor patterns page or the post type page in wp-admin as you noted in #52150 (comment).
The "manage patterns" links in the editor only direct classic themes to the second link above.
Adding an Appearance > Patterns link to My understanding is that for all classic themes to be directed to the site editor's pattern management page we need a core patch so that access is permitted to that page when reloading etc. Would that prevent this change landing in Gutenberg, which could be used on WP <6.4? If that means we can't then update the manage pattern links, would it not be better to keep classic themes all going to the same patterns page? If all that isn't a concern, then I don't have a strong opinion on direction, so if you're happy with all classic themes going to the patterns management page in the site editor, we should proceed with this PR. It will just need to update all the links missed that point to the |
In my opinion, in the future, all themes should have access to the Patterns page in the Site Editor (
I think you are right. I have come up with the following plan to allow all themes to have access to the Site Editor's Patterns page in the future while maintaining full backward compatibility.
Assuming this PR is merged and a patch like the one above is applied to WP6.4, the following scenario would be possible:
Sorry if there are any points that are confusing. I would appreciate your honest feedback 🙇 |
Yes, I agree 100% that, in the future, all themes will have access to the patterns management page in the site editor. I don't think that future will be until the minimum supported version is WP 6.4.
I believe the inconsistency here is a problem. Users should be taken to the same location via the Appearance > Patterns link or the "manage patterns" link. As noted previously, my vote is that the Appearance > Patterns link for classic themes in 6.4 goes to I do appreciate the desire to access the new site editor page, it is the goal long term, no arguments here. To maintain a consistent experience in a backwards-compatible manner, I think we need to take this interim step. |
I see, so it seems better to proceed with #54342, which has consistent Pattern links and is a more step-by-step approach? However, I have concerns regarding the following specification:
I think this would work well for WordPress with the core patch applied as suggested in #54342. For example, suppose you have a WordPress 6.3 site with the Classic theme with
I would like to consider whether this backward compatibility issue is acceptable. |
|
@t-hamano thanks for your patience in working through this one 👍 The backwards compatibility issue is a concern, 100%. When I initially floated allowing classic themes with All commands from the command palette, the "manage patterns" links, and the Appearance > Patterns link should go to |
That's right, I'm sorry I couldn't convey my intentions well 😅 Based on this, my new plan is as follows.
It will probably be around WP6.5 or WP6.6 that classic themes will be able to consistently use the site editor pattern page, but I think backwards compatibility will be maintained. Alternatively, I think the following approach may be possible:
|
|
Correct me if I'm wrong, so here's basically the plan now: In Core WP 6.4:
In gutenberg after the minimum required version is WP 6.4:
(Oops, @t-hamano beat me to it 😂) |
The new plan makes sense to me 👍 |
|
Thanks for all your advice🙇 Then I would like to submit the core patch as soon as possible. Or should we update this patch submitted by @kevin940726? |
I haven't submitted the patch, it's just a branch on my fork 😅 . But we can certainly use that if you prefer. Given that it's only a few lines, and you've done most of the work, I think you can submit the patch so that you'll get the credit you deserve. 💯 |
Whatever is easiest for you! I like @kevin940726's suggestion that you take the deserved credit. Whether it ended up as a small change or not, it took patience, perseverance, and care to arrive at the best result. That's worth celebrating in my book 🎉 Kudos also to Kai for helping us work through possibilities 👍 |
|
I discovered the unintended behavior while writing a core patch to allow access to the Pattern page from the classic theme. Therefore, I have submitted #54422 which fixes this one. |
|
Core patch submitted: WordPress/wordpress-develop#5201 |
There was a problem hiding this comment.
I believe the consensus ended up that all the links to patterns page should remain consistent.
From that, my understanding is that the command palette changes here need to be reverted so that classic themes will still be directed to the edit.php?post_type=wp_block page matching the "manage patterns" links in the editor along with the new Appearance > Patterns link introduced in WordPress/wordpress-develop#5201
|
This PR was originally intended for the classic theme to have consistent access to the Patterns page in the Site editor ( As a result, I think this PR should be closed, as I suggested in this comment. Instead, I'm filing #54422 to address the issue that can occur if users access the pattern page directly. Is my understanding correct? |
No issues here with closing this PR 👍 I was confused given this PR was linked from your core patch in this comment which said you wanted to ship it. |
Admittedly, the Gutenberg link I put in the core patch wasn't the right one 🙈 I would like to close this PR and update the core patch information. |
All good. I'm easily confused though 😂 |
|
I have been trying out this new 6.4 feature to learn more about exposing patterns in classic themes. I am basically following to the instructions outlined in this PR and using these testing environment:
I could replicate the instructions for the TT3 theme described here. For the TT1, I was only able to expose template parts, but not the patterns described in the instructions. I get the following message:
What am I doing wrong here? |
|
Hi @tinjure20, You are not performing an incorrect test procedure. The #5201 has not yet been committed into the WP core, so even if you update to WordPress Beta, you still cannot access it from the Classic Editor except for the Template Parts Editor in the Site Editor. |
|
Hello @t-hamano, Thank you very much for your prompt response. Just a quick questions: when this feature would be available? by RC1?. Thank you. |
|
Hi @tinjure20, I don't yet know in which phase it will be included, but we are doing our best to ship this in 6.4. |
|
Unfortunately, this has been moved to WP 6.5. More info can be found on the related issue and core patch |
Part of #52150
Core ticket: https://core.trac.wordpress.org/ticket/58827
What?
This PR allows client-side access to the Patterns page for all non-block themes.
Why?
Currently, only classic themes that support
block-template-partscan use the Template Parts Editor in the Site Editor.Starting with WordPress 6.3, all themes can create unsynced patterns. For non-block themes, the Patterns page in the Site Editor is not accessible, so users always need to use the WP-Admin style pattern editing UI.
However, I believe the Patterns page in the Site Editor is designed to be available regardless of the type of theme.
How?
This PR permits the following in the classic theme:
block-template-partssupport.Testing Instructions
Preparation
If this change makes sense, I would like to propose the following changes in the core after this PR is merged:
block-template-partsor not (Do not show the Template Parts menu).For now, comment out the following to temporarily allow access:
https://github.com/WordPress/wordpress-develop/blob/54a177910a80083930ee1284f648c1db5bf2e5c0/src/wp-admin/site-editor.php#L22-L32
To test the hybrid theme, you may want to use the wp-env test environment that has Emptyhybrid theme (
localhost:8889/wp-admin).Block Theme
All operations are still possible and there should be no commands, pages, or UI that will be unavailable in this PR.
Hybrid Theme (Emptyhybrid)
Classic Theme (Twenty Twenty One)
About Backward Compatibility
My concern is that unless the Patterns menu item is added to the core side and the classic theme has access to Patterns page, it will not work ideally.
Specifically, classic themes that support
block-template-partscan access the Template Parts page from the template parts menu item in the admin panel. However, the pattern page cannot be accessed directly, and the back button must be pressed once from the template parts page.So, if this feature is implemented in WP6.4, then the above inconsistency will occur until the minimum WordPress version supported by the Gutenberg plugin is raised to 6.4.
If you have a better approach to resolving this issue, please let me know.