Improve UX/UI of enabling performance features via standalone plugins

[mukeshpanchal27]

Summary

Fixes #1031

Before:

After:

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: mukeshpanchal27 <mukesh27@git.wordpress.org>
Co-authored-by: felixarntz <flixos90@git.wordpress.org>
Co-authored-by: thelovekesh <thelovekesh@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>
Co-authored-by: joemcgill <joemcgill@git.wordpress.org>
Co-authored-by: ThierryA <tweetythierry@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[swissspidy]

Could you perhaps add a before/after screenshot/video to the PR description? Makes reviews a bit easier :)

[mukeshpanchal27]

@swissspidy Screenshot added in PR description.

[thelovekesh]

When the user loads the performance-lab options page for the first time, the options page takes time to load since it queries the plugin's API. As the number of plugins grows this time will increase. Can we do something to make this step async?

Also, can we use @wordpress/components here instead of SSR'd content? That could handle the above case but it would require a build step in the PL plugin.

[thelovekesh]

@mukeshpanchal27 On the WP trunk the rendered screen seems to be breaking. Can you please confirm? Or am I missing something 🤔

[swissspidy]

This doesn't yet address all the suggestions for an initial improvement aka MVP outlined at #1031 (comment), specifically:

• The feature name and description shouldn't be simply the content pulled from .org, but rather custom content that is more tailored to the user experience of what the feature unlocks.
• We can hard-code the list of features, i.e. the name, description, and underlying plugin slug.
• The logic to get information from wordpress.org can be removed.

The latter two points should also alleviate the load time concern mentioned above

Also, can we use @wordpress/components here instead of SSR'd content? That could handle the above case but it would require a build step in the PL plugin.

That's a bit out of scope for an MVP like this, but something we should consider for future iterations.

[felixarntz]

@swissspidy

This doesn't yet address all the suggestions for an initial improvement aka MVP outlined at #1031 (comment), specifically:

• The feature name and description shouldn't be simply the content pulled from .org, but rather custom content that is more tailored to the user experience of what the feature unlocks.
• We can hard-code the list of features, i.e. the name, description, and underlying plugin slug.
• The logic to get information from wordpress.org can be removed.

This was initially defined as a requirement, but later discarded. See #1031 (comment) for the latest requirements.

Also, can we use @wordpress/components here instead of SSR'd content? That could handle the above case but it would require a build step in the PL plugin.

That's a bit out of scope for an MVP like this, but something we should consider for future iterations.

Agreed, this is out of scope here. Let's discuss such a change separately.

cc @mukeshpanchal27

[felixarntz]

Thank you @mukeshpanchal27, leaving a bit of feedback.

[mukeshpanchal27]

WP trunk the rendered screen seems to be breaking. Can you please confirm? Or am I missing something 🤔

@thelovekesh Trunk version works fine for me. Could you please check if you can replicate the issue?

@felixarntz @westonruter PR ready for review.

[swissspidy]

Should we remove the "Learn more" link? It's redundant right now as you can just click on the title to open the modal.

[mukeshpanchal27]

Should we remove the "Learn more" link? It's redundant right now as you can just click on the title to open the modal.

Certainly, if we agree to remove it, I'll go ahead and do so. However, it's worth noting that it was included in the requirement outlined here: #1031 (comment)

[felixarntz]

@mukeshpanchal27 This looks great, almost good to go. Just a few last points.

[felixarntz]

@swissspidy @mukeshpanchal27 I think we should keep the "Learn more" link. Even though you can access the modal from two places, the Learn more link is more obvious in allowing that than the title being clickable. FWIW, that's also how the regular plugin cards at Plugins > Add New handle it.

[felixarntz]

@mukeshpanchal27 Awesome work, LGTM!

[thelovekesh]

@thelovekesh Trunk version works fine for me. Could you please check if you can replicate the issue?

@mukeshpanchal27 In this particular core commit, few new classes were added to /wp-admin/css/list-tables.css which are causing some styles issues I shared above in #1091 (comment). Can you please check it once again? I think following steps should help with testing on wp-env.

• Destroy existing wp-env containers - npm run wp-env destroy.
• Add override config .wp-env.override.json:

   {
     "core": "WordPress/WordPress#master"
   }

• Run wp-env again - npm run wp-env start.

[felixarntz]

Thanks @thelovekesh, this is a great catch. In that case, it looks like we may need to output CSS conditionally, depending on WordPress version.

The current CSS works correctly for WP <6.5. But based on the changes from that commit, it looks like for WP >=6.5 we will have to include different CSS.

@mukeshpanchal27 Could you test this and conditionally include the CSS for WP >=6.5?

[mukeshpanchal27]

@thelovekesh Great catch. I have added the CSS fix that was introduced in the latest (6.5) version. Could you please take a look when you have a moment?

[felixarntz]

Thanks @mukeshpanchal27 for the update. I've tested this in 6.4 and 6.5-RC3 and it looks good in both now. ✅

[thelovekesh]

LGTM