Skip to content

/ gutenberg

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

Jump to bottom

Site Editor: Implement a settings object filter #33737

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from
Open

Site Editor: Implement a settings object filter #33737

wants to merge 4 commits into from
+10 −12

Conversation

@jeyip
Copy link
Contributor

@jeyip jeyip commented Jul 28, 2021 •
edited

Potentially solves #33736

Description

The post editor has the block_editor_settings_all and block_editor_settings filter that allows us to modify the settings object injected into the block editor. A similar filter, however, doesn't exist for the site editor.

How has this been tested?

There should be no visible changes to the UI.

  1. Set up local Gutenberg dev environment
  2. Activate a block based theme
  3. Navigate to the site editor
  4. Smoke test the site editor to ensure that updates still persist correctly, block insertion still functions as expected, etc.

Screenshots

Types of changes

New feature

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
@jeyip
Add block editor settings filter for site editor init
4870110 
The post editor has the block_editor_settings_all and block_editor_settings filter that allows us to modify the settings object injected into the block editor. A similar filter, however, doesn't exist for the site editor. We apply the same filter here for the site editor.
@jeyip jeyip self-assigned this Jul 28, 2021
@jeyip jeyip changed the title Add block editor settings filter for site editor init Site Editor: Implement a settings object filter Jul 28, 2021
@jeyip
jeyip reviewed Jul 28, 2021
View changes
lib/full-site-editing/edit-site-page.php Outdated Show resolved Hide resolved
@jeyip jeyip requested review from mattwiebe and nosolosw Jul 28, 2021
@jeyip jeyip mentioned this pull request Jul 29, 2021
Open
@mattwiebe
mattwiebe requested changes Jul 29, 2021
View changes
Copy link
Contributor

@mattwiebe mattwiebe left a comment

Left a comment about needing another param and Core's filter documentation convention. Otherwise, this is great, and good call on not needing to call gutenberg_experimental_global_styles_settings directly, since it runs on the filter.
lib/full-site-editing/edit-site-page.php Outdated Show resolved Hide resolved
jeyip added 2 commits Jul 29, 2021
@jeyip
Add site editor context param to apply filters
6aea831 
We need to add a second param to our call to apply filters because it is included in Core WordPress.
@jeyip
Add comment for call to apply filters
1441568
@jeyip jeyip mentioned this pull request Jul 30, 2021
Open
@gziolo
gziolo reviewed Jul 30, 2021
View changes
lib/full-site-editing/edit-site-page.php
@@ -101,7 +101,10 @@ function gutenberg_edit_site_init( $hook ) {
'__experimentalBlockPatternCategories' => WP_Block_Pattern_Categories_Registry::get_instance()->get_all_registered(),
)
);
$settings = gutenberg_experimental_global_styles_settings( $settings );

$site_editor_context = new WP_Block_Editor_Context();

This comment has been minimized.

Sign in to view
@gziolo

gziolo Jul 30, 2021
Member

I'm not quite sure why the edit site page doesn't integrate this filter like other screens. If we decide to change it, we should refactor the code to use gutenberg_get_block_editor_settings to handle it all behind the scenes rather that duplicating a similar logic.

This comment has been minimized.

Sign in to view
@jeyip

jeyip Jul 30, 2021
Author Contributor

Makes sense to me. I'll make the changes. 👍

This comment has been minimized.

Sign in to view
@jeyip

jeyip Jul 30, 2021
Author Contributor

Updated in 72b175c
@jeyip
Refactor with gutenberg_get_block_editor_settings
72b175c
@jeyip
jeyip reviewed Jul 30, 2021
View changes
lib/full-site-editing/edit-site-page.php
@@ -89,19 +89,17 @@ function gutenberg_edit_site_init( $hook ) {
*/
$current_screen->is_block_editor( true );

$settings = array_merge(
gutenberg_get_default_block_editor_settings(),

This comment has been minimized.

Sign in to view
@jeyip

jeyip Jul 30, 2021 •
edited
Author Contributor

gutenberg_get_default_block_editor_settings will be merged in the gutenberg_get_block_editor_settings function, which is why we remove it from here.

gutenberg/lib/compat/wordpress-5.8/block-editor.php

Lines 264 to 272 in c96feeb

function gutenberg_get_block_editor_settings( $custom_settings, $block_editor_context ) {
$editor_settings = array_merge(
gutenberg_get_default_block_editor_settings(),
array(
'allowedBlockTypes' => gutenberg_get_allowed_block_types( $block_editor_context ),
'blockCategories' => gutenberg_get_block_categories( $block_editor_context ),
),
$custom_settings
);

@gziolo gziolo requested a review from youknowriad Jul 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gziolo gziolo

@mattwiebe mattwiebe

@spacedmonkey spacedmonkey

@TimothyBJacobs TimothyBJacobs

@nosolosw nosolosw

@youknowriad youknowriad

Requested changes must be addressed to merge this pull request.

Assignees

@jeyip jeyip

Labels
[Feature] Full Site Editing [Feature] Site Editor
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@jeyip @mattwiebe @gziolo