-
Notifications
You must be signed in to change notification settings - Fork 801
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
Clean up duplicate post types in featured content #10196
Conversation
Add array_unique for the post_types list. array_merge appends arrays, and since post is set as the default, it means that for many post will be listed twice. For example in Canard (an Automattic theme) where post_type is set to post and page the post_type will become array( 'post', 'post', 'page' ) https://github.com/Automattic/themes/blob/b8444897830573b47505a19623e1f3459a45aac4/canard/inc/jetpack.php#L22 By filtering the list we prevent the register_taxonomy_for_object_type from being called multiple times for the same post type, and we ensure the post_types are listed once when we later call get_posts. To be honest I don't imagine it will make a huge amount of difference to anything but I was debugging a featured content issue in my theme and saw this and it confused me. This eliminates that confusion.
This is automated check which relies on Generated by 🚫 dangerJS |
Could you add testing instructions to your PR so we can easily reproduce the issue and see if your branch fixes things? Thank you! |
done |
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.
Thanks for the extra steps. It works well for me! 👍
* Readme: add boilerplate for next release, 6.6 * Add 6.5 to the changelog.txt file * Set boilerplate testing list for 6.6 * Readme: update stable tag to 6.5 * Add bullets to 6.5 changelog items * Readme: add link to previous changelogs This will help folks who want to know more about past releases, while keeping the readme.txt short so as to not overwhelm translators and site owners only looking for information about the last release. * Changelog: add information at the top of the changelog file. * Changelog: add #10054 * Changelog: add #10078 * Changelog: add #10079 * Changelog: add #10064 * Changelog: add #10094 * Changelog: add #10096 * Testing list: add more information based on #10087 * Changelog: add #9847 * Changelog: add #10084 * Changelog: add #9918 * Changelog: add #7614 * Changelog: add #10116 * Changelog: add #10108 * Changelog: add #10041 * Changelog: add #10121 * Changelog: add #10134 * Changelog: add #10130 * Changelog: add #10109 * changelog: add #10137 * changelog: add #9952 * changelog: add #10120 * changelog: add #10162 * Changelog: add #10163 * Changelog: add #10092 * changelog: add #10156 * Changelog: add #10154 * changelog: add #10122 * Changelog: add #10101 * changelog: add #10105 * changelog: add #10190 * Changelog: add #10196 * changelog: add #10152 * Changelog: add #10153 * Testing list: add more details to Site Verification testing steps. @see #10143 (comment) * changelog: add #10194 * Changelog: add #10193
Caution: This PR has changes that must be merged to WordPress.com |
Add array_unique for the post_types list.
array_merge appends arrays, and since post is set as the default, it means that for many post will be listed twice.
For example in Canard (an Automattic theme) where post_type is set to post and page the post_type will become array( 'post', 'post', 'page' )
https://github.com/Automattic/themes/blob/b8444897830573b47505a19623e1f3459a45aac4/canard/inc/jetpack.php#L22
By filtering the list we prevent the register_taxonomy_for_object_type from being called multiple times for the same post type, and we ensure the post_types are listed once when we later call get_posts.
To be honest I don't imagine it will make a huge amount of difference to anything but I was debugging a featured content issue in my theme and saw this and it confused me. This eliminates that confusion.
Testing instructions
To test you should add theme support for Jetpack Featured Content as normal, but make sure to include the additional post types.
Then, somewhere in your theme, add:
var_dump( Featured_Content::$post_types );
And you should see that post is being listed twice.