-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Make footnotes support opt-in. #6043
Conversation
|
||
return true; | ||
} | ||
|
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.
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()
.
) | ||
); | ||
} | ||
} |
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 will cause the tests to fail but is required for testing wp_register_footnotes_meta_field
works as expected.
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 Core SVNCore Committers: Use this line as a base for the props when committing in SVN:
GitHub Merge commitsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
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() { |
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.
Could this use get_post_types_by_support()
?
/* | ||
* 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'; | ||
} |
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'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.
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.
That said, +1 for the approach.
if ( in_array( 'footnotes', $features, true ) ) { | ||
$index = array_search( 'footnotes', $features, true ); | ||
unset( $features[ $index ] ); | ||
$features[] = 'footnotes'; | ||
} |
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.
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.
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.
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
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. |
Alternative to: #6003
Related to: WordPress/gutenberg#58573
Gutenberg Issue: WordPress/gutenberg#58341