-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Changes for FSE backport in core #36201
Merged
Merged
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
157af1e
Changes for FSE backport in core
ntsekouras 55d6173
split files
ntsekouras f7bda38
rename files
ntsekouras 11e116b
more changes
ntsekouras 882f021
rename gutenberg_get_block_template
ntsekouras a6e8def
rename _gutenberg_build_template_result_from_post
ntsekouras 939489d
rename gutenberg_get_default_template_types
ntsekouras 88612a6
rename _gutenberg_get_template_file
ntsekouras 9029417
rename _gutenberg_add_template_info + _gutenberg_add_template_part_ar…
ntsekouras 679bd49
rename gutenberg_get_block_file_template
ntsekouras a44cb1b
rename _gutenberg_build_template_result_from_file
ntsekouras ce18660
rename gutenberg_filter_template_part_area
ntsekouras f3f4519
reorder functions to match core file
ntsekouras 6788aea
rename _gutenberg_get_template_files
ntsekouras ce829f5
fix get_block_templates
ntsekouras ec558ec
load utils earlier
ntsekouras bc011db
rename test functions
ntsekouras 91d63d4
add back `gutenberg` prefix in get_block_template and get_block_templ…
ntsekouras 529df1f
move templates controller to compat/5.9
ntsekouras 76645bf
rename to `gutenberg_set_unique_slug_on_create_template_part` and bai…
ntsekouras File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
Large diffs are not rendered by default.
Oops, something went wrong.
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
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
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This endpoint can also be moved to the 5.9 folder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw that the controller was introduced in
5.8
. Should I put the whole controller under5.9
folder? 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5.9 because it changed between the two versions, so it should only be removed after 5.9 is the minimum version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how to handle the endpoint here, as in load I guess we should have the check for if the class exists, but this will be true for
5.8
as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we could check something that only exists on 5.9 and have a comment there. Also the call registering the endpoint should use the same check and same comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also we should keep the class name different in order to be able to register it in 5.8 even if it already exists (Gutenberg prefix)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we keep the prefix, do we need a
5.9
check?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily because the versions are exactly the same, that said, I think keeping the check is important for one case: imagine if in the future, we need to make changes to the endpoint, this means the endpoint need to move to
wordpress-6.0
folder but if we don't have the check in place, we won't notice it and we could update it but keep it inwordpress-5.9
which means we'll forget about it when it comes time to make backports for 6.0There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm.. okay. I think this is coupled with the registration of the post types (
wp_template and wp_template_part
) handling as well (rest_controller_class
prop) and we can check it in a follow up? I guess a check forGutenberg_REST_Global_Styles_Controller
would be a good candidate when the patch makes it to core?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it complicates things too much, it might not be worth it.