Make WordPress Core

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#21240 closed enhancement (fixed)

Ability to "Turn On" Native custom columns for registered custom taxonomies

Reported by: jtsternberg's profile jtsternberg Owned by: nacin's profile nacin
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords: has-patch commit
Focuses: Cc:

Description

I believe it would be very beneficial to have an argument for displaying custom columns for registered custom taxonomies (register_taxonomy). Attached is a class to get it started for hierarchical taxonomies. Would need tag-like taxonomy support added.

Attachments (11)

custom-taxonomy-columns.php (1.8 KB) - added by jtsternberg 12 years ago.
tax-columns.png (41.5 KB) - added by jtsternberg 12 years ago.
This is what the output would look like in theory.
21240.diff (7.2 KB) - added by jtsternberg 12 years ago.
Incorporating 'show_taxonomy_columns' in the register_post_type function as well as setting up the columns
21240.2.diff (9.0 KB) - added by jtsternberg 12 years ago.
Ok I've incorporated tags and categories into the new API and moved the column header array creation to WP_Post_List_Table
21240.3.diff (8.4 KB) - added by jtsternberg 12 years ago.
One more diff where I add the 'show_taxonomy_columns' taxonomies to an object variable for access in the other methods
21240.4.diff (8.7 KB) - added by jtsternberg 12 years ago.
Add filter to allow plugins/themes to modify/add/remove taxonomy columns
21240.5.diff (6.3 KB) - added by SergeyBiryukov 12 years ago.
21240.6.diff (6.3 KB) - added by SergeyBiryukov 12 years ago.
21240.7.diff (4.8 KB) - added by nacin 12 years ago.
21240.8.diff (5.2 KB) - added by nacin 12 years ago.
21240.9.patch (591 bytes) - added by ocean90 12 years ago.

Download all attachments as: .zip

Change History (52)

#1 follow-up: @scribu
12 years ago

We talked about this in IRC. The idea is to make it easy for custom taxonomies to generate columns on CPT WP_List_Table screens, just like the Categories and Tags columns on wp-admin/edit.php

Here's my proposal:

register_taxonomy( 'presentation', 'presentation_type', array(
  ...
  'show_admin_column' => true
) );

This would create a 'Presentations' column in WP Admin -> Presentations.

And the implementation (as a plugin): https://gist.github.com/3098902

Last edited 12 years ago by scribu (previous) (diff)

@jtsternberg
12 years ago

This is what the output would look like in theory.

#2 in reply to: ↑ 1 @jtsternberg
12 years ago

Here's my proposal:

register_taxonomy( 'presentation', 'presentation_type', array(
  ...
  'show_admin_column' => true
) );

This would create a 'Presentations' column in WP Admin -> Presentations.

And the implementation (as a plugin): https://gist.github.com/3098902

From your estimation, how close is this to being complete? What still needs work?

#3 @scribu
12 years ago

Well, if you have the same taxonomy registered for multiple post types, you can't control on which screens it shows up.

Maybe besides

'show_admin_column' => true

we could also have

'show_admin_column' => array( 'post', 'movie' ) )

which would make it so it shows up only on the post and movie tables.

#4 follow-up: @jtsternberg
12 years ago

I think I may be partial to adding the flag to the register_post_type parameters instead of register_taxonomy. Normal workflow is to add custom taxonomies TO a custom post type rather than the other way around I think. It may be symantics, but what about:

register_post_type( 'movies', array(
  ...
  'show_taxonomy_columns' => array( 'actors', 'locations' ) )
) );

#5 @scribu
12 years ago

My thinking was that you need to register the post type before the taxonomy, but that isn't really a problem, since the column rendering is done much later.

#6 in reply to: ↑ 4 @wpsmith
12 years ago

  • Cc travis@… added

+1 to adding it to CPTs. To me it's more logical workflow and since the screen is a CPT screen. However, it would be even better if it didn't matter.

Replying to jtsternberg:

I think I may be partial to adding the flag to the register_post_type parameters instead of register_taxonomy. Normal workflow is to add custom taxonomies TO a custom post type rather than the other way around I think. It may be symantics, but what about:

register_post_type( 'movies', array(
  ...
  'show_taxonomy_columns' => array( 'actors', 'locations' ) )
) );

#7 @scribu
12 years ago

You bring up a good point. If we store it on the CPT, then it won't work if you a have a user taxonomy.

#8 @jtsternberg
12 years ago

So should it have a separate complementary function for flexibility? something like

register_taxonomy_columns( $taxonomies, $object_types, $args );

#9 follow-up: @scribu
12 years ago

21240.diff looks like a step in the right direction.

  1. Any particular reason for making get_taxonomy_column_headers() a stand-alone function?
  2. What is the purpose of the 'taxonomy_column_header' filter?

@jtsternberg
12 years ago

Incorporating 'show_taxonomy_columns' in the register_post_type function as well as setting up the columns

#10 in reply to: ↑ 9 @jtsternberg
12 years ago

Replying to scribu:

21240.diff looks like a step in the right direction.

  1. Any particular reason for making get_taxonomy_column_headers() a stand-alone function?
  2. What is the purpose of the 'taxonomy_column_header' filter?
  1. vs rolling it into the get_column_headers() function? No particular reason, just thought it might be cleaner.
  2. The filter will allow you to rename the taxonomy column headers if you want. As is, the Taxonomy column headers will be the $taxonomy_object->label, but with this filter could be changed to anything you wanted. Maybe the the $taxonomy_object->label in
    $empty = !empty( $empty ) ? $empty : __( 'No '. $taxonomy_object->label ); 
    

should have that same filter applied.

#11 follow-up: @scribu
12 years ago

  • Keywords has-patch added

vs rolling it into the get_column_headers() function?

No, vs making it a protected method.

__( 'No '. $taxonomy_object->label )

You can only pass plain strings to __().

But even doing:

sprintf( __( 'No %s' ), $taxonomy_object->label )

won't be good enough. We'll probably have to introduce $taxonomy_object->labels->no_terms.

PS: How about handling the built-in 'categories' and 'tags' columns using this new API?

Last edited 12 years ago by scribu (previous) (diff)

#12 in reply to: ↑ 11 @jtsternberg
12 years ago

Replying to scribu:

vs rolling it into the get_column_headers() function?

No, vs making it a protected method.

Do you envision this being incorporated into class WP_List_Table then?

We'll probably have to introduce $taxonomy_object->labels->no_terms.

I like that idea.

PS: How about handling the built-in 'categories' and 'tags' columns using this new API?

will do.

#13 @scribu
12 years ago

Do you envision this being incorporated into class WP_List_Table then?

Into WP_Post_List_Table for now, but keeping it as a function works too.

@jtsternberg
12 years ago

Ok I've incorporated tags and categories into the new API and moved the column header array creation to WP_Post_List_Table

@jtsternberg
12 years ago

One more diff where I add the 'show_taxonomy_columns' taxonomies to an object variable for access in the other methods

#14 @jtsternberg
12 years ago

In 21240.4.dff I added

$this->taxonomy_columns = apply_filters( 'show_taxonomy_columns', $taxonomies, $post_type );

This line allows plugins and themes to filter the taxonomy columns. i.e.

add_filter( 'show_taxonomy_columns', 'add_resource_category_column', 10, 2 );
function add_resource_category_column( $tax_columns, $post_type ) {
	if ( $post_type == 'resources' ) $tax_columns[] = 'resource-categories';
	return $tax_columns;
}

This allows taxonomies registered after the fact to add their columns to post-types.

Last edited 12 years ago by jtsternberg (previous) (diff)

@jtsternberg
12 years ago

Add filter to allow plugins/themes to modify/add/remove taxonomy columns

#15 @SergeyBiryukov
12 years ago

21240.5.diff is another take.

  • As far as I can see, there's no need for no_items label to be based on menu_name (this wouldn't be localizable). It can fall back to "No Tags" / "No Categories", like the other labels.
  • We should probably keep the existing $posts_columns keys and CSS classes for categories and tags for backwards compatibility.
  • A separate taxonomy_column() method doesn't seem necessary. We could just combine the cases in single_row().
  • I think the taxonomy columns should be added before the comments and date columns.
  • $posts_columns filters should be applied after the taxonomy columns are added, not before.
  • __( $taxonomy_object->label ) is not localizable.
  • taxonomy_column_header filter doesn't seem necessary, we can add it later if needed.
  • taxonomy_column_args filter doesn't seem necessary as well.
  • Some minor edits as per the coding standards.

#16 @scribu
12 years ago

In 21240.5.diff, aren't these lines redundant:

 	46	                'labels' => array( 
 	47	                        'no_items' => __( 'Uncategorized' ), 
 	48	                ), 

?

#17 @scribu
12 years ago

Also, I'm not sure these two lines have the intended effect:

$taxonomies = array_diff( $taxonomies, array( 'category', 'post_tag' ) );
$this->taxonomy_columns = array_merge( array( 'categories', 'tags' ), $taxonomies ); 

You always end up with 'categories' and 'tags', even though they might not be desired.

#18 @scribu
12 years ago

My suggestion would be to transform the taxonomy names into the desired column names later, right before rendering.

#19 @SergeyBiryukov
12 years ago

Replying to scribu:

In 21240.5.diff, aren't these lines redundant:

Currently the category column says "Uncategorized" if there's no categories:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-admin/includes/class-wp-posts-list-table.php#L613

But it could say "No Categories" as well. Removed from 21240.6.diff for consistency.

You always end up with 'categories' and 'tags', even though they might not be desired.

Thanks, fixed in 21240.6.diff.

Not sure if those empty( $post_type ) checks in lines 274 and 279 (introduced in [12702]) are still needed. With WP_Screen, is it possible to have an empty $post_type there?

My suggestion would be to transform the taxonomy names into the desired column names later, right before rendering.

My concern was that a plugin or custom code might rely on categories or tags array key when hooking to the manage_posts_columns filter.

#20 @jtsternberg
12 years ago

Any word on this ticket? Would LOVE to see it in 3.5!

#21 @scribu
12 years ago

  • Type changed from feature request to enhancement
case ( in_array( $column_name, $this->taxonomy_columns ) ) : 

That is absolutely wicked! :D

Instead of this:

$custom_taxonomies = array_diff( $taxonomies, array( 'category', 'post_tag' ) ); 
$this->taxonomy_columns = array_merge( $this->taxonomy_columns, $custom_taxonomies ); 

Why not just set 'show_taxonomy_columns' => array( 'category', 'post_tag' ) when registering the 'post' post type?

Last edited 12 years ago by scribu (previous) (diff)

#22 @scribu
12 years ago

That would also require unsetting the 'categories' and 'tags' columns, which I think would be good, to further narrow the divide between these two native taxonomies and custom ones.

#23 @jtsternberg
12 years ago

Anything I can do to help move this along?

@nacin
12 years ago

#24 follow-up: @nacin
12 years ago

I've looked at this a number of times but wasn't a diehard fan of the current approach. Couldn't figure out why exactly. Today in IRC I started to offer a review of the patch. There are some smaller things pointed out there like column and taxonomy names conflicting, overly clever code, etc.

I don't think that tying this directly to register_post_type() is particularly useful. First, this kind of modification is just as likely to be done by a plugin adding a taxonomy or otherwise, and not by the plugin registering the post type. Second, it'd probably make more sense for it to be controlled by register_taxonomy(), as that's what is being shown, and they are specifically being registered against a post type anyway. It also makes for really complicated code.

People are used to registering a custom column with one filter (which is very simple to do). Then, they are used to registering a callback against an action to handle output of the column's contents, row by row. Not much harder to do, but it's annoying when all you want is a taxonomy column. You have to replicate the 'tags' code from the list table. That's no fun, and that's really what (I think) this ticket is trying to get at.

So, 21240.7.diff.

It simply abstracts out the code used to render both 'categories' and 'tags' columns. For compatibility with the column management filters, it keeps the names for both of those columns, translating them back and forth as necessary with 'category' and 'post_tag'.

For the other taxonomies, a 'taxonomy-{$taxonomy}' column is automatically registered based on the taxonomy names returned from a new manage_taxonomies_for_{$post_type}_columns filter. If the post type supports categories or tags, those are automatically passed to the filter as a starting point.

The only other thing I would possibly add here is a show_column_on_post_screens-like argument (that name is terrible, I know) to register_taxonomy(). If true, then that taxonomy will be a column by default (passed to the filter the same way categories and tags are now), for all post types to which it is registered.

Last edited 12 years ago by nacin (previous) (diff)

@nacin
12 years ago

#25 in reply to: ↑ 24 @nacin
12 years ago

Replying to nacin:

The only other thing I would possibly add here is a show_column_on_post_screens-like argument (that name is terrible, I know) to register_taxonomy(). If true, then that taxonomy will be a column by default (passed to the filter the same way categories and tags are now), for all post types to which it is registered.

Added in 21240.8.diff.

#26 follow-up: @nacin
12 years ago

Also, my patches change "No Tags" to a simple em-dash. It avoids introducing a new string for the taxonomy object and, I think, cleans up the screen a bit.

For categories, it still says "Uncategorized" — eventually, we'll remove the one-category requirement and maybe rip that out.

#27 @nacin
12 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [21788]:

Allow easy registration of taxonomy columns on post (and custom post type) list table screens.

To register a column for a list table, use the new manage_taxonomies_for_{$post_type}_columns
filter. Introduces show_admin_column => true for register_taxonomy(), which automatically
displays that column on all associated post types.

props jtsternberg, SergeyBiryukov for initial patches.
fixes #21240.

#28 @nacin
12 years ago

  • Milestone changed from Awaiting Review to 3.5

@ocean90
12 years ago

#29 in reply to: ↑ 26 ; follow-up: @ocean90
12 years ago

  • Keywords 2nd-opinion removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to nacin:

Also, my patches change "No Tags" to a simple em-dash. It avoids introducing a new string for the taxonomy object and, I think, cleans up the screen a bit.

For categories, it still says "Uncategorized" — eventually, we'll remove the one-category requirement and maybe rip that out.

I think we can remove the "Uncategorized" part. Currently a post will always have the default category (#14901, #11576). So get_the_terms( $post->ID, 'category') will always return a term. And because it's possible to rename the default category it's not always "Uncategorized".

And if get_the_terms( $post->ID, 'category')` returns nothing, then somebody probably has removed the default category successfully and we should respect it.

21240.9.patch does address this.

#30 @alexvorn2
12 years ago

What about the option to sort columns alphabeticaly, by name, or by number?

#31 @scribu
12 years ago

If you mean in WP Admin -> Posts, no, we're not going to do that.

If you mean in WP Admin -> Categories, it's already possible.

#32 in reply to: ↑ 29 @scribu
12 years ago

  • Keywords commit added

+1 for ocean90's reasoning and patch.

#33 @toscho
12 years ago

  • Cc info@… added

#34 @navjotjsingh
12 years ago

  • Cc navjotjsingh@… added

#35 @nacin
12 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [21806]:

Don't show 'Uncategorized' in the category column on edit.php if there are no categories (including the default one). props ocean90. fixes #21240.

#36 @Bueltge
12 years ago

  • Cc frank@… added

great idea/solution. thanks

#37 @tott
12 years ago

The changed behavior in putting previous category and post-tag cases into the default section in r21788 breaks manage_*posts_custom_column hooks if you also want to include custom taxonomies via these hooks as the $taxonomy value is not reset.
The following change fixes this.

Index: wp-admin/includes/class-wp-posts-list-table.php
===================================================================
--- wp-admin/includes/class-wp-posts-list-table.php     (revision 21921)
+++ wp-admin/includes/class-wp-posts-list-table.php     (working copy)
@@ -629,6 +629,7 @@
                        break;
 
                        default:
+                               $taxonomy = null;
                                if ( 'categories' == $column_name )
                                        $taxonomy = 'category';
                                elseif ( 'tags' == $column_name )

#38 @nacin
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#39 @nacin
11 years ago

Thanks tott — I had coded this not realizing it was a foreach loop going through the columns, but rather a function call for each column.

#40 @nacin
11 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [21945]:

Reset $taxonomy in single_row's default branch. props tott. fixes #21240.

#41 @nacin
11 years ago

In 23081:

Media list table: Fix the categories column, hidden thanks to a typo. props eddiemoya, fixes #22764. see #21240, #21391.

Note: See TracTickets for help on using tickets.