Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#41235 closed enhancement (wontfix)

New load_screen_** action

Reported by: aussieguy123's profile aussieguy123 Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.9
Component: General Keywords:
Focuses: Cc:

Description

WordPress includes a load-<page> hook, however this is not specific enough to have a function that runs only on one single screen in the admin area without additional logic. Lets say I have a custom post type "killed_attachment" and I want to have a function which will only run on the edit posts screen for this post type.

My proposed solution:

<?php
/**
 * Fire load_screen_* hooks
 * @action admin_init (or a better action, suggestions?)
 */
 public function run_load_screen_hooks() {
        $current_screen = function_exists( 'get_current_screen' ) ? get_current_screen()->id : null;
        do_action('load_screen_' . $current_screen);
 }

With this new action, instead of requiring logic to detect the screen as in:

<?php
        /**
         * @action admin_init
         */
        public function run_on_killed_attachments_edit_post_only() {
                $current_screen = function_exists( 'get_current_screen' ) ? get_current_screen()->id : null;

                switch ( $current_screen ) {
                        case 'edit-killed_attachment':
                                //Code which will only run on the killed attachment edit posts screen 
                                break;
                        default:
                                break;
                }
     }

You could simplify this as:

<?php
        /**
         * @action load_screen_edit-killed_attachment
         */
        public function admin_enqueue_scripts() {
            //Code which will only run on the killed attachment edit posts screen 
        }

Basically both functions do the same thing but one is 9 lines long and contains extra logic, the other is one line long.

Shorter functions and less logic means less bugs and more readable + maintainable code.

Change History (6)

#1 @westonruter
7 years ago

  • Keywords reporter-feedback added

@aussieguy123 thanks for the ticket. There is actually a current_screen action that currently triggers. It seems like you're essentially suggesting a current_screen_{$screen_id} action?

#2 @aussieguy123
7 years ago

Yes. I saw the existing current_screen hook. However that would also need extra logic in code hooked into it to have code run on a specific screen. So this is a new load_screen_{$screen_id} action

Perhaps the code to set up this new action could be hooked into the existing current_screen action?

Something like:

<?php
add_action( 'current_screen', function() {
       $current_screen = function_exists( 'get_current_screen' ) ? get_current_screen()->id : null;
       do_action('load_screen_' . $current_screen);
});
Last edited 7 years ago by aussieguy123 (previous) (diff)

#3 @aussieguy123
7 years ago

  • Keywords reporter-feedback removed

#4 @DrewAPicture
7 years ago

Looking at your examples, I'm not really sure what the advantage to a new dynamic hook would be other than making what is already pretty straightforward with current_screen slightly more specific.

As pointed out, the existing current_screen hook could be used for what you want, and the screen object is passed to it:

<?php
add_action( 'current_screen', function( $screen ) {
    if ( isset( $screen->id ) && 'killed_attachment' === $screen->id ) {
        // Do stuff.
    }
} );

#5 @aussieguy123
7 years ago

@DrewAPicture the idea of the action is to avoid the need for the if statement (or other logic)

This is similar to how alot of MVW style frameworks work, instead of using an if statement to detect a page they would use something else (like a route or action) to hook into the page directly.

With the screen passed to current_screen, the code to set up this new action could be as simple as:

<?php
add_action( 'current_screen', function( $screen ) {
   do_action( 'load_screen_'.$screen->id );
} );

These 3 lines would save alot of if statement lines

Version 5, edited 7 years ago by aussieguy123 (previous) (next) (diff)

#6 @DrewAPicture
7 years ago

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

@aussieguy123 I understand your use case – I'm saying that don't think that reason alone is adequate justification for adding a new dynamic hook in this context.

Thank for you the suggestion and your willingness to make your case, however, I'm going to close this ticket was wontfix. If a greater need for such a hook should arise in the future, we can certainly reopen this ticket to revisit it.

Last edited 7 years ago by DrewAPicture (previous) (diff)
Note: See TracTickets for help on using tickets.