Make WordPress Core

Opened 8 months ago

Closed 8 months ago

#59587 closed enhancement (worksforme)

Pass column count to "after_plugin_row" action hooks in WP_Plugins_List_Table

Reported by: tobiasbg's profile TobiasBg Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Administration Keywords:
Focuses: Cc:

Description

A site's list of plugins on /wp-admin/plugins.php is generated by the WP_Plugins_List_Table class.

This generates an HTML table with one row for each plugin.
The number of columns in that table can vary, depending on a user's capabilities, singlesite/multisite, etc.
The number of columns is available via the inherited get_column_count() method, and it's e.g. used when Core shows an update notice in a row with a colspan (where the colspan has to match the number of columns, to not break the HTML code).

The WP_Plugins_List_Table also offers actions hooks after_plugin_row and after_plugin_row_{$plugin_file} that run after each printed table row, commonly used by plugins to also insert extra rows into the HTML table. (As the hooks run in between the rendering of an HTML table, inserting a new row is pretty much the only possible output that such an action can do.)

However, plugins then face the challenge that they do not know the number of visible columns in the table, as they can not access the get_column_count() method from within the action hook function, as they don't have access to the WP_Plugins_List_table object.

Therefore, it would be reasonable to pass the get_column_count() value as an extra parameter to the after_plugin_row and after_plugin_row_{$plugin_file} action hooks.

Attachments (2)

59587.patch (4.5 KB) - added by TobiasBg 8 months ago.
59587-example.png (157.8 KB) - added by TobiasBg 8 months ago.

Download all attachments as: .zip

Change History (8)

@TobiasBg
8 months ago

#1 @afragen
8 months ago

  • Keywords reporter-feedback dev-feedback added

@TobiasBg I'm a little unclear as to why this is needed. Is there an existing code example? Images of what you're looking to achieve would be great.

#2 @TobiasBg
8 months ago

  • Keywords reporter-feedback removed

@afragen: Sure, I'll try to explain a bit more!

Basically, the after_plugin_row hook can only really be used to echo another <tr> in the WP_Plugins_List_Table (the action hook runs inside the <tbody>, after each plugin's <tr>).

For adding that <tr>, the action hook handler needs to know the number of visible columns (no matter if it prints a <td> for each column or if it prints one <td colspan=...> (probably the more common case)).

I noticed this when trying to replicate something like from the attached screenshot 59587-example.png (which shows that the code that adds that red notice likely uses a hard-coded number for that colspan -- resulting in a row that is too "short").

Some code for this could be

add_action( 'after_plugin_row', 'my_custom_plugin_row', 10, 3 );
function my_custom_plugin_row( $plugin_file, $plugin_data, $status ) {
	echo
		'<tr class="plugin-update-tr">' .
		'<td colspan="4" class="plugin-update colspanchange">' .
		'<div class="update-message notice inline notice-error notice-alt"><p>' .
		'Your custom message goes here.' .
		'</p></div></td></tr>';
}

Note how the colspan is hard-coded to 4, as the function can't know the correct value, which depends on things like the user's capabilities, etc.

#3 @afragen
8 months ago

I'm really not sure how you are not getting the full width of the plugin row when attempting to add your license renewal. I have plugins that use the after_plugin_row_{plugin_name} and do not suffer this. Perhaps it's just CSS that needs to be used with your row addition.

You can also use the wp_list_table_class_name hook to essentially replace the function in question or the entire WP_Plugin_List_Table class.

In either case core does not need to be modified. Here's a snippet of code that I use for the opening of the plugin row.

	$wp_list_table = _get_list_table( 'WP_Plugins_List_Table' );

	$open = '<tr class="git-updater plugin-update-tr' . $active . '"' . $multisite_theme_open . '>
		<td colspan="' . $wp_list_table->get_column_count() . '" class="plugin-update colspanchange">
		<div class="">';

As you can see from the snippet above, I am able to directly access the column width.

#4 @TobiasBg
8 months ago

Thanks for sticking with me, @afragen!

As these are <tr> rows of a table, their width is governed by the number of cells (each row needs to have the same number of cells/colspans). This has nothing to do with CSS.

I thought about your approach as well, but _get_list_table( 'WP_Plugins_List_Table' ) will create a whole new instance of the WP_Plugins_List_Table class, which doesn't really feel performant at all.
In fact, it appears that your code only does this to retrieve that column count, so it would benefit form the suggested addition as well (unless you are using the $wp_list_table for something else later).

Also, creating a custom child class of WP_Plugins_List_Table for this and using that instead of Core's class (on Core's Plugins screen -- not a screen of a plugin!) won't be reasonable, for the introduced overhead. (And this will actually fail as soon as more than one plugin does this.)

The suggested addition of the (already determined) column count as a parameter to those action hooks will however greatly enhance and simplify how they can be used.

#5 @afragen
8 months ago

I'll let others chime in but you have 2 workable solutions that do not require any modifications to existing hooks and the potential cost of ongoing support.

If you're worried about potential performance issues with my suggestions, my advice is to demonstrate that performance issue.

Cheers.

#6 @TobiasBg
8 months ago

  • Keywords has-patch dev-feedback removed
  • Milestone 6.5 deleted
  • Resolution set to worksforme
  • Status changed from new to closed

@afragen, thanks for your persistence! Your suggestion will in fact work nicely, with just a small modification.
There's no need to reinstantiate WP_Plugins_List_Table, but we can simply reuse the existing global variable that is already available:
So, instead of

$wp_list_table = _get_list_table( 'WP_Plugins_List_Table' );

this line will do:

global $wp_list_table;

This does not suffer from the duplicated class instantiation and the involved computational cost but allows retrieving that column count number from inside the action hook handler -- so no need for passing that as a parameter anymore :-)

I'll resolve this as "worksforme", as we found a suitable solution (which even is an optimization for your plugin). Thanks for working on this with me!

Note: See TracTickets for help on using tickets.