WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#13229 closed enhancement (fixed)

Search Custom Post Type in Media Library

Reported by: lucaskeiser Owned by:
Milestone: 3.0 Priority: normal
Severity: normal Version: 3.0
Component: Posts, Post Types Keywords: has-patch
Focuses: Cc:

Description

When attempting to attach a media item to a post type, the search box only allows for posts or pages to be searched for and not custom post types.

Attachments (4)

13229.txt (5.1 KB) - added by markoheijnen 5 years ago.
Patch 13229
box2.png (12.4 KB) - added by markoheijnen 5 years ago.
New box
13229-2.diff (5.6 KB) - added by markoheijnen 5 years ago.
use function get_post_types. Changed post type var media_add to attachment. Also add comment line for the attachment var for post type. Still need to look for futher implementation.
13229-3.diff (3.7 KB) - added by markoheijnen 5 years ago.
Build it up again with the latest revision. Removed the parameter. I still think it should be there but that is my opinion. The reason of the parameter is to disable it when an plugin developer want to. @t31os_ Thanks for the labels array hint. Didn't thouht about that

Download all attachments as: .zip

Change History (17)

comment:1 @markoheijnen5 years ago

  • Cc markoheijnen added

I did look at the code and was wondering why there are hard checks on if it is an page or an post.

Files that need to be change
/wp-admin/includes/template.php find_posts_div line 3287 - 3290
/wp-admin/js/media.js line 34-38
/wp-admin/admin-ajax.php line 1256

I think the solution is to remove all the hard checks and work with an filter.
Add an variable or an supports option to register_post_type.
Also the message 'No posts found.' should change so posts will be the name of the post_type.
At admin-ajax you check if it allowed to do the action.

comment:2 @nacin5 years ago

  • Component changed from Administration to Post Types
  • Milestone changed from Unassigned to 3.0

@markoheijnen5 years ago

Patch 13229

@markoheijnen5 years ago

New box

comment:3 @markoheijnen5 years ago

  • Keywords has-patch added

Patch is ready. Do need to be reviewed. Not happy about my solution at template.php

comment:4 @nacin5 years ago

We have get_post_types. Maybe an attachments (supports?) arg?

@markoheijnen5 years ago

use function get_post_types. Changed post type var media_add to attachment. Also add comment line for the attachment var for post type. Still need to look for futher implementation.

comment:5 @ryan5 years ago

__('No '. $what .' found.')

__() must be passed literal strings. Concatenation is not allowed. I'd leave this line as 'No posts found' for 3.0.

id="find-posts-<?php echo $post->name; ?>"

We should esc_attr() just to be safe.

patch: **** malformed patch at line 37: Index: wp-admin/admin-ajax.php

Patch doesn't apply.

comment:6 @ryan5 years ago

Although adding attachment as an attribute to the type means you can use the get_post_types() filters, I'm wondering it this should be in the supports array instead.

comment:7 @markoheijnen5 years ago

ryan. I will create an new patch. I still need to look how the add attachment works.
I think when it isn't supported it shouldn't add the image to the media library.

How I did it now it maybe should. This because it is now standard false. In case of standard true then I would say it shouldn't be in support.

comment:8 @t31os_5 years ago

Shouldn't this be pulling the post type object to ascertain what the relevant labels are? Like has been done in edit.php for dealing with the various post type labels.

Post types can register these labels already, so it's a matter of updating the media files (amongst other areas) to be updated inline with the method used in edit.php, use the relevant post type's label(s) array to determine what should be displayed.

Example registered post type with appropriate labels registered, follows the same style as those set for posts (have left out other parameters, this is for illustration only).

register_post_type( 'book', array(
	'labels' => array(
		'name' => 'Books',
		'singular_name' => 'Book',
		'add_new' => 'Add New',
		'add_new_item' => 'Add New Book',
		'edit_item' => 'Edit book',
		'edit' => 'Edit',
		'new_item' => 'New Book',
		'view_item' => 'View Book',
		'search_items' => 'Search Books',
		'not_found' => 'No books found',
		'not_found_in_trash' => 'No books found in Trash',
		'view' => 'View Book'
	)
) );

Additionally i don't think you need that extra "attachment" parameter, if you leave the existing radio buttons in place and do a seperate foreach over get_post_types like so, you'll not need the new parameter..

				<input type="radio" name="find-posts-what" id="find-posts-posts" checked="checked" value="posts" />
				<label for="find-posts-posts"><?php _e( 'Posts' ); ?></label>
				<input type="radio" name="find-posts-what" id="find-posts-pages" value="pages" />
				<label for="find-posts-pages"><?php _e( 'Pages' ); ?></label>
			<?php
			$selected = 'checked="checked" ';
			$post_types = get_post_types( array( '_builtin' => false ) , 'objects' );
			
			foreach ($post_types AS $post) {
				if( post_type_supports( $post->name , 'editor' ) ) {
				?>
				<input type="radio" name="find-posts-what" id="find-posts-<?php echo $post->name; ?>" value="<?php echo $post->name; ?>" <?php echo $selected?>/>
				<label for="find-posts-<?php echo $post->name; ?>"><?php echo $post->label; ?></label>
				<?php 
				$selected = '';
				}
			} 
			?>

Again for illustration, the above should probably fetch the labels from the post type object, but i borrowed the code from the patch so i could be quick and give a simple illustration (i had the code there already).

You guys replied whilst i was looking at this. I also had to apply the patch by hand, not working. If the post type supports the editor, it supports attachments right? I figured it seemed like the obvious choice for above..

Didn't look at the JS, but i'm sure it's trivial compared with the other suggested changes.

@markoheijnen5 years ago

Build it up again with the latest revision. Removed the parameter. I still think it should be there but that is my opinion. The reason of the parameter is to disable it when an plugin developer want to. @t31os_ Thanks for the labels array hint. Didn't thouht about that

comment:9 @ryan5 years ago

I don't think you need to check _builtin, just public = true and then skip over attachment in the loop. That way the special casing for post and page can be removed. Using the not_found label is good. I forgot that was added. In situations where we'd have to create a one-off label, I wouldn't bother. not_found fits this situation though.

comment:10 @markoheijnen5 years ago

Without _builtin you see Media (I think that is wrong). First I did only show_ui and that did do the trick. But I wasn't sure about if that is correct.

comment:11 @ryan5 years ago

Media is the attachment post type. That's why I suggested tossing it out during the loop.

comment:12 @ryan5 years ago

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

(In [14869]) Allow searching custom post types when attaching media. Props markoheijnen. fixes #13229

comment:13 @ryan5 years ago

markoheijnen, I massaged the patch a little bit. Double check my work.

Note: See TracTickets for help on using tickets.