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

Template Loader: Drop $_wp_current_template_id #22196

Merged

Conversation

ockham
Copy link
Contributor

@ockham ockham commented May 7, 2020

Description

Final piece of a refactor that started with #21959, #21980, and #22143.

This PR changes the gutenberg_template_loader_filter_block_editor_settings filter to detect if we're dealing with the Site Editor from the $settings array, allowing us to drop the $_wp_current_template_id global.

How has this been tested?

Verify that Full Site Editing still works as before.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • 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.

@github-actions
Copy link

github-actions bot commented May 7, 2020

Size Change: 0 B

Total Size: 1.13 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.26 kB 0 B
build/block-directory/style-rtl.css 955 B 0 B
build/block-directory/style.css 955 B 0 B
build/block-editor/index.js 106 kB 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/editor-rtl.css 7.85 kB 0 B
build/block-library/editor.css 7.86 kB 0 B
build/block-library/index.js 129 kB 0 B
build/block-library/style-rtl.css 8.02 kB 0 B
build/block-library/style.css 8.02 kB 0 B
build/block-library/theme-rtl.css 749 B 0 B
build/block-library/theme.css 751 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 196 kB 0 B
build/components/style-rtl.css 15.9 kB 0 B
build/components/style.css 15.9 kB 0 B
build/compose/index.js 9.6 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.17 kB 0 B
build/edit-navigation/index.js 8.26 kB 0 B
build/edit-navigation/style-rtl.css 1.04 kB 0 B
build/edit-navigation/style.css 1.04 kB 0 B
build/edit-post/index.js 303 kB 0 B
build/edit-post/style-rtl.css 5.6 kB 0 B
build/edit-post/style.css 5.6 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3.13 kB 0 B
build/edit-site/style.css 3.13 kB 0 B
build/edit-widgets/index.js 9.34 kB 0 B
build/edit-widgets/style-rtl.css 2.54 kB 0 B
build/edit-widgets/style.css 2.54 kB 0 B
build/editor/editor-styles-rtl.css 486 B 0 B
build/editor/editor-styles.css 487 B 0 B
build/editor/index.js 44.8 kB 0 B
build/editor/style-rtl.css 3.82 kB 0 B
build/editor/style.css 3.82 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 561 B 0 B
build/format-library/style.css 562 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 537 B 0 B
build/list-reusable-blocks/style.css 537 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 681 B 0 B
build/nux/style.css 676 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 788 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@ockham ockham force-pushed the update/template-hierarchy-no-current-template-id-global branch from 9875c5e to e1f8148 Compare May 12, 2020 13:03
@ockham ockham marked this pull request as ready for review May 12, 2020 13:09

if ( ! $post || ! post_type_exists( 'wp_template' ) || ! post_type_exists( 'wp_template_part' ) ) {
return $settings;
}

// Create template part auto-drafts for the edited post.
$post = isset( $_wp_current_template_id )
? get_post( $_wp_current_template_id ) // It's a template.
$post = isset( $settings['templateId'] )
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is only set in edit-site. This would break the post editor with template parts then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Revisiting this, I'm not sure we actually need the ternary checking for $_wp_current_template_id at all.

That global is (only) set by gutenberg_override_query_template(), which (via gutenberg_find_template_post_and_parts()) also calls create_auto_draft_for_template_part_block(). So the template case should be covered, leaving only the post editor case.

Copy link
Contributor Author

@ockham ockham Jun 17, 2020

Choose a reason for hiding this comment

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

But that sounds wrong, since this is clearly targeted to the editor case, and gutenberg_override_query_template() is only called for front-end template resolution. That might be an oversight on my part going back to #21959 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However, gutenberg_find_template_post_and_parts is called for all template types in lib/edit-site-page.php, so template part auto-drafts are indeed auto-generated for the site editor.

Copy link
Member

Choose a reason for hiding this comment

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

How would this make sense in the post editor anyways? i.e. how would there be a template set for an individual post? And if there is a template set, why do we care about the template parts associated with the template (at this point in time)? Wouldn't we only care about template parts inside the post_content of the post?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't we only care about template parts inside the post_content of the post?

Exactly, yeah 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed d6dc5a3 to reflect these musings in code.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is leftover from when you could switch to a template view in the post editor.

@ockham ockham force-pushed the update/template-hierarchy-no-current-template-id-global branch from 8958614 to c8e32f5 Compare June 17, 2020 22:09
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

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

This tests well on my end! Were there any remaining concerns here?

@ockham ockham changed the title Template Loader: Infer current template ID from settings Template Loader: Drop $_wp_current_template_id Jun 22, 2020
@ockham ockham merged commit ded587c into master Jun 22, 2020
@ockham ockham deleted the update/template-hierarchy-no-current-template-id-global branch June 22, 2020 10:02
@github-actions github-actions bot added this to the Gutenberg 8.5 milestone Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants