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

How do I create multiple unique instances? #184

Open
qwertydude opened this issue Mar 28, 2014 · 28 comments
Open

How do I create multiple unique instances? #184

qwertydude opened this issue Mar 28, 2014 · 28 comments

Comments

@qwertydude
Copy link

I've got five plugins I want using TGM-P-A, and each might have different plugin dependencies.

However, in testing, I'm only getting messages from one (probably the last or first loaded by WP).

So I need each of my plugins to display their own dependency message.

@tivnet
Copy link
Contributor

tivnet commented Mar 28, 2014

@qwertydude Do you have TGM class in each of your plugins? Or do you load TGM in theme? Somewhere else?

@tivnet
Copy link
Contributor

tivnet commented Mar 28, 2014

I made a TGM-based plugin example for myself, and tested together with a theme. Got combined messages from both. https://github.com/TIVWP/tivwp-dm (see docs/examples.php for what I did in theme)

@qwertydude
Copy link
Author

TGM class is in each plugin. Not using it at all in themes or in the theme on my site.

Will check out that info

@tivnet
Copy link
Contributor

tivnet commented Mar 28, 2014

@thomasgriffin Thomas, all the problems with multiple instances - why not have a plugin-wrapped version of TGM, and let everyone call the hook(s)? There should be only one TGM.
We did it in ReduxFramework, and it's same story here I believe.

@qwertydude
Copy link
Author

Not sure a plugin wrapped version would work. How do you then make it a dependency when you need it to be the dependency checker?

Have a look at WP Updates. They have a way of uniquely id'ing each instance of their class.

I think something similar, where the dev has to add their unique ID, could or should work with TGMPA.

We've been doing it for years with functions to avoid conflicts with other plugins and WP - eg myid_get_title();

I would have also thought if we had to instantiate it in our plugin (instead of it being self-instantiating), it would have been fine. i.e. $mydependencies = new TGM_Plugin_Activation();

@qwertydude
Copy link
Author

I think it's mandatory that unique instances is a feature within TGMPA. Definitely not something devs should need to tack on, or else it won't always happen.

If plugin X and plugin Y are both using TGMPA, only one of them (the last loaded, I believe from testing) will display their dependencies.

They will also require unique settings in the user table so dismissing a notice doesn't clear it for all plugins using TGMPA..

@qwertydude
Copy link
Author

Oops! My apologies. I am a little wrong. I think I was partly misled by the default wording that says "This theme requires..."

Indeed TGMPA does appear to gather all required plugins into one message. I think that message should be a little clearer now TGMPA support plugins.

The single message could be, for example: The current theme and/or plugins requires the following plugin

@qwertydude
Copy link
Author

There is a problem tho with the Dismiss Notice value. I've set one plugin to false and one to true, but it's showing both of them in a dismissable box.

@thomasgriffin
Copy link
Contributor

The dismiss_notice key is a config setting and does not apply to individual plugins.

@jamesckemp
Copy link

I have a similar problem to this. I want to change the strings to read "'Plugin Name' requires the following plugin: %1$s.", which for me is better than 'this theme'. However, it always uses the last one loaded.

So I could have a requirement in one plugin, but it will use the strings from my last plugin and give the wrong plugin name. I thought the 'id' parameter would solve the issue.

Is it possible to not merge all requirements into one message?

@jrfnl
Copy link
Contributor

jrfnl commented Mar 19, 2015

@jamesckemp As it's not the same issue, please check if there is an issue already open for it or if not, open a new issue for it.
FYI: we intend to solve this is v3 of the plugin for which work will start very soon.

@jamesckemp
Copy link

@jrfnl Great! Sounds good. Although, I believe it is the same issue?

@jrfnl jrfnl added this to the 3.0.0 milestone Jun 3, 2015
@brian7997
Copy link

I am trying to create an instance for my plugin however the unique ID doesn't seem to do anything, my required plugins are over-written by the theme.

Should I rename the class and functions?

@jrfnl
Copy link
Contributor

jrfnl commented Jan 14, 2016

@brian7997 The id is only for the notices. Please check the TGMPA page again. The required plugins shouldn't be overwritten, but merged with those from the theme, so they should display as expected on the TGMPA page.
Do not rename the class and functions, you will break things, quite apart from the fact that the intention of TGMPA is to display one page displaying dependencies from all, not a different page for each plugin/theme using TGMPA which would only become very confusing and user-_un_friendly to the end-user.

@brian7997
Copy link

I did get it working with the theme. However Avada, a very popular theme hijacks TGMPA which makes it seem like my required plugins are for their theme :( For this reason I think it would be good to have an option to have required/recommends separated.

I was able to change the names in the functions and classes without breaking TGMPA and sort of disagree this is bad UI, since now I have a submenu relevant to my plugin (not in the theme options menu) that lists the plugins required for my plugin, instead of having it hijacked by the theme. In theory, I guess it is bad UI, but in the real world it makes sense. IF it was a perfect world where it wasn't misused by themes and more plugins started using TGMPA, then having it merged would make more sense for sure.

BTW, I really appreciate TGMPA and I hope I don't come off ungrateful. A big thanks to the developers and community.

@jrfnl
Copy link
Contributor

jrfnl commented Jan 15, 2016

@brian7997 You probably would have been better off using the filters which are build in to TGMPA or setting a lower prio for the action where you register your plugins and config.

You've now broken the upgrade path (and a new version is expected) and we will no longer offer you any support - as we can't support customized versions -.

@brian7997
Copy link

@jrfnl I didn't know there was filters that could be used. Is there an example or documentation? I rather not remove the ability to upgrade if I can.

@jrfnl
Copy link
Contributor

jrfnl commented Jan 15, 2016

@brian7997 Not at this moment, though some of the filters & actions are documented inline in the code.

To have your configuration take priority over the configuration provided by Avada, you can just change the prio of where you hook into TGMPA with your own function:

add_action( 'tgmpa_register', 'my_theme_register_required_plugins', 15 );

@reidbiztech
Copy link

reidbiztech commented Oct 7, 2016

I agree with brian7997. TGMPA needs to be specific to the plugin or theme that includes it. I have set the action priority to PHP_INT_MAX, yet the plugins for the theme (which includes TGMPA) on the site I am working on appear on my plugin's instance/id of TGMPA. And the resulting notice has my plugin name, yet lists the theme dependencies. It is not usable in its current state for me.

So I too will have to rename all of TGMPA's classes, functions, globals, constants, etc. in order to be able to use it.

If you insist on making it so that only a single page is intended to be used for all plugins and themes, then a different solution is in order. There needs to be some way to distinguish which plugins/themes have which dependencies. Also, if this is how it is to be used, the menu page should be separate from the theme or plugin that includes TGMPA and should not be configurable. A top level menu item like: "Plugin Dependencies", and in the WP_List_Class that lists the plugins, the theme or plugin that is requesting it should be a column. That would be good UX.

Please reconsider your position on this, thanks.

@jrfnl
Copy link
Contributor

jrfnl commented Oct 7, 2016

@reidbiztech The v2 branch will keep the current logic, but the v3 branch is intended to be far more closely integrated with WP core and address a number of the gripes you have, including UX.

For more information on the direction we're looking to take it in (slightly outdated), have a look/read here:
http://wordpress.tv/2016/07/05/juliette-reinders-folmer-managing-dependencies-the-wordpress-way/
#447
#394

@reidbiztech
Copy link

Thanks for the update! Glad to hear it. No time to watch the video now, I might check it out later, thanks.

@gfirem
Copy link

gfirem commented Dec 17, 2017

I have the same issue in my end. I use the library in multiples plugins of my ecosystem, but if one fail with his dependency it show the name of other with other dependency. How i can address this?

@baden03
Copy link

baden03 commented Nov 26, 2018

@brian7997 Not at this moment, though some of the filters & actions are documented inline in the code.

To have your configuration take priority over the configuration provided by Avada, you can just change the prio of where you hook into TGMPA with your own function:

add_action( 'tgmpa_register', 'my_theme_register_required_plugins', 15 );

After some testing, I can verify that this does NOT un-hijack what Avada is doing. If Avada is installed it will replace the entire TGMPA interface with Avada's own avada-install-plugins interface regardless of the priority set in the tgmpa_register action of a plugin. Both the parent_slug config option and, by extension, the url are hijacked by Avada.

@jrfnl
Copy link
Contributor

jrfnl commented Nov 26, 2018

@baden03 Thanks for adding that information. The original issue is from 2014, so the Avada theme has probably changed quite a bit since then and they may now be actively trying to overrule TGMPA,

@XedinUnknown
Copy link

It appears that every plugin has to bundle its own copy. TGMPA is not namespaced, but uses if ( class_exists() ) everywhere. Furthermore, the tgmpa() function explicitly uses the global instance of the main TGMPA class to register the plugins and config. While the plugin registration could work more or less, keeping in mind the above it is not clear whether/how it could support multiple config() calls from multiple plugins. And yet, the docs contain this line for the config structure:

'id'           => 'tgmpa',                 // Unique ID for hashing notices for multiple instances of TGMPA.

Which kinda seems to imply that there can be multiple instances (presumably for different plugins), but the recommended approach does not seem to allow that.

@jrfnl
Copy link
Contributor

jrfnl commented Dec 4, 2018

@XedinUnknown The id bit is only for the admin notifications.

TGMPA was designed to be allowed to be included by both a theme as well as plugins and to collect all plugin requirements and recommendations from these on one page.

Multiple instances is not supported as that would decrease the user-friendliness dramatically - imagine a user having to wade through three or more different TGMPA plugin installation pages... not good at all.

@XedinUnknown
Copy link

Thanks, @jrfnl, I understand the usability concerns. However, that does not answer the questions:

  1. Why does the documentation of the id configuration parameter imply that multiple instances are supported? Maybe this should be removed?
  2. How do multiple plugins configure TGMPA for themselves, invoking config() multiple times with different values? Because this is what is realistically going to happen when multiple plugins/themes use TGMPA in the way it is described in the docs.

@jrfnl
Copy link
Contributor

jrfnl commented Dec 4, 2018

Why does the documentation of the id configuration parameter imply that multiple instances are supported? Maybe this should be removed?

Or improved ? PRs for the documentation can be made against the gh-pages branch, though the docs actually do state quite clearly:

This is used to prevent admin notices not showing up when there are several themes/plugins using TGMPA and the admin notice has been dismissed before.


How do multiple plugins configure TGMPA for themselves, invoking config() multiple times with different values? Because this is what is realistically going to happen when multiple plugins/themes use TGMPA in the way it is described in the docs.

This is a known issue - see #666 but more than that #394.
Currently the config array will be merged and the order is determined by the hook-in, i.e. the order in which plugins/themes have registed their add_action( 'tgmpa_register', 'my_theme_register_required_plugins' ); + the hook priority.

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

No branches or pull requests

10 participants