Skip to content
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

Add "Recommended Plugins" input box #402

Merged
merged 28 commits into from
Jun 23, 2023
Merged

Conversation

mikachan
Copy link
Member

@mikachan mikachan commented Jun 19, 2023

This PR adds a "Recommended Plugins" input to the wp-admin page and editor versions of the plugin.

Closes #399.

Testing Instructions:

  1. Open the wp-admin page or the editor sidebar panel
  2. Add some text to the recommended plugins input box
  3. Export the theme
  4. The text should be added to the theme's readme, under a "Recommended Plugins" heading

Screenshots:

wp-admin:
image

Editor sidebar panel:
image

@mikachan mikachan added the enhancement New feature or request label Jun 19, 2023
@mikachan mikachan self-assigned this Jun 19, 2023
@mikachan mikachan marked this pull request as ready for review June 20, 2023 09:41
@pbking
Copy link
Contributor

pbking commented Jun 20, 2023

I see the additional form on the editor. However, nothing sticks when I edit it.
image

I don't see the form on the wp-admin 'Create Blank' form. (Though I do see it on Clone and Create Child).

The forms don't seem to add the value to the exported theme though.

@mikachan
Copy link
Member Author

Thanks for testing, @pbking!

I see the additional form on the editor. However, nothing sticks when I edit it.

It looks like the editor version was only updating the style.css file, so I've updated this PR so that the readme.txt file is also updated from the editor.

I don't see the form on the wp-admin 'Create Blank' form.

This is intentional (similar to how the image credits input works), as I was thinking that people wouldn't want to add recommended plugins to a blank theme. I've also removed the 'Recommended Plugins' input from the 'Create' sidebar panel, as I'm now thinking it's only useful on the 'Update' panel for now. We could revisit this when we align the wp-admin version and editor versions closer together.


$default_copyright_section = "== Copyright ==
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've refactored this and moved it lower down so that it's contained in copyright_section().

Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a placeholder with an example? It's been hard to find an explanation of this section; for instance, it's not listed in the docs https://developer.wordpress.org/themes/basics/main-stylesheet-style-css/

I wondered if I should input a URL or if just the plugin's name would suffice, for example. I can also picture folks wondering if they should separate plugins with a comma or if the textarea will be printed into the document as is.

Copy link
Contributor

@jffng jffng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested and verified this is working as expected in the wp-admin and site editor flows.

While some of the changes are just formatting, I do think this is a lot of code to add for what is effectively just reading/writing some unstructured text to the readme, without enforcement or validation of those plugins coming from wp.org, for example. Unless I am missing something? It's a fair suggestion, but what's the real utility? For example, Gutenberg has a mechanism to prompt users to install respective plugins for blocks that are missing from templates / patterns a theme supplies.

I don't mean to block the PR, just adding my general thoughts.

return response.data;
}
return {
recommendedPlugins: '',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to return an object with only this key as an empty string? Seems like a strange exception.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated this so it's now returning an empty object with no specific keys.

Copy link
Contributor

@jffng jffng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, I can see how having this info is useful for theme authors to inform users what plugins to install, and a lot of the changes are just refactoring / formatting, so giving it the ✅ .

@mikachan
Copy link
Member Author

Thanks, both, for testing this out!

It's been hard to find an explanation of this section

I was also looking for an example, and the best reference I can find is here: https://make.wordpress.org/themes/handbook/review/required/#6-plugins. I think a placeholder is a good idea, I've added one here: 1b293ab, which looks like this:

Plugin Name
https://wordpress.org/plugins/plugin-name/
Plugin Description

and a lot of the changes are just refactoring / formatting

Yes... sorry about that, this snowballed as I saw loads of improvements to the existing readme logic! Thank you for working through it.

@pbking and I paired on this earlier today, and with our latest changes, the 'Recommended Plugins' textarea in the editor will now pre-populate with data from the current readme file, if this section already exists. This functionality also makes it easier to expand in the future for other parts of the readme file (e.g. image credits). I think this enhances the UX, as the user sees some feedback from updating the theme data.

Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the placeholder! I gave it another go and it LGTM.

@pbking
Copy link
Contributor

pbking commented Jun 23, 2023

I made a teeny tiny tweak to the regex processing to ensure that if another section is in readme.txt following the recommended plugins section that the title == TITLE HERE == of the section remains unchanged (it was removing the first =='s)

Otherwise this cleaned up great! Well done @mikachan ! This was a blast to pear on!

@pbking pbking merged commit b5c1f5b into trunk Jun 23, 2023
@pbking pbking deleted the add/recommended-plugin-input branch June 23, 2023 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a text input to handle recommended plugins
4 participants