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

Update: Increase footnotes meta priority and separate footnotes meta registration. #58882

Merged
merged 4 commits into from Feb 16, 2024

Conversation

jorgefilipecosta
Copy link
Member

This PR does two actions:

  • Separates the footnotes meta registration from the block registration.
  • Increases the priority of the registration to 20.

Most post types are registered on init with a priority of 10 having a priority of 20 makes sure the footnotes meta is registered for all of the post types with a priority of 20. While also allowing plugins to change the footnotes meta if they have an advanced need ( as long as they use a priority higher than 20).

Testing Instructions

  • In the core install you are using to test Gutenberg comment the line add_action( 'init', 'register_block_core_footnotes' ); in src/wp-includes/blocks/footnotes.php (otherwise, the meta is registered there anyway).
  • Verify it is still possible to add footnotes to a post.

Copy link

github-actions bot commented Feb 9, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org>
Co-authored-by: peterwilsoncc <peterwilsoncc@git.wordpress.org>
Co-authored-by: mcsf <mcsf@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

*
* @since 6.5.0
*/
function wp_register_footnotes_post_meta() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe??

Suggested change
function wp_register_footnotes_post_meta() {
function register_block_core_footnotes_post_meta() {

I'm unclear why the post types are limited to public. If someone has a private post type using the block editor, they probably want access to footnotes.

That said, if there are reasons then post types can be public but not actually public. You'll need to change the code below to:

	$post_types = get_post_types( array( 'show_in_rest' => true ) );
	$post_types = array_filter( $post_types, 'is_post_type_viewable' ) );

Outside the diff sorry so can't do an inline suggestion

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @peterwilsoncc, your points are well taken! Footnotes work even on non-public post types, so I removed the public filter. I also updated the function name. Thank you for the suggestions 👍

@jorgefilipecosta jorgefilipecosta force-pushed the update/increase-footnotes-meta-priority branch from e19d9e9 to d6efccb Compare February 12, 2024 21:15
Copy link

Flaky tests detected in d6efccb.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7878017123
📝 Reported issues:

Copy link
Contributor

@mcsf mcsf left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this. Let's not forget about upstream

packages/block-library/src/footnotes/index.php Outdated Show resolved Hide resolved
@jorgefilipecosta jorgefilipecosta force-pushed the update/increase-footnotes-meta-priority branch from 11d1e5a to a197383 Compare February 16, 2024 16:51
@jorgefilipecosta jorgefilipecosta merged commit 96cb434 into trunk Feb 16, 2024
56 checks passed
@jorgefilipecosta jorgefilipecosta deleted the update/increase-footnotes-meta-priority branch February 16, 2024 17:40
@github-actions github-actions bot added this to the Gutenberg 17.8 milestone Feb 16, 2024
@youknowriad youknowriad added the Backport to WP Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 20, 2024
getdave pushed a commit that referenced this pull request Feb 20, 2024
…registration. (#58882)

Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org>
Co-authored-by: peterwilsoncc <peterwilsoncc@git.wordpress.org>
Co-authored-by: mcsf <mcsf@git.wordpress.org>
@getdave
Copy link
Contributor

getdave commented Feb 20, 2024

I just cherry-picked this PR to the cherry-pick-beta-2 branch to get it included in the next release: 245b544

@getdave getdave removed the Backport to WP Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 20, 2024
youknowriad pushed a commit that referenced this pull request Feb 20, 2024
…registration. (#58882)

Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org>
Co-authored-by: peterwilsoncc <peterwilsoncc@git.wordpress.org>
Co-authored-by: mcsf <mcsf@git.wordpress.org>
@@ -105,8 +105,10 @@ function register_block_core_footnotes_post_meta() {
}
}
}
// Most post types are registered at priority 10, so use priority 20 here in
// order to catch them.
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

@jorgefilipecosta Sorry, I missed this at the time -- comments should start with /* (a single asterisk) rather than a double asterisk). The latter is used for docblocks only to allow them to be picked up by parser for developer.wordpress.org

Are you able to do a follow up commit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure doing a follow up fix, thank you for catching this 👍

@getdave getdave changed the title Update: Increate footnotes meta priority and separate footnotes meta registration. Update: Increase footnotes meta priority and separate footnotes meta registration. Feb 21, 2024
@creativecoder creativecoder added [Type] Enhancement A suggestion for improvement. [Block] Footnotes Affects the Footnotes Block labels Feb 21, 2024
@creativecoder
Copy link
Contributor

Note that as part of curating the Gutenberg 17.8 changelog, I added the labels [Type] Enhancement and [Block] Footnotes to this PR to better categorize it.

Please let me know if there is a different label that would be a better fit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Footnotes Affects the Footnotes Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants