DO NOT MERGE | POC | Enhancement / 652 Migration UI From Old Modules to Standalone Plugins #806
Conversation
…es-to-standalone-plugins
…ation-ui-from-old-modules-to-standalone-plugins
… plugin managment UI and REST endpoints.
…in applicable load files, enqueue necessary scripts for settings screen.
There was a problem hiding this comment.
@10upsimon I left a bit of early feedback, mostly related to the overall approach outlined on #652.
On another note, please create a feature branch for this work, and then this PR should be based on that instead of trunk.
| // Load Composer dependencies if applicable. | ||
| if ( file_exists( PERFLAB_PLUGIN_DIR_PATH . '/vendor/autoload.php' ) ) { | ||
| require_once PERFLAB_PLUGIN_DIR_PATH . '/vendor/autoload.php'; | ||
| } |
There was a problem hiding this comment.
While Composer is generally great, using it in a WordPress plugin comes with additional quirks and interoperability problems that I would prefer to avoid unless truly beneficial. Let's require_once any class files instead, as there shouldn't be any meaningful performance benefit from autoloading those few files anyway.
| * @access private | ||
| * @ignore | ||
| */ | ||
| class REST_Plugins_Controller { |
There was a problem hiding this comment.
Do we really need those REST routes? While I agree the REST API is generally the recommended approach over the oldschool AJAX request, since we are working on a WP core feature plugin here and since this functionality is supplemental, we should keep our maintenance burden as small as possible and use what core already provides. Is there a good reason not to use wp_ajax_install_plugin() and WP_REST_Plugins_Controller?
| @@ -48,5 +48,8 @@ | |||
| "psr-4": { | |||
| "PerformanceLab\\Tests\\": "tests/utils" | |||
| } | |||
| }, | |||
| "autoload": { | |||
| "classmap": ["includes/"] | |||
There was a problem hiding this comment.
See above, let's avoid the usage of Composer within the actual plugin.
| $managed_standalone_plugins = array( | ||
| 'webp-uploads' => array( | ||
| 'Name' => esc_html__( 'WebP Uploads', 'performance-lab' ), | ||
| 'Description' => esc_html__( 'This plugin adds WebP support for media uploads within the WordPress application.', 'performance-lab' ), | ||
| 'Author' => esc_html__( 'WordPress Performance Team', 'performance-lab' ), | ||
| 'AuthorURI' => esc_url( 'https://make.wordpress.org/performance/' ), | ||
| 'PluginURI' => esc_url( 'https://wordpress.org/plugins/webp-uploads/' ), | ||
| 'Download' => 'wporg', | ||
| ), | ||
| 'sqlite-database-integration' => array( | ||
| 'Name' => esc_html__( 'SQLite Database Integration', 'performance-lab' ), | ||
| 'Description' => esc_html__( 'Allows testing an SQLite integration with WordPress and gather feedback, with the goal of eventually landing it in WordPress core.', 'performance-lab' ), | ||
| 'Author' => esc_html__( 'WordPress Performance Team', 'performance-lab' ), | ||
| 'AuthorURI' => esc_url( 'https://make.wordpress.org/performance/' ), | ||
| 'PluginURI' => esc_url( 'https://wordpress.org/plugins/sqlite-database-integration/' ), | ||
| 'Download' => 'wporg', | ||
| ), | ||
| ); |
There was a problem hiding this comment.
All this shouldn't be hard-coded. Instead, we should rely on the plugin.json file for the slugs of which plugins to cover here, and all other data should come from the WordPress.org API, which we can query in the same way that it's done on the Plugins > Add New screen.
|
|
||
| $managed_standalone_plugins[ $plugin_slug ]['Status'] = $status; | ||
| $managed_standalone_plugins[ $plugin_slug ]['Slug'] = $plugin_slug; | ||
| $managed_standalone_plugins[ $plugin_slug ]['HandoffLink'] = isset( $managed_standalone_plugins[ $plugin_slug ]['EditPath'] ) ? admin_url( $managed_standalone_plugins[ $plugin_slug ]['EditPath'] ) : null; |
There was a problem hiding this comment.
What does "HandoffLink" refer to? What is it for?
| return new WP_Error( 'wpp_plugin_failed_deactivation', __( 'Failed to deactivate plugin.', 'performance-lab' ) ); | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Similarly to my comment on the REST classes, why do we need methods to activate and deactivate? Can't we use what core already provides for that purpose?
10upsimon
deleted the
enhancement/652-migration-ui-from-old-modules-to-standalone-plugins
branch
Summary
Addresses: #652
Relevant technical choices
Checklist
[Focus]orInfrastructurelabel.[Type]label.no milestonelabel.