Make WordPress Core

Opened 14 years ago

Closed 9 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 14 years ago.
Aforementioned patch.
15000.2.diff (4.1 KB) - added by iamfriendly 10 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 10 years ago.
15000.4.diff (3.4 KB) - added by egill 10 years ago.
15000.5.diff (4.1 KB) - added by iamfriendly 10 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 10 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 10 years ago.
15000-tests.diff.2 (3.3 KB) - added by igmoweb 10 years ago.
Unit tests included
1500-tests.3.diff (3.0 KB) - added by igmoweb 10 years ago.
Unit tets diff extension file fixed
15000.6-refresh.diff (3.1 KB) - added by meloniq 9 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 9 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 9 years ago.
Combine tests + update docs

Download all attachments as: .zip

Change History (41)

@coffee2code
14 years ago

Aforementioned patch.

#1 @nacin
14 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
14 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
14 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.


11 years ago

#5 @ericlewis
11 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
11 years ago

  • Keywords good-first-bug added

#7 @ericlewis
11 years ago

  • Keywords needs-patch added

Patch needs refresh

#8 @SergeyBiryukov
11 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 11 years ago by SergeyBiryukov (previous) (diff)

@iamfriendly
10 years ago

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

#9 @iamfriendly
10 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
10 years ago

@egill
10 years ago

#10 @egill
10 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 10 years ago by egill (previous) (diff)

#11 @nacin
10 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
10 years ago

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

#12 @iamfriendly
10 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 10 years ago by iamfriendly (previous) (diff)

#13 @DrewAPicture
10 years ago

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

#14 @nacin
10 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
10 years ago

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

#15 @madalin.ungureanu
10 years ago

  • Keywords has-patch added; needs-patch removed

#16 @DrewAPicture
10 years ago

  • Owner changed from iamfriendly to madalin.ungureanu

@mordauk
10 years ago

#17 @mordauk
10 years ago

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

@igmoweb
10 years ago

Unit tests included

#18 @igmoweb
10 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
10 years ago

Unit tets diff extension file fixed

#19 @igmoweb
10 years ago

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

#20 @jared_smith
10 years ago

  • Keywords needs-unit-tests removed

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

@meloniq
9 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
9 years ago

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

#22 @wonderboymusic
9 years ago

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

oops

#23 @iamfriendly
9 years ago

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

#24 @DrewAPicture
9 years ago

  • Keywords needs-refresh added

Patch needs to be refreshed.

@meloniq
9 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
9 years ago

  • Keywords needs-refresh removed

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

#26 @DrewAPicture
9 years ago

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

#27 @DrewAPicture
9 years ago

  • Milestone changed from Awaiting Review to 4.4

@DrewAPicture
9 years ago

Combine tests + update docs

#28 @DrewAPicture
9 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
9 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.