WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 months ago

#15000 closed enhancement (fixed)

add_meta_box() should accept array of post types

Reported by: coffee2code Owned by: 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 5 years ago.
Aforementioned patch.
15000.2.diff (4.1 KB) - added by iamfriendly 20 months 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 20 months ago.
15000.4.diff (3.4 KB) - added by egill 20 months ago.
15000.5.diff (4.1 KB) - added by iamfriendly 20 months 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 17 months 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 17 months ago.
15000-tests.diff.2 (3.3 KB) - added by igmoweb 16 months ago.
Unit tests included
1500-tests.3.diff (3.0 KB) - added by igmoweb 16 months ago.
Unit tets diff extension file fixed
15000.6-refresh.diff (3.1 KB) - added by meloniq 5 months 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 4 months 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 4 months ago.
Combine tests + update docs

Download all attachments as: .zip

Change History (41)

@coffee2code
5 years ago

Aforementioned patch.

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


20 months ago

#5 @ericlewis
20 months 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
20 months ago

  • Keywords good-first-bug added

#7 @ericlewis
20 months ago

  • Keywords needs-patch added

Patch needs refresh

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

@iamfriendly
20 months ago

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

#9 @iamfriendly
20 months 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
20 months ago

@egill
20 months ago

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

#11 @nacin
20 months 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
20 months ago

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

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

#13 @DrewAPicture
17 months ago

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

#14 @nacin
17 months 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
17 months ago

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

#15 @madalin.ungureanu
17 months ago

  • Keywords has-patch added; needs-patch removed

#16 @DrewAPicture
17 months ago

  • Owner changed from iamfriendly to madalin.ungureanu

#17 @mordauk
17 months ago

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

@igmoweb
16 months ago

Unit tests included

#18 @igmoweb
16 months 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
16 months ago

Unit tets diff extension file fixed

#19 @igmoweb
16 months ago

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

#20 @jared_smith
16 months ago

  • Keywords needs-unit-tests removed

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

@meloniq
5 months ago

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

#21 @wonderboymusic
5 months ago

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

#22 @wonderboymusic
5 months ago

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

oops

#23 @iamfriendly
4 months ago

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

#24 @DrewAPicture
4 months ago

  • Keywords needs-refresh added

Patch needs to be refreshed.

@meloniq
4 months 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
4 months ago

  • Keywords needs-refresh removed

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

#26 @DrewAPicture
4 months ago

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

#27 @DrewAPicture
4 months ago

  • Milestone changed from Awaiting Review to 4.4

@DrewAPicture
4 months ago

Combine tests + update docs

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