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

Make footnotes support opt-in. #6043

Closed

Conversation

peterwilsoncc
Copy link
Contributor

@peterwilsoncc peterwilsoncc commented Feb 6, 2024

Alternative to: #6003

Related to: WordPress/gutenberg#58573

Gutenberg Issue: WordPress/gutenberg#58341

@peterwilsoncc peterwilsoncc changed the title Add/footnotes support Make footnotes support opt-in. Feb 6, 2024

return true;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given the snowflake code in add|remove_post_type_support this block may no be needed. It accounts for developers modifying $_wp_post_type_features directly but it may be possible to consider doing so out of warranty.

If it's decided to keep this then similar code will probably be needed in get_all_post_type_supports() and get_post_types_by_support().

)
);
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will cause the tests to fail but is required for testing wp_register_footnotes_meta_field works as expected.

@peterwilsoncc peterwilsoncc marked this pull request as ready for review February 7, 2024 21:35
Copy link

github-actions bot commented Feb 7, 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.

Core SVN

Core Committers: Use this line as a base for the props when committing in SVN:

Props peterwilsoncc.

GitHub Merge commits

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: peterwilsoncc <peterwilsoncc@git.wordpress.org>

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

*
* @link https://github.com/WordPress/gutenberg/pull/57353
*/
function wp_register_footnotes_meta_field() {
Copy link
Member

Choose a reason for hiding this comment

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

Could this use get_post_types_by_support()?

Comment on lines +665 to +681
/*
* Move footnotes last.
*
* Footnotes are a special case and require other features to be supported:
* 'editor', 'revisions' and 'custom-fields'. This moves the footnotes to
* be added last so that the other features are already added before the
* check in `add_post_type_support` is made.
*/
if ( isset( $this->supports['footnotes'] ) ) {
$footnotes = $this->supports['footnotes'];
unset( $this->supports['footnotes'] );
$this->supports['footnotes'] = $footnotes;
} elseif ( in_array( 'footnotes', $this->supports, true ) ) {
$index = array_search( 'footnotes', $this->supports, true );
unset( $this->supports[ $index ] );
$this->supports[] = 'footnotes';
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest moving this code to its method and then invoking it here before the loop. What's going on here as a very specific purpose beyond but in support of add_supports(). It's like the prep work for before adding the supports.

Copy link
Contributor

Choose a reason for hiding this comment

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

That said, +1 for the approach.

Comment on lines +2218 to +2222
if ( in_array( 'footnotes', $features, true ) ) {
$index = array_search( 'footnotes', $features, true );
unset( $features[ $index ] );
$features[] = 'footnotes';
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this code will run 2x, as it's also being done in WP_Post_Type::add_supports() which first moves it to last and then invokes add_post_type_support(). Wondering if it can be combined to do this only once.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking more about this ...

How about moving the "move to last" code in WP_Post_Type::add_supports() to here?

What might the impact be within WP_Post_Type's usage of its supports property? Hmm

@peterwilsoncc
Copy link
Contributor Author

Closing this off as the discussion in the GB repo is to make footnotes opt-out via the block APIs rather than a post type feature.

@peterwilsoncc peterwilsoncc deleted the add/footnotes-support branch February 8, 2024 21:17
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.

3 participants