-
Notifications
You must be signed in to change notification settings - Fork 843
Social: Move Share post action from post-list to Publicize #46323
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
base: trunk
Are you sure you want to change the base?
Conversation
Move the Share post action functionality from the post-list package to the Publicize package for better discoverability. The Share action now appears automatically for users with plans that support the 'republicize' feature, while maintaining backward compatibility with the existing `jetpack_post_list_display_share_action` filter.
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Social plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 4 files.
|
manzoorwanijk
left a comment
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 PR is a good and timely reminder that we need to wire up this with the unified modal.
@manzoorwanijk You mean you want it to open the modal as well? Current trunk behaviour is opening the editor with the Jetpack tab open |
No, the current behavior is to open the resharing modal. You may be testing on a site with unified UI feature flag. I have created a task for it - SOCIAL-277 |
|
It's done here jetpack/projects/js-packages/publicize-components/src/components/block-editor/shared-utils.ts Lines 24 to 25 in 5988af7
|
|
Yeah the flag could be the cause. I'll extend this PR to make it work with the new modal |
|
Or it might also make sense to close this with the move, and I can create a separate one for fixing the modal, to keep them atomic |
Yeah, we can do that and it's better to keep them separate |
manzoorwanijk
left a comment
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 but I have a suggestion about a minor improvement to retain the current behavior.
Also, there is some logic related to it (jetpack_post_list_display_share_action) in the Social plugin as well. Should we clean that up?
|
I have made that unified modal change in #46336. That should make it easier to test this change with and without the unified UI feature flag. |
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.
Pull request overview
This PR moves the "Share post" row action from the post-list package to the Publicize package to improve discoverability. The Share action is now automatically enabled for sites with plans that support the republicize feature, while maintaining backward compatibility with the existing jetpack_post_list_display_share_action filter.
Key Changes:
- The Share action is now automatically shown for plans with
republicizesupport (Jetpack Social or higher) - Added new methods
maybe_add_share_action()andadd_share_action()toPublicize_Setupclass - Maintained backward compatibility with the
jetpack_post_list_display_share_actionfilter
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
projects/packages/publicize/tests/php/Share_Action_Test.php |
New test file covering Share action functionality including filter behavior, screen checks, and post type support validation |
projects/packages/publicize/src/class-publicize-setup.php |
Adds Share action implementation with automatic plan-based activation and filter hook integration |
projects/packages/publicize/changelog/add-share-post-action |
Documents the addition of Share action to Publicize package |
projects/packages/post-list/tests/php/Post_List_Test.php |
Updates tests to remove Share action assertions and focus on thumbnail/copy link functionality |
projects/packages/post-list/src/class-post-list.php |
Removes Share action methods (maybe_add_share_action and add_share_action) |
projects/packages/post-list/changelog/remove-share-action |
Documents the removal of Share action from post-list package |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add proper tear_down() method that clears the Current_Plan static cache using reflection to prevent test isolation issues with subsequent tests that depend on plan feature checks.
|
@gmjuhasz what about this?
|
|
Yeah that got lost in the comments. Added it now @manzoorwanijk . Also removed from the wpcomsh plugin as now the filter defaults to true so its redundant |
|
I am waiting for the build to finish to test the wpcomsh change on Atomic sites. |
manzoorwanijk
left a comment
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.
Works fine on Jetpack, Atomic, and Simple ✅
Thank you for the changes.
| */ | ||
| function wpcomsh_post_list_init() { | ||
| add_filter( 'jetpack_block_editor_republicize_feature', '__return_true' ); | ||
| wpcomsh_maybe_enable_share_action(); |
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.
Will this change cause issues on Atomic when they receive the wpcomsh change today but Jetpack change on Monday?
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.
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.
Otherwise, we can extract the wpcomsh change into a separate PR and merge that after the next Atomic deployment. That will unblock this PR.
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 think it's fine to wait until Monday, it's not urgent to ship, and not likely to get conflicts either
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.
Sounds good. Thanks
Let us wait till Monday to merge this.

Fixes SOCIAL-16
Proposed changes:
post-listpackage to thePublicizepackage for better discoverabilityrepublicizefeaturejetpack_post_list_display_share_actionfilterOther information:
Does this pull request change what data or activity we track or use?
No
Testing instructions:
On a site with a plan that supports
republicize(Jetpack Social or higher):On a site without
republicizesupport:add_filter( 'jetpack_post_list_display_share_action', '__return_true' );to enable itVerify the Share action only appears for:
publicize