Make WordPress Core

Opened 8 years ago

Closed 6 years ago

#37526 closed enhancement (maybelater)

Introduce the possibility to register new administration panels

Reported by: flixos90's profile flixos90 Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Administration Keywords: 2nd-opinion has-patch needs-unit-tests
Focuses: multisite Cc:

Description

I'm currently trying to build a plugin that implements a completely custom administration panel. Unfortunately it's currently not possible to fully make it work with the Core implementation. Therefore my proposal is to introduce a function like register_administration_panel( $name, $args ) that developers can use to build their own custom admin panel and have Core support it. They would need to take care of the functionality for that panel themselves - the registration would only ensure that the parts that are unified for all administration panels in Core will act in a compatible manner.

WordPress Core supports two additional administration panels on Multisite ('network' and 'user') which could leverage that function as well. The idea is that everywhere we currently check for is_network_admin() and is_user_admin(), we'd now use the administration panels registered to detect where we currently are. This would provide a flexible API and also clean up some code. The default admin would not be checked for there, instead it will be the fallback as it currently is in Core implementation as well.

Note that registering a custom administration panel should be considered an edge-case. The reason I personally would appreciate that functionality for is that I'm building a plugin that implements a global administration panel (for Multinetwork). Note that while the examples I mentioned are all Multisite-related, the function shouldn't be restricted to Multisite. The average plugin won't and shouldn't use it, but the function can be useful for custom projects which require an interface that other plugins should not interact with at all, for example a member area of a membership site with custom functionality.

Introducing this functionality would also make #37445 and #37446 unnecessary (two tickets I opened earlier).

I will add a patch as a proof-of-concept of what this could look like.

Attachments (5)

37526.diff (12.8 KB) - added by flixos90 8 years ago.
37526.2.diff (13.0 KB) - added by flixos90 8 years ago.
37526.3.diff (20.4 KB) - added by flixos90 8 years ago.
37526.4.diff (21.3 KB) - added by flixos90 8 years ago.
37526.5.diff (20.7 KB) - added by flixos90 8 years ago.

Download all attachments as: .zip

Change History (15)

@flixos90
8 years ago

#1 @flixos90
8 years ago

37526.diff is a first take on what this functionality could look like.

  • register_administration_panel( $name, $args ) and related getter functions are introduced in wp-admin/includes/misc.php
  • the 'network' and 'user' administration panels are registered using that function (in wp-admin/admin.php; plugins can use it on admin_init)
  • areas that perform general checks on which administration panel is currently active now use the new functionality (not sure if I covered it all yet though); affected by this:
    • the menu.php file which is included (in wp-admin/admin.php)
    • the private and public hooks run to allow modification of the admin menu (in wp-admin/includes/menu.php)
    • the admin title tag content and the hook for admin notices (in wp-admin/admin-header.php)
    • the process of setting the $in_admin property of WP_Screen (in wp-admin/includes/class-wp-screen.php)
    • the function that self_admin_url() uses to return the current admin URL (in wp-includes/link-template.php)

One problem yet to be solved is the definition of WP_BLOG_ADMIN in the beginning of wp-admin/admin.php: at this point the administration panels are not registered yet, so we can't properly detect that. I'm not sure whether this is a real problem since the constant is only used in is_blog_admin() (and even there, only before the current screen is set). Maybe we can ignore the constant in some way and modify is_blog_admin() since the blog admin is the default admin anyway. The registered administration panels could be checked, and if none of them is active, assume we're in the blog admin.

@flixos90
8 years ago

#2 @flixos90
8 years ago

I just noticed a severe problem in the first patch: It reversed the order of the admin_init and admin_menu hooks which obviously we shouldn't do. 37526.2.diff fixes this:

  • at the point of setting up the menu, the administration panels need to be registered already; this makes it impossible to use admin_init for it, so I introduced a new action admin_setup which runs a little earlier
  • plugins must call register_administration_panel() from that new admin_setup hook

The rest of the patch is similar to the first one, so the before comment still applies to everything else.

#3 follow-up: @ocean90
8 years ago

Just thinking out loud, instead of introducing a new global, can't this be part of WP_Screen?

#4 in reply to: ↑ 3 @flixos90
8 years ago

Replying to ocean90:

Just thinking out loud, instead of introducing a new global, can't this be part of WP_Screen?

You mean as a static property? I'd definitely prefer to not use a global, but I'm not sure it fits into WP_Screen. The screen class holding the different administration panels would kind of be the wrong way around I think, as administration panels rather contain the screens.

We might wanna introduce a new class WP_Administration_Panel - then we wouldn't need to use plain objects for the administration panels (they could be instances of that class), and the class could also contain the administration panels existing in a static property (to get rid of the global). Even the methods to register and get panels could be moved into that class as static methods. What do you think?

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


8 years ago

@flixos90
8 years ago

#6 @flixos90
8 years ago

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

In 37526.3.diff I introduce a new class WP_Administration_Panel and also a new file for Administration Panel API functions. The panel class handles its own instances through a static property and methods.

@flixos90
8 years ago

#7 @flixos90
8 years ago

37526.4.diff brings 4 improvements over the previous patch:

  • only define WP_BLOG_ADMIN as true in wp-admin/admin.php if it hasn't been defined before (and therefore define it as false in wp-admin/network/admin.php and wp-admin/user/admin.php)
  • only register the network and user administration panels if we're on a multisite
  • merge two almost similar checks in WP_Administration_Panel::register() into one
  • remove an incorrect private doc annotation for action hook in wp-admin/includes/menu.php (accidentally introduced in the previous patch)

@flixos90
8 years ago

#8 @flixos90
8 years ago

37526.5.diff improves the get_current_administration_panel() method so that it no longer needs to support a parameter. The is_*_admin() functions already take care of checking whether to use the current screen or the constant for detection.

Also the constant_name property is not used anymore. However I think it should remain part of the class since every administration panel should have its own constant to signalize that it's active - just to make this more clear to the user, I would leave it in. But I'm open for discussion.

Anyway, does anyone have feedback on this whole thing? :)

#9 @flixos90
8 years ago

I just noticed kind of a blocker for this one. :(

The $pagenow global is set in wp-includes/vars.php, so the code in there would need to be adjusted to support custom administration panels as well. At first I thought the action admin_setup could be moved in there, but the file is loaded before all regular plugins.

Maybe we can find a way to adjust the way this file is handled, also considering that it is rather messy. Outsourcing parts of it into real functions would be a good first step.

#10 @flixos90
6 years ago

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

This is an edge-case issue and involves tons of changes, so let's not think about it further at this point.

Note: See TracTickets for help on using tickets.