several "issues" with 2.4 release

[senlin]

Before version 2.4 the class was using add_submenu_pagethis has now been changed to add_theme_page. Although that perhaps makes a lot of sense for themes using the class, it makes no sense whatsoever for plugins using the class.

Also after successful installation of the required plugin it makes no sense (in a plugin scenario) to "return to the dashboard". For starters, the user did not come from the dashboard and I much rather have that the user returns to the Plugins page, because there is a link to my plugin's settings available.

Then I don't understand the switch to the tgmpa text domain. This class is included in themes and plugins, wouldn't it make much more sense that the theme's/plugin's translation files contain the strings of the class?

[themeblvd]

I'm not the plugin author, but just some friendly observations...

Before version 2.4 the class was using add_submenu_page this has now been changed to add_theme_page.

For theme devs, this makes it so inclusion of the class in a theme will allows it to pass the theme check plugin. I think that's the reason this was changed. I'm not sure how it could be done to appease both theme authors and and plugin authors. If add_submenu_page exists in the class, it will flag a theme submitted to wordpress.org, ThemeForest, etc, as this is not allowed.

Then I don't understand the switch to the tgmpa text domain.

It is bad practice to have any kind of PHP-executed variable as a text domain.

http://markjaquith.wordpress.com/2011/10/06/translating-wordpress-plugins-and-themes-dont-get-clever/

... Which is also something that gets flagged from theme check plugin, but should be this way, no matter if it's for plugin authors or theme authors.

[senlin]

@themeblvd thanks for your reply.
I understand that it makes sense for theme devs, but for plugin devs it does not make sense at all. It would be nice if in a next version a variable can be included to use themes.php for theme devs and plugins.php for plugin devs.

Actually the comments of the article you link to tell a different story, the text domain does not get parsed at all and can literally be anything.
I did notice however, when integrating the new example.php with my plugin and running an update with PO Edit, the strings were automatically added, so I guess that's all working fine.

[tivnet]

@senlin POEdit, as far as I know, takes all translations, regardless of the text domain. Other tools may take the text domain into account, and therefore having a variable there is not good (though very convenient).

[senlin]

@tivnet at least the paid version of POEdit does :) Anyways, the textdomain "issue" is solved as that magically works without a problem.

I guess for the other things, I'll just need to update the class and adapt it to my own wishes every time an update comes out. Hopefully an option for plugin devs can be build in...

[tivnet]

@senlin I have POEdit Pro 1.6.4, and I do not see any "textdomain" in the Catalog properties. Where did you see it?

[senlin]

@tivnet apologies, my explanation was probably not so good.
It's not with the Catalog properties.
When I integrated the new example.php into my plugin files and ran an update with POEdit on the language files, it automatically added the strings.
Previously I had set those strings explicitly to the text domain of my plugin, but now the textdomain is tgmpa and that seems to work. I was afraid that the strings of the tgmpa textdomain would no longer make it into my language files.
Hopefully it's more clear now?

[tivnet]

@senlin Yes, POEdit automatically parses all PHP files in the folder, and extracts all strings, does not matter which textdomain is specified in the __() functions.
The problem starts when you want to separate translations into the separate .po files, by textdomains.

[senlin]

@tivnet cool, I didn't know that. Learn sth new every day :)
Thanks

[thomasgriffin]

@senlin - thanks for your comments. As @themeblvd mentioned, I did this to ensure TGMPA was good to go with theme checks since it is most widely used in themes. However, I do understand where you are coming from with plugins, so I think I can definitely add something to allow that to happen.

The only drawback to this is that I can no longer use singletons and will need to make sure I manage instances properly. I'll do some testing to see if I can get it worked out so that it makes sense for both plugin and theme authors contextually.

[senlin]

@thomasgriffin - thanks for your reply. I understand that TGMPA is more used in themes than in plugins.

Before you get rid of all your coding in singletons, could it perhaps be an idea to make a separate branch for plugins?

That way these two branches (or a fork perhaps) can live next to each other and you don't have to make compromises on your code.

[thomasgriffin]

@senlin - I really wouldn't want a separate branch when basically the only difference is add_theme_page vs add_options_page. Everything else should function the same. It would make more sense to keep this in house since I don't think it would require too much refactoring. Getting away from singletons would also allow access to your unique object and address #184.

[qwertydude]

+1 for moving away from singletons!! That will mean you can identify to the user which plugin or theme is requiring the dependent plugin. And that is a really good thing. Folks get a bit toey installing things without knowing what wants it. It didn't matter when TGMPA was just themes, since there was only one theme asking.

It will also mean you'll be able to display two message boxes: One dismissable, and one that's not. (Currently, the dismiss status of the last plugin loaded wins out)

As a plugin developer, I can see TGMPA being very popular with plugin devs. I'll be using it to make Redux a dependency. And there's a couple of other plugins I'll make recommended in my plugins.

So I think it won't be uncommon in the future for a user to have 1,2,3 or more plugins using TGMPA.

[tivnet]

@qwertydude Chris, I also made an example. Here: http://wordpress.org/plugins/tivwp-dm-development-manager/
And, of course, Redux is in all my plugins and themes - via TGMPA. But, having my "wrapping", I do not need more than one instance of TGMPA.

[qwertydude]

Thanks, Gregory. I did look at that, but was little confused, so just worked with TGMPA as is.

[senlin]

It would actually be better to keep everything in the Plugins Menu, so add_plugins_page would work better than add_options_page.
Also every reference to admin_url( 'themes.php' ) would need to be replaced with admin_url( 'plugins.php' ) and a few text strings that point to the Dashboard could better point back to the Plugins page.
Sample of the adapted file I'm currently using in one of my plugins: https://github.com/senlin/so-related-posts/blob/master/inc/class-tgm-plugin-activation.php

[tivnet]

@qwertydude Of course. I just wrote and example. TGM should have a plugin version, and proper hooks for everyone to use - as a single instance. Having more than one TGM, or more than one Redux is nonsense, IMO. They should behave as a "kind of" Core. A Core "extension".

[qwertydude]

@tivnet Singleton is preferred, provided it can provide the necessary functionality to distinguish between plugins and themes and support a theme plus multiple plugins with unique configs. A repository pattern may be the way to go?

[tivnet]

In my "wrapper-around-TGM" plugin, I made an example of how a theme and a plugin can both use a single TGM, with unique configs. No conflicts. Of course, what I did is very simple, and should be "polished" - but I believe that should be done right here, where we are talking, by the TGM authors, and not by me :-)

[thomasgriffin]

I like the idea of having a core TGM component that houses all the main functionality of the class, and then having separate classes that extend the functionality for unique instances, such as for themes and plugins.

This should be something on the roadmap for v3 of TGM for sure. :-)

[jrfnl]

@senlin I have send in a PR to make the parent page for this plugin configurable - #221. Feel free to +1 if you'd find it helpful.

@thomasgriffin @qwertydude PR #219 does not move it away from singleton, but does make the class extendible which means you can overload methods for your own implementation. Hope that goes some way towards easing supporting the different uses of TGM.