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

Recognize plugins #8

Merged
merged 6 commits into from
Apr 19, 2024
Merged

Conversation

rlankhorst
Copy link
Contributor

Context

I think it would be really helpful for users if the actual plugin or theme that sets the option is listed. The user can then easily see if the plugin is still installed or not. A later improvement could be to automatically check the active/installed status, and show that as well.

Summary

This PR can be summarized in the following changelog entry:

  • Show plugin or theme that places the option

Relevant technical choices:

  • I've chosen to create a plugin list as array in a php file, as opposed to a csv or text file, as I think this is the fastest way to handle this. Also, PHP array gives easy structuring options, like nested arrays for option prefixes lists.
  • I haven't added unit tests yet, first waiting to see if there's interest in this approach :)

Test instructions

  • Install for example WooCommerce. It should show in the column as 'Woocommerce'

Relevant test scenarios

  • Changes should be tested with the browser console open
  • Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
  • Changes should be tested on different editors (Block/Classic/Elementor/other)
  • Changes should be tested on different browsers
  • Changes should be tested on multisite

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Documentation

  • I have written documentation for this change.

Quality assurance

  • [v ] I have tested this code to the best of my abilities.
  • I have added unit tests to verify the code works as intended.
  • I have checked that the base branch is correctly set.

Fixes #

@jdevalk
Copy link
Member

jdevalk commented Apr 19, 2024

I really like this idea... I'm struggling a bit with how to do this... "nicely". We should probably have a database online that has this data where people can register their options prefixes.

@rlankhorst
Copy link
Contributor Author

I think as a first iteration, it will make it easier to just have it in the plugin, and let people do PR's to add new ones. A bit like with Wappalyzer: https://github.com/HTTPArchive/wappalyzer/tree/main/src/technologies.
I think most submitters will be plugin developers, so probably won't have a problem with doing a PR to add a prefix.

They do it in json, but I don't think that has added value in this case. But it might be useful to split it in alphabetically separated files, like it's done there.

I think maintaining an online database and handling submissions will be a lot of continuous work, and might take a lot of resources later on (although using S3 with downloadable jsons might limit that). You'd also need to pull that in to the plugin. So I would personally prefer the "in the plugin" approach to keep it simple and straight forward.

Because the current setup is very simple, this makes it easy to add plugins/prefixes for all contributors, but it is quite possible there is a more efficient or elegant way to do this. On the other hand, I always like to have a starting point, and take it from there. My guess is there will be suggestions for improvements once it's there. Another advantage of keeping it in the plugin is that all dev decisions kan be changed or reverted later on without breaking stuff.

@rlankhorst
Copy link
Contributor Author

I just noticed you changed it to json, probably a better standard which is easier to exchange and maintain in the long run. My guess would be that a json will be slightly slower when compared to loading a php file, but in this case performance is probably not an issue, as it only is loaded sporadically on the back-end.

@jdevalk
Copy link
Member

jdevalk commented Apr 19, 2024

I was running into more issues as I was playing with this so fixed a few other things along the way 😅

The JSON is easier to maintain I think, and we could read that from a remote thing if we wanted to later on.

Going to merge and release :)

@jdevalk jdevalk merged commit 9921800 into Emilia-Capital:main Apr 19, 2024
6 of 7 checks passed
@rlankhorst
Copy link
Contributor Author

Great! 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants