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

Multiple not related tgmpa_register actions are coupled #786

Open
dominikj111 opened this issue Jan 11, 2020 · 9 comments
Open

Multiple not related tgmpa_register actions are coupled #786

dominikj111 opened this issue Jan 11, 2020 · 9 comments

Comments

@dominikj111
Copy link

Downloaded theme is using tgmpa version 2.6.1, I want to use same version in my personal plugin and because the theme one is loaded first, the class TGM_Plugin_Activation is declared from there.

I used Custom TGMPA Generator where I specified unique text domain and function prefix (Name was generated from text domain).

My register hook is like this:

add_action( 'tgmpa_register', 'pd__register_required_plugins' );

function pd__register_required_plugins() {

   $plugins = array(
   	array(
   		'name'      => 'Hello Dolly',
   		'slug'      => 'hello-dolly',
   		'required'  => false
   	)
   );

   $config = array(
   	'id'           => 'plugin-director',
   	'default_path' => '',
   	'menu'         => 'pd-tgmpa-install-plugins',
   	'parent_slug'  => 'plugins.php',
   	'capability'   => 'manage_options',
   	'has_notices'  => true,
   	'dismissable'  => true,
   	'dismiss_msg'  => '',
   	'is_automatic' => false,
   	'message'      => ''
   );

   tgmpa( $plugins, $config );
}

And the issue is that the 'Hello Dolly' plugin appeared in the theme plugin list. So the parent_slug in the plugin's $config is ignored and probably whole $config is also ignored.

@jrfnl
Copy link
Contributor

jrfnl commented Jan 11, 2020

Correct. The whole point of TGMPA is to present the end-user with one list with all plugins registered from various sources.

You could let TGMPA use your config by changing the priority on the add_action() call, but it will still collect everything on one page.

@dominikj111
Copy link
Author

dominikj111 commented Jan 12, 2020

OK, and wouldn't be better to show the source where the requirement and recommendation come from as many plugin can use this. Or maybe it could be handy to add description column.

@jrfnl
Copy link
Contributor

jrfnl commented Jan 13, 2020

wouldn't be better to show the source where the requirement and recommendation come from as many plugin can use this

Those would be features we'd surely consider if someone would create a PR to that effect.

@dominikj111
Copy link
Author

Those would be features we'd surely consider if someone would create a PR to that effect.

So, could it be like this? Maybe?
Comparison with domino2/TGM-Plugin-Activation

@jrfnl
Copy link
Contributor

jrfnl commented Jan 13, 2020

@domino2 I don't think that would help.

  1. Only one description would show, even if both a theme and a plugin register the same plugin.
  2. It doesn't address your original proposal "showing where the requirement/recommendation comes from".

@dominikj111
Copy link
Author

OK, I can imagine some better solution then, but it will take a little bit more time. I will back then.

@magicoli
Copy link

Hello

It's an old thread, but the issue is still there. And when I say issue it sounds more like a bug to me.
While I agree with the idea of showing only one list (not my preference, but I could deal with it), the way it works now makes the messages unpredictable.

I develop a plugin, and at least two the other plugins/themes I use are using the tgmpa library. Let's say "ANOTHER_PLUGIN" and "ANOTHER_THEME" below...

When I activate my_plugin, I get a scrambled messsage:

  • "ANOTHER_PLUGIN" requires this plugin
  • This theme recommends these plugins

So I changed "this theme" by "this plugin"... But when I activated MY dependencies, the theme notified "This plugin recommends (...) "
I guess ANOTHER_PLUGIN sets custom strings to include its name, and ANOTHER_THEME stick with the default ones.

So if one of the plugins defines custom strings, they could be displayed by any other one, depending on the load priority. Whoever sets the higher priority will scramble messages from other plugins/thems, so priority cannot be a solution.

For the record, the "ANOTHER_PLUGIN" I talk about happens to be one of my dependencies, so I can not choose to use the library, it would guarantee at least one of them (this plugin or mine) would display wrong messages.

IMHO wp users expect plugins to have their own notification, not notifications from apparently unrelated plugins/themes grouped together.

Couldn't we use local or prefixed variables, so each plugin gives its own notifications? I guess that would solve the problem totally.

@dominikj111
Copy link
Author

dominikj111 commented Mar 25, 2021

Hello, I suppose that you want to offer to users of your plugin a better experience when using it. At least I wanted, but I left that idea (not enough time) and maybe worth to put the responsibility to install dependent plugins on users and mention it in the description for now.

Or if you doing it for the business, maybe worth to check some of these articles.
Using composer manage Wordpress themes plugins
Install and manage Wordpress with composer
Composer and Wordpress
and plenty others.

I didn't try, but what about this plugin?
plugin-dependencies

Or maybe use lightweight alternative to TGM Plugin Activation class.

I'm curious what you will end up as I left this problem.

@magicoli
Copy link

Thanks for your suggestions, domino2

I think composer is an option on dev side (I use it and it's great) but not for the end user. I am looking for a solution working entirely from the admin area.

I actually already made a lightweight alternative: slightly more sophisticated than you example, but still very light). I wanted to give TGMPA a try before pushing development further.

As I didn't want to give up, I ended up replacing all prefixes in tgmpa library (tgm, tgmpa, TGMPA, with my plugin prefixes. Although it now works very well (no more conflicts), it's not easy to implement (400+ occurrences of 5 variants) and updates would be a pain.

For the record, I also noticed after my message that if two plugins using the class define a different menu slug or parent_slug, one of them produce broken installation links (and as said before, the link used could be anyone of them, whatever the dependency belongs to).

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

No branches or pull requests

3 participants