Make WordPress Core

Opened 13 years ago

Closed 8 years ago

#15000 closed enhancement (fixed)

add_meta_box() should accept array of post types

Reported by: coffee2code's profile coffee2code Owned by: drewapicture's profile DrewAPicture
Milestone: 4.4 Priority: normal
Severity: minor Version: 3.0.1
Component: Administration Keywords: good-first-bug has-patch
Focuses: Cc:

Description

I often find myself defining an identical meta box for different post types. It would be nice if add_meta_box() accepted an array as its $page argument. For backward-compatibility the function will still accept a string. So for all else being equal, the same metabox could be added to multiple post types in a single line of code. Of course, if the meta box needs per-post_type configuration, add_meta_box() could be called separately for each post type.

Example:

add_meta_box( 'my-geo', 'Geolocation Info', array( &$this, 'geo_form' ), 'post', 'side' );
add_meta_box( 'my-geo', 'Geolocation Info', array( &$this, 'geo_form' ), 'page', 'side' );
add_meta_box( 'my-geo', 'Geolocation Info', array( &$this, 'geo_form' ), 'event', 'side' );

would become:

add_meta_box( 'my-geo', 'Geolocation Info', array( &$this, 'geo_form' ), array( 'post', 'page', 'event' ), 'side' );

Of course, the former would continue to work.

One could certainly loop through the post types manually to call add_meta_box(), but I think the proposed patch clarifies coding intention.

Patch is attached. (In the patch, I didn't indent the content of the newly introduced foreach() ... endforeach; since it would look like the whole function got changed when in fact I only made a very minor change. The function also has a precendent of a non-indented foreach. However, I would anticipate the actual commit might want the indentation, which can be provided.)

(Note: affects, or affected by, #13937)

Attachments (12)

15000.diff (1.6 KB) - added by coffee2code 13 years ago.
Aforementioned patch.
15000.2.diff (4.1 KB) - added by iamfriendly 9 years ago.
Refreshed patch. Now accepts an array of post types or an array of screen objects
15000.3.diff (6.0 KB) - added by egill 9 years ago.
15000.4.diff (3.4 KB) - added by egill 9 years ago.
15000.5.diff (4.1 KB) - added by iamfriendly 9 years ago.
Updated patch with changes to remove_meta_box() as well as add_meta_box()
15000.6.diff (2.6 KB) - added by madalin.ungureanu 9 years ago.
Added the same functionality for remove_meta_box() function and made some modiffications in the description of the functions
15000-tests.diff (1.3 KB) - added by mordauk 9 years ago.
15000-tests.diff.2 (3.3 KB) - added by igmoweb 9 years ago.
Unit tests included
1500-tests.3.diff (3.0 KB) - added by igmoweb 9 years ago.
Unit tets diff extension file fixed
15000.6-refresh.diff (3.1 KB) - added by meloniq 8 years ago.
Refreshed last one patch (6) to current version of WP (34082). Also moved the Screen ID check out of the loop.
15000.6-refresh-2.diff (3.2 KB) - added by meloniq 8 years ago.
Refreshed last one patch (6) to current version of WP (rev. 34900). Also moved the Screen ID check out of the loop.
15000.7.diff (6.5 KB) - added by DrewAPicture 8 years ago.
Combine tests + update docs

Download all attachments as: .zip

Change History (41)

@coffee2code
13 years ago

Aforementioned patch.

#1 @nacin
13 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #13305 which was wontfix'd.

#2 @coffee2code
13 years ago

A roundabout solution presented there, but so it goes. A straightforward foreach would be more concise:

foreach ( array( 'post', 'page', 'event' ) as $page )
    add_meta_box( 'my-geo', 'Geolocation Info', array( &$this, 'geo_form' ), $page, 'side' );

#3 @hakre
13 years ago

+1 for wontfix, iterating should be something a developer handles on it's own normally.

This ticket was mentioned in IRC in #wordpress-dev by tw2113. View the logs.


9 years ago

#5 @ericlewis
9 years ago

  • Keywords has-patch removed
  • Resolution duplicate deleted
  • Status changed from closed to reopened

I think we should do this. Passing arrays of post types is common practice in other APIs.

Think

WP_Query( array( 'post_type' => array( 'post', 'page' ) ) );

which also supports

WP_Query( array( 'post_type' => 'post' ) );

#6 @ericlewis
9 years ago

  • Keywords good-first-bug added

#7 @ericlewis
9 years ago

  • Keywords needs-patch added

Patch needs refresh

#8 @SergeyBiryukov
9 years ago

  • Milestone set to Awaiting Review

A workaround from comment:7:ticket:13305 that does not require a foreach loop:

function your_callback( $post_type ) {
	if ( in_array( $post_type,  array( 'post', 'page' ) ) ) {
		add_meta_box( 'your-id', 'Title', 'your_box_callback', $post_type );
	}
}
add_action( 'add_meta_boxes', 'your_callback' );
Last edited 9 years ago by SergeyBiryukov (previous) (diff)

@iamfriendly
9 years ago

Refreshed patch. Now accepts an array of post types or an array of screen objects

#9 @iamfriendly
9 years ago

  • Keywords has-patch added; needs-patch removed

Refreshed patch. Now accepts an array of post types or an array of screen objects. Also updated the docblock to reflect that it can accept an array.

@egill
9 years ago

@egill
9 years ago

#10 @egill
9 years ago

Personally not a big fan of circular references, would prefer something like:
https://core.trac.wordpress.org/attachment/ticket/15000/15000.3.diff
or even a simple wrapper function for the original function:
https://core.trac.wordpress.org/attachment/ticket/15000/15000.4.diff

Just my $.02 :)

p.s. just a suggestion, haven't tested the patches thoroughly.

Last edited 9 years ago by egill (previous) (diff)

#11 @nacin
9 years ago

I don't mind the circular reference. However, we also need to think about remove_meta_box(), too.

If you are allowed to pass something to add_meta_box() that actually splits into multiple meta boxes due to individual contexts, then remove is going to end up needing to be called for each individual context. Or, it would need to do the same circular reference.

I just think this complicates things. As to whether that outweighs the syntactical sugar, I'm not sure.

@iamfriendly
9 years ago

Updated patch with changes to remove_meta_box() as well as add_meta_box()

#12 @iamfriendly
9 years ago

I understand what you mean in terms of pros vs cons of this approach, @nacin.

Personally, I don't mind how this is implemented (with circular references or otherwise), but I think the ease of adding/removing metaboxes from multiple screens at once is useful and, it seems to fall in line with what we do elsewhere.

I've updated the patch with additions to remove_meta_box() (as well as slightly tweaking the docs)

Last edited 9 years ago by iamfriendly (previous) (diff)

#13 @DrewAPicture
9 years ago

  • Owner set to iamfriendly
  • Status changed from reopened to assigned

#14 @nacin
9 years ago

  • Keywords needs-patch needs-unit-tests added; has-patch removed

No need to change the spaces in the @param lines — those are accurate as is.

Rather than the ! isset return at the end, I'd just return after the foreach.

We'd also have to make a similar change for remove_meta_box() too. Ideally, we'll add some basic unit tests as well, to tests/phpunit/tests/admin/includesTemplate.php.

@madalin.ungureanu
9 years ago

Added the same functionality for remove_meta_box() function and made some modiffications in the description of the functions

#15 @madalin.ungureanu
9 years ago

  • Keywords has-patch added; needs-patch removed

#16 @DrewAPicture
9 years ago

  • Owner changed from iamfriendly to madalin.ungureanu

@mordauk
9 years ago

#17 @mordauk
9 years ago

15000-tests.diff includes some basic tests for add/remove_meta_box()

@igmoweb
9 years ago

Unit tests included

#18 @igmoweb
9 years ago

I've included a couple of tests more for this ticket based on @mordauk ones.

I improved the test_remove_meta_box. When we use remove_meta_box, $wp_meta_boxes['post']['advance']['default']['metabox1'] was set to false but the key 'metabox1' was still present.

I splitted the tests so now we have tests for add_meta_box when $screen cannot be an array and when $screen can be an array of post types.

Sorry for the two EOLs in includesScreen.php as I started to make the tests there by mistake.

@igmoweb
9 years ago

Unit tets diff extension file fixed

#19 @igmoweb
9 years ago

Forget about 15000-tests.diff.2​, the extension was wrong, my fault. I included 1500-tests.3.diff

#20 @jared_smith
9 years ago

  • Keywords needs-unit-tests removed

This ticket has unit tests, so I'm removing the "needs-unit-tests" keyword.

@meloniq
8 years ago

Refreshed last one patch (6) to current version of WP (34082). Also moved the Screen ID check out of the loop.

#21 @wonderboymusic
8 years ago

  • Milestone changed from Awaiting Review to 4.3
  • Resolution set to fixed
  • Status changed from assigned to closed

#22 @wonderboymusic
8 years ago

  • Milestone changed from 4.3 to Awaiting Review
  • Resolution fixed deleted
  • Status changed from closed to reopened

oops

#23 @iamfriendly
8 years ago

Would love this getting another pair of eyes so it could be included in 4.4

#24 @DrewAPicture
8 years ago

  • Keywords needs-refresh added

Patch needs to be refreshed.

@meloniq
8 years ago

Refreshed last one patch (6) to current version of WP (rev. 34900). Also moved the Screen ID check out of the loop.

#25 @meloniq
8 years ago

  • Keywords needs-refresh removed

Refreshed the last one patch as the functions has moved location to template-functions.php

#26 @DrewAPicture
8 years ago

  • Owner changed from madalin.ungureanu to DrewAPicture
  • Status changed from reopened to accepted

#27 @DrewAPicture
8 years ago

  • Milestone changed from Awaiting Review to 4.4

@DrewAPicture
8 years ago

Combine tests + update docs

#28 @DrewAPicture
8 years ago

In 34951:

Administration: Add the ability to pass an array of screen IDs to add_meta_box() and remove_meta_box().

The $screen parameter in both functions can now accept a single screen ID, WP_Screen object, or an array of screen IDs.

Adds tests.

Props coffee2code, iamfriendly, madalinungureanu, mordauk, igmoweb, meloniq, DrewAPicture.
See #15000.

#29 @DrewAPicture
8 years ago

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

In 34952:

Docs: Normalize documentation spacing in the DocBlocks for add_meta_box() and remove_meta_box() following [34951].

Fixes #15000.

Note: See TracTickets for help on using tickets.