While I'm usually against including CSS as an internal style sheet, I think that due to the nature of the library file we're creating, it's best if we do that.

1. The style sheet is small (although made twice as big by the license blurb) - the benefit gained from having it cached is negligible.
2. Since this is for the back-end, and front-end performance is not affected, what negligible impact there is is fine to have.
3. The impact is offset by the reduction of a HTTP request (even to get a 304 Not Modified header).
4. The main benefit, is that it's one less file that the developer has to copy into his project, set a $config['css_path'] for (or not, if they cope and paste the styles into the existing theme's admin style sheet), or ensure that his structure matches our default path to from the theme root to the style sheet file.

What we want is a single library file that can be dropped into any part of the theme hierarchy, referenced by a require that the developer can set, and not have any dependencies.

I suggest moving the styles out admin-css.css into the styles() method, and hook this to admin_print_styles instead of admin_enqueue_scripts.

As per http://wpcandy.com/reports/the-tgm-plugin-activation-class-on-github#comment-212334 consider a force_activation boolean plugin argument that automatically activates a registered plugin IF IT IS ALREADY INSTALLED.

If it is not installed, then do nothing. I wouldn't be at all happy if themes could start grabbing plugins from any source and activating without human user interaction, but if the plugin has already been added by other means, and still present, then I think it should be satisfactory to activate it - again, it's a lot of power for theme developers registering plugins, but nothing they couldn't do in about 5 lines of code directly within functions.php anyway.

$args and $instance are not self-documenting - arguments for what? An instance of what?

Better would be $plugins and $plugin respectively, so then you know it's data for multiple plugins, or a single plugin.

If believe Plugin_Upgrader will handle all the WP_Error objects for us, so I don't think the is_wp_error( $upgrader ) section is necessary. I need to check and make sure, but I'm pretty sure we can remove it because Plugin_Upgrader will catch everything.

Include @Package, @SInCE and @author tags on the class-level DocBlock.

Remove the word "new" from the long description, since at some point in the future, it won't be new any more.

Currently, not all strings include a textdomain as the second argument.

An API method could also be added that could update the $domain property from outside of the class, so that a developer could change it to match the theme text domain.

Strings should not be split up. Instead of:

$message = sprintf( __( 'Sorry, but you do not have the correct permissions to install the <strong>' . $args['plugin_name'] . '</strong> plugin. Contact the administrator of this site for help on getting the plugin installed.', $this->domain ) ); $output = printf( '<div id="tgm-plugin-activation" class="updated"><p>%1$s</p></div>', $message );

I would suggest:

$message = sprintf( __( 'Sorry, but you do not have the correct permissions to install the <strong>%s</strong> plugin. Contact the administrator of this site for help on getting the plugin installed.', $this->domain ), $args['plugin_name'] ); $output = printf( '<div id="tgm-plugin-activation" class="updated"><p>%1$s</p></div>', $message );

See #3 regarding changing the argument names, #8 for using add_settings_error() and #13 for property names.

The nag message for installing the plugin still appears (even when the plugin is installed) for multisite users without the correct permissions.

The plugin installation/activation messages and the links following the installation/activation can use some doctoring up and streamlining. Specifically being able to add in the "Activate this plugin" link if the plugin (for some reason) does not automatically activate.

Instead of getting developers to amend the current $args array property to add to the list of required plugins, create a register() method as an API that would allow themes to add to a protected $plugins array property (effectively $args, renamed).

As developers would be required to do something like:

TGM_Plugin_Activation::instance->register( $my_plugin );

it might be nice to create a helper wrapper function (outside of the class):

    TGM_Plugin_Activation::instance->register( $plugin_args );
}

So they can use the slightly simpler (easier to remember):

$my_plugin = array ( 'name' => '...', 'destination' => '...', 'source' => '...' ); $my_plugin2 = array ( 'name' => '...', 'destination' => '...', 'source' => '...' ); tgmpa_register_plugin( $my_plugin ); tgmpa_register_plugin( $my_plugin2 );

You might even create another helper function tgmpa_register_plugins() which can take an array of plugin arrays.

What we're working towards is not having the developers needing to touch anything inside your code at all.

Allow the registrations of plugins to specify a minimum version number of the plugin, via an optional version argument.

If the argument is present, and the installed and activated plugin does not meet that minimum version, we show an admin message suggesting they update (or a slightly different message if they don't have permissions to do so).

If the argument is not present, we continue as we currently do.

Current nag message is all bold, only the plugin name and hyperlink should be bold.

Currently:

$source = get_stylesheet_directory() . '/lib/tgm-plugin-activation/plugins/' . $instance['zip_file'];

A theme may want to store it's plugins elsewhere in it's heirarchy. I'd suggest doing the equivalent of:

$source = $instance['source'];
(see #4 for the array key name change)
as that allows them to place both the auto-install file, and the plugins folder wherever they wanted.

So that a check can be done to see if all of the required plugins are already installed and active (i.e., nothing for the user to do), the (current) 3 add_action() calls should be moved into a new init method, which is hooked within the constructor, to 'init' action.
At this point, is_plugin_active() and other functions should be available, that aren't available when the constructor is being parsed.

Simpler and fewer is better.

We already know this is going to be for plugins, so no need for "plugin_" prefixes.

I'd suggest changing:

• the current plugin to destination (there may be a way to work around this, and grab this data programatically, such that it's not needed to be specified at all).
• plugin_name to name
• zip_file and repo_file to source - we can check internally if source is a URL or not as to what type of source this is.
• input_name - scrap this - we can use the name to create a unique string to work out which submit button was pressed.

Ideally, that leaves just the plugin name and source as the two required values. If one or both are missing, then the registration can bail out early.

Currently the thickbox for the plugins does not work on all the admin pages. It only works on the Dashboard, Appearance and Plugins main pages. There may be a script dependency for install_plugin_information that is only provided on those particular pages.

So Dave Decker has pointed out that there may be some issues with the FTP part of WP_Filesystem and potentially nonces. His host is such that he is required to enter his FTP creds, which he states works fine, but that entering them in and continuing did not work. It brought about that dreaded "Are you sure you want to do this? Please try again." message, which if you remember we saw when we didn't have the 'return true;' statement in the right spot. Ultimately it was a nonce issue. I don't know if this is the same, but it might be.

He did state that he has to enter FTP credentials every time and prefers to just manually upload everything instead, so it could just be his host. Either way, if the issue pops up more we should look into it for sure.

Consider adding an information link for plugins that are detected to have a source of the .org repo (some related re-work needed for this, as currently a missing source argument is only populated when the plugin is being installed), that pops up a thickbox of the plugin page.

Hold off until http://core.trac.wordpress.org/ticket/18289 settles down.

Have you looked at the idea of removing example.php from the repo and instead doing something like this:

1. Clone repo
2. Create configuration file, which is in the .gitignore list (either xml or json)
3. Your class looks for a configuration file sitting one directory above
4. Your file parses the xml/json and the "magic happens"

Esstentially you would take your vars @ https://github.com/thomasgriffin/TGM-Plugin-Activation/blob/master/tgm-plugin-activation/example.php#L37 and map those with an xml/json file. This would allow developers that are using your plugin to easily clone your repo into their theme and (assuming their theme is repo'd also) they could then maintain this a sub-module, easily "pulling" as updates are done.

A sample structure would look something like this:

wp-content/themes/my-awesome-theme/ (user has all their needed theme stuff here)
wp-content/themes/my-awesome-theme/plugin-config.xml
wp-content/themes/my-awesome-theme/ tgm-plugin-activation/class-tgm-plugin-activation.php

plugin-config.xml, would just contain the needed params, for plugins to be prompted.

I'm only suggesting/more like asking, because I am working on a similar architecture to build custom post types and custom taxonomies. As of know I've "thought" about the idea of using a xml/json file as like a "config" and ignore that, repo'ing the architecture separately. Here's a few gist's of my code since as of know its a private repo, I plan on making it public once I button down a few more things.

*note the files are part of a plugin.

Sample "config":
https://gist.github.com/1268607

Interface:
https://gist.github.com/1268610

sample method from abstract:
https://gist.github.com/1268620

and in your class:
https://gist.github.com/1268641

The early early alpha can be seen here: http://zanematthew.com/task/#/ note we have about (24 issues open that I'm working on).

This is a strange bug. The settings message is duplicated on the admin options (Settings) main page and subpages. Not sure where this bug is coming from, but it should be addressed before we push out to master.

Just for me personally, I've been keeping track of changes from 1.1.0 to 2.0.0. I'll be putting them in the readme today. If anything is missing, let me know so I can add it in. I think we are only a few issues away from letting out 2.0. :)

Although discussed often, there wasn't a ticket that addressed the automatic activation of a plugin after we'd installed it.

I'm tempted not to check for current_user_can( 'activate_plugins' ) when doing it - this would allow administrators to remove access to the plugins page (or that capability at least), and still allow users on a multisite to install and have activated the required plugins for that theme.

Right now the nag messages say that the plugins are required. What if theme authors simply want to suggest plugins or specify their degree of importance?

Maybe we could pass another key/value pair within the plugin array, like this:

$plugins = array(
        array(
            'plugin' => 'tgm-example-plugin/tgm-example-plugin.php', // The main plugin file (including the plugin folder)
            'name'   => 'TGM Example Plugin', // The plugin name
            'source' => get_stylesheet_directory() . '/lib/plugins/tgm-example-plugin.zip', // The plugin source
            'importance' => 'required' // The degree of importance for the plugin, defaults to 'required'

        ),
        array(
            'plugin' => 'edit-howdy/edithowdy.php',
            'name'   => 'Edit Howdy',
            'source' => 'http://downloads.wordpress.org/plugin/edit-howdy.zip',
            'importance' => 'suggested'
        ),
    );

where the importance string would simply replace the default 'required' statement. If left blank, it will default to required.

Thinking from the user's perspectives, currently the admin notices appear so long as the required or recommended plugin is not (installed and) active. Since someone might implement their own plugin that achieves the same thing as the registered plugins, we need a way for the admin user to dismiss the notices.

One suggestions is a small "Dismiss" link in the bottom right of each notice (#25 suggests there should only be one or two notices anyway), which when clicked, adds a value to an array of options stored under a singe tgmpa key in the DB - we can then check this before displaying the notices.

Alternatively, a link on the TGMPA page for each plugin (on the same line as the button) might work better, since this is then:

• per-plugin (an automatically updated theme which registers another plugin later on, would not show a notice for the new plugin if there was only a "dismissed" value in the DB for all / required & recommended plugins)
• If there are just one or two notices, showing a comma-separated list of plugins, it would be clearer which plugin notice the dismiss link would be applying to.

The menu link will still be available if there are plugins not installed or activated, so no need for any UI that un-dismisses the notices.

When installing a plugin via the WP Add New link, the action links after installation are removed - the filter is not looking for amending the links just on installations via TGMPA.

Need to setup a way for theme authors to define required/recommended plugins without showing admin notices. The plugins would just show on the Install plugins page.

No check before displaying nag messages to see if all plugins have been activated. I used the same logic as with displaying the form.

Instead of rolling your own notice markup, make use of add_settings_error() (and subsequent settings_errors() to display them) - this keeps it consistent with how WP does it, and means the localised strings are far simpler.

Did I miss the installation steps?

In the example.php file I see that it's contents should be copied and pasted into the functions.php file for my child theme but what do I do beyond that?

When I added the code from the example.php into my functions.php and copied and added the tgm-plugin-activation to the plugins file but the code I pasted kept breaking the site.

Any help is greatly appreciated!

Jason Hobbs

Following on from @ZmK's suggestion in #41, I do like the idea of having an XML file as a configuration for registering plugins and options - the structure is such that it would be relatively trivial to understand, and read from.

Without restricting where the plugin and configuration should go (together) within a theme, looking for a tgmpa.xml file in the same directory as the class file (or thinking of a way to pass in a reference to the class file), or one level above it if it doesn't exist may work sufficiently well.

Update: A JSON config file should also be considered.

In a multisite environment, it might be handy for admins to be able to give a user a full package (activate theme, have desired plugins easily installable). If a user wanted to change the theme though, the admin may want to help the inexperienced user by having the plugins specific to that theme be automatically deactivated.

If we add a force_deactivation boolean argument, we'd be giving the administrator more power - it would be up to them to use it wisely. Deactivating plugins that added CPTs, shortcodes or other content, for instance, would be bad for the user's site as the content would be temporarily lost until they manually reactivated those plugins.

Still, it would be a benefit, if say, switching from a Genesis child theme to a Woo theme, to be able to deactivate all of the Genesis-related plugins in the background.

To implement, we'd need to hook into switch_theme and check against our list of registered plugins to see if any should be forced to deactivate.

The current version should be 1.1.0, not 1.1 - it's WP guidelines to always have three numbers, even if one or two of them are zeros.

Read http://semver.org - once the functions go protected, and register() and other API methods are introduced, each change that breaks backwards-compatibility on the API should bump the major version.

Your use of wp_enqueue_style() does not currently use a version number to cache-bust. I'd strongly suggest using one.

Currently:
@return boolean, true on success, false on failure

This should be:
@return boolean True on success, false on failure

No punctuation until after the space after the type (same for @param tags), and start the description with a capital (again, same for @param, @global, @author, etc.)

Although the registration of plugins (as found in example.php) can stay the same, I think internally, the plugins property of the main class should contain an associative array of objects, rather than an indexed array of arrays.

For this, we'll need a new class (suggested as TGMPA_Plugin).

The current implementation of the strings does not allow the theme author to specify required/recommended for each plugin since only one blanket message can be applied. I still think we should have a boolean true/false for required and be able to apply it to each plugin. Maybe like this:

$required = ( true == $plugin['required'] ) ? 'required' : 'recommended';

Then that can somehow be placed into a message string that the author can still filter.

The thickbox iframe is too tall in 3.3 with the new admin bar. The height can just be reduce for the iframe to fix this issue.

$args and $instance are not self-documenting - arguments for what? An instance of what?

Better would be $plugins and $plugin respectively, so then you know it's data for multiple plugins, or a single plugin.

There's a handy function called submit_button() for creating submit buttons - make use of it instead of rolling your own markup, to keep consistent with WP.

Although unfortunate, I can forsee some theme authors requiring/recommending many plugins. Right now, each plugin outputs an admin message. If we get upwards or 4 or 5 plugins, that is a lot of clutter in the UI. We should consolidate the each of the plugin messages into at max 2 messages. One message that lists all plugins that need to be installed and one message that lists all plugins that are installed but still need to be activated.

Is there a was to include updated plugins that are self hosted or published somewhere else?

So if I update my theme that has a new version of a required plugin it also gets updated?

Hey guys, loving this class :)

This could just be a preference thing, but I feel like it might be good to have the dismissable notifications return when a theme is re-activated. At the moment, if you:

1. Activate
2. Dismiss the notice
3. De-activate
4. Then activate again

the notice doesn't re-appear.

You may have chosen to do this by design, in which case that's fine. I just expected it to come back at that point.

Change each of the class properties from using the var keyword to the private or protected (if we expect the class might be extended upon) keyword, since they shouldn't be able to be accessed from outside the class.

The WP standard for files (predominantly) containing classes would suggest that our main file should be called class-tgm_plugin_activation.php. This would make it easier for a developer to recognise exactly what the file contained when listed with a load of other theme library files - it is also more unique than "auto-install".

I'd also suggest renaming the current theme-file.php to example.php, since that's the simplest and clearest name that purports what the code in that file is for.

Instead of giving your wrapping div an id attribute, give it a class attribute (just tgmpa is sufficient, no need for -form suffix). This way, the specificity scores for the CSS selectors will be considerably lower (20/21 instead of 110/111).

Same goes for removing tgmpa-go as an ID attribute value too.

Instead of creating API methods to support the changing of the $domain (string), $menu (string), $default_path (string) and $strings (array of strings), it might be worth creating a single method, config() which takes an array of all of those values, and sets them as needed.

An alternative is to still keep these properties protected, but add in a magic __set() method that allows the property to apparently by updated publicly, but still allow some degree of validation or checking as it passes through the __set() function.

see this blog post by Mark Jaquith http://markjaquith.wordpress.com/2011/10/06/translating-wordpress-plugins-and-themes-dont-get-clever/

Although the main class can be extended, the tgmpa() function forces the registration through our main class.

Consider adding a filter inside tgmpa() which can change the name of the called class.
Developers can then add an add_filter() line, and a function which returns the name of their extended class.

The current alternative to doing this, is that a developer would need to write their own equivalent of tgmpa() which calls their own class, and reference that new function name when they register the plugins. In this case, tgmpa() isn't a complicated function, but duplication of code is bad.

Perhaps the best solution would be to allow the name of a class to be passed in via the $config array, which can be pulled out immediately (and unset from $config), and use that to register the plugins and the remaining configurations. Unfortunately, the use of the scope resolution operator(::) with a variable is not supported until PHP 5.3.0, so this is currently a no-go, unless there's a workaround.

Instead of:

_e( '<h2>Install Required Plugins</h2>', $this->domain );

where you repeat the page title (with localised markup, which isnt needed on this occasion), just do:

screen_icon( 'themes' ); ?>
<h2><?php echo esc_html( get_admin_page_title() ); ?></h2>
<?php

This makes use of the string you added as one of the arguments in the add_theme_page() call, as well as adding a screen icon.

Adding a button or a simple link to the plugin information would be excellent. Kind of like the "See version x.x details" link that WordPress gives before you upgrade a plugin. We should add this in a thickbox so users can see all the details of the plugin they are about to install. This should enhance the user experience and make them feel more at ease when installing a foreign plugin.

When building up a URL, such as:
$url = 'update.php?action=install-plugin&plugin=' . $instance['plugin'];
make use of the add_query_arg() function to ensure that it's built-up properly.