Move default template types and template part areas to REST API #66459
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gigitux
force-pushed
the
fix/rest-template-parts
branch
from
5a5a7e9
to
86b2e8f
Compare
|
Size Change: +358 B (+0.02%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
gigitux
force-pushed
the
fix/rest-template-parts
branch
from
86b2e8f
to
95d6d1c
Compare
gigitux
force-pushed
the
fix/rest-template-parts
branch
from
7b905f6
to
5b83ef7
Compare
This was referenced Oct 25, 2024
gigitux
commented
Nov 1, 2024
Comment on lines
1718
to
1729
| export const __experimentalGetDefaultTemplatePartAreas = createRegistrySelector( | ||
| ( select ) => | ||
| createSelector( () => { | ||
| const areas = | ||
| select( coreStore ).getEntityRecord( 'root', '__unstableBase' ) | ||
| ?.defaultTemplatePartAreas || []; | ||
|
|
||
| return areas.map( ( item ) => { | ||
| return { ...item, icon: getTemplatePartIcon( item.icon ) }; | ||
| } ); | ||
| } ) | ||
| ); |
There was a problem hiding this comment.
I think that we should deprecate this selector and create a dedicated hook to be reused across the codebase: what do you think?
youknowriad
reviewed
Nov 4, 2024
packages/core-data/src/entities.js
Outdated
| @@ -31,6 +31,8 @@ export const rootEntitiesConfig = [ | |||
| 'site_icon_url', | |||
| 'site_logo', | |||
| 'timezone_string', | |||
| 'defaultTemplatePartAreas', | |||
| 'defaultTemplateTypes', | |||
There was a problem hiding this comment.
Looks like these are the only fields that are camelCase? should they follow the other pattern?
youknowriad
requested review from
Mamaduka,
jsnajdr,
jorgefilipecosta and
TimothyBJacobs
|
Do you think we can update this PR to target trunk directly (avoid all the changes related to the action)? That way we can land it first. |
Thanks! Sure! Thanks for tagging other folks! After that, I will make the changes, I will mark this PR ready to be reviewed! |
gigitux
force-pushed
the
fix/rest-template-parts
branch
from
344f380
to
6aaf160
Compare
|
Flaky tests detected in 230ba72. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12026241537
|
Mamaduka
reviewed
Nov 15, 2024
Comment on lines
41
to
47
| const templateAreas = | ||
| select( coreStore ).getEntityRecord( 'root', '__unstableBase' ) | ||
| ?.default_template_part_areas || []; | ||
|
|
||
| const templateTypes = | ||
| select( coreStore ).getEntityRecord( 'root', '__unstableBase' ) | ||
| ?.default_template_types || []; |
There was a problem hiding this comment.
Mamaduka
reviewed
Nov 15, 2024
Comment on lines
+34
to
+35
| 'default_template_part_areas', | ||
| 'default_template_types', |
There was a problem hiding this comment.
Changes here also require an update to the preload path in lib/compat/wordpress-6.8/preload.php. See the comment above the _fields property.
There was a problem hiding this comment.
TIL! Thanks! fdc291d
There was a problem hiding this comment.
Why didn't you keep the same order?
gigitux
force-pushed
the
fix/rest-template-parts
branch
from
dc78bf9
to
1f47713
Compare
gigitux
force-pushed
the
fix/rest-template-parts
branch
from
745b904
to
078d2f6
Compare
youknowriad
reviewed
Nov 25, 2024
| @@ -0,0 +1,38 @@ | |||
| <?php | |||
There was a problem hiding this comment.
The file name is a bit weird but it's not important. Could be rest-api.php or something.
There was a problem hiding this comment.
+1. I think a rest-api.php file is already in WP 6.8 compat dir. We can move this code there.
youknowriad
approved these changes
Nov 25, 2024
|
Sorry for the delay, but last week I was busy with other duties! Thanks for your great review! 🙇 |
|
@Mamaduka I will wait for your approval before merging this PR! |
|
Should I handle backporting this change to WordPress? |
Yes. It's recommended to start backport PR as soon as possible. I think there's an even CI check for it. See https://github.com/WordPress/gutenberg/blob/trunk/backport-changelog/readme.md for additional details. |
oandregal
reviewed
Nov 26, 2024
| deprecated( | ||
| "select('core/editor').__experimentalGetDefaultTemplateTypes", | ||
| { | ||
| since: '6.7', |
There was a problem hiding this comment.
shouldn't this be 6.8? (same below)
There was a problem hiding this comment.
Nice catch! I'm going to open a follow-up PR! Thanks 🙇
I created a backport PR: WordPress/wordpress-develop#7895. I'm not sure if I should follow these steps given that this PR has already merged. |
|
I think those files are used to sync backport tracking issues for a release automatically. See #62973. You could create a |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR moves the
defaultTemplateTypes,defaultTemplatePartAreasto thegetEntityRecord( 'root', '__unstableBase' ) endpoint.Given that the impacted selectors aren't specific to the
editorstore, I decided to deprecate them all. I couldn't find a suitable location to ensure accessibility across different packages, leading to some duplicated code. Should we create a@wordpress/templatepackage (similar to@wordpress/patterns) to house this logic? Create a dedicated package could help us with #65390 too.Why
This PR is necessary to remove the
core/editordependency across the application. In the specific, this refactor is needed to allow the packagefieldsto have access to thedefaultTemplateTypes,defaultTemplatePartAreaswithout that editor settings are initialized. More details here: #65390 (comment)Testing Instructions
Visit the Site Editor.
On the sidebar click on
Pattern.Click on
All template parts.Ensure that all the patterns are visible (you should see
HeaderandFooterlabels too)On the top right, click on
Add New Pattern.Create a new template part
Ensure that the template part is created correctly.
Visit the Site Editor.
Visit the
Blog Home.Ensure that the right panel under the template section renders the right data:
Add a block into the footer.
Click save.
Ensure that
are you ready to saveshows the right data:Testing Instructions for Keyboard
Screenshots or screencast