-
Notifications
You must be signed in to change notification settings - Fork 810
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
Likes and Sharing Gutenberg extension: Add another hook to catch late-registered CPTs #12031
Conversation
Caution: This PR has changes that must be merged to WordPress.com |
It looks good for custom post types and posts. When editing pages however, the JP sidebar doesn't show up. Is there a setting I'm missing? |
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.
Looks like this is not a regression and pages just don't seem to have the Jetpack sidebar. Shouldn't be a blocker for this PR, but we should find out the reasons behind it and what it means for sharing & likes settings there
Thanks @obenland! I'm checking with Gutenpack folks on whether we can enable the Jetpack sidebar on pages as well. |
Convo about the Pages issue here #12045 |
I can't seem to be able to get this to work; I don't see the Jetpack sidebar at all when editing Testimonials. |
87c02d3
to
441df54
Compare
Thank you for the great PR description! When this PR is ready for review, please apply the Scheduled Jetpack release: May 7, 2019. |
With the fix for pages in now, Testimonials work for me too. |
@Automattic/jetpack-crew Is there a chance you could give this another look and ✅? |
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.
Fixes for public custom post types. 🎉
Doesn't fix for private custom post types such as re-usable blocks (/wp-admin/edit.php?post_type=wp_block
), though. I'd assume same problem for potential wp_template custom post type down the road.
Thanks @simison ! Indeed I'm noting the non-public post type issue for follow-up work. I think we should just hide the sidebar in that case. |
Switched to "needs review" so that this will show up at Crew's lists. |
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 works well for me now. Merge when ready!
Eagle-eyed testers noticed that this extension (see #11709) wasn't working properly with some custom post types. This PR fixes it by running
register_rest_field
on bothrest_api_init
andrestapi_theme_init
.Changes proposed in this Pull Request:
Some post types, such as testimonials and comics, are conditionally registered if the theme supports them, so instead of being hooked to
rest_api_init
they hook torestapi_theme_init
. This runs afterrest_api_init
, meaning they weren't getting the necessary API fields for the extension to work.The default action priority is 10. SO if we hook to
restapi_theme_init
with a priority of 20 we'll get to add this functionality to any late-registered CPTs.In testing I found that we still need to hook to
rest_api_init
in order for posts to function correctly -- I haven't dug into why (is the theme_init not called for endpoint requests for posts?) but I don't believe there's any harm in callingregister_rest_field
twice as it will simply override the prior call.Testing instructions:
Full testing instructions from #11709:
define( 'JETPACK_BETA_BLOCKS', true );
in yourwp-config.php
.yarn build-extensions
in the checked-out branch.Proposed changelog entry for your changes: