-
Notifications
You must be signed in to change notification settings - Fork 800
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
Admin menu: Change order of Jetpack sub-menu items #39095
Conversation
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 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Follow this PR Review Process:
Still unsure? Reach out in #jetpack-developers for guidance! Beta plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Boost plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Social plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Protect plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
…1000 was resulting in the Jetpack menu item remaining on JN sites
@@ -64,6 +64,7 @@ private function __construct() { | |||
|
|||
add_action( 'admin_init', array( $jetpack_react, 'react_redirects' ), 0 ); | |||
add_action( 'admin_menu', array( $jetpack_react, 'add_actions' ), 998 ); | |||
add_action( 'admin_menu', array( $jetpack_react, 'remove_jetpack_menu' ), 2000 ); |
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 line and the relevant function borrowed from #38768.
The priority has been changed to 2000 though as the Jetpack menu item was still appearing on JN test sites.
key: 'activityLog', | ||
pattern: 'https://jetpack.com/redirect/?source=cloud-activity-log-wp-menu', |
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.
It seems to me this isn't needed, as I don't see how we'd be able to see the Activity Log menu item as a selected item, since it is an external link. Leaving for now to ensure I'm not introducing any bugs but this can also be removed if there is no known reason to keep it.
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 tests well for me, on single and on multisite., and on WoA Nice work!
I keep finding it weird to have "Settings" before "Dashboard", and at the very end of the list, but I suppose I'll get used to it!
Fixes https://github.com/Automattic/jetpack-roadmap/issues/856
Continuation of #38768
Proposed changes:
View the order of items here
My JetpackStats
Boost
VideoPress
Social
Protect
Akismet Anti-spam
VaultPress Backup
Activity Log (External)
Scan
Search
Subscribers (External)
WordAds
Settings
Dashboard
Jetpack Manage (External)
Beta Tester
jetpackPageOrder
(ensuring the correct menu item is showing as selected). Since there is a level of urgency in getting this change out I'll follow up separately: https://github.com/Automattic/vulcan/issues/482add_jetpack_submenu
inmu-plugins/jetpack-newsletter.php
Other information:
Jetpack product discussion
See https://github.com/Automattic/jetpack-roadmap/issues/856
Does this pull request change what data or activity we track or use?
No.
Testing instructions:
This is relevant for self-hosted and WoA sites - Simple sites override menu locations.
define( 'ENABLE_WORDADS_SHARED_UI', true );
in yourwp-config.php
file. Subscribers will be added if you enable the Newsletter feature at/wp-admin/admin.php?page=jetpack#/newsletter
. I found the best way to add the other items if not testing locally is using the Jetpack Beta tester plugin, then activating all relevant plugins. However as Akismet is not included with the Beta tester plugin, you'd have to apply the branch for a plugin that uses the admin-ui package, which is most (eg. Jetpack, Beta plugin, Boost..).