farinspace / wpalchemy

Thin framework for wordpress
http://wpalchemy.com/
Other
416 stars 110 forks source link

Resolving possible conflicts with multiple includes/instantiating #95

Open vladan-me opened 10 years ago

vladan-me commented 10 years ago

I'd like to help with this issue and I probably need little guidance, to tell me if my approach is valid. I'll start with explaining issue and then proposing my idea how to resolve it.

Issue that can appear, I believe, is already noted in @todo lines 13-14

// todo: perhaps move _global_head and _global_foot locally, when first run
// define a constant to prevent other instances from running again ...

So, problem that may arise is that multiple includes of this file would have multiple actions added. Also, there will be notices about trying to redefine already defined constants.

First, I believe that those actions

add_action('admin_head', array('WPAlchemy_MetaBox', '_global_head'));
add_action('admin_footer', array('WPAlchemy_MetaBox', '_global_foot'));

should be within class itself, in constructor of it to be precise, once class is instantiated.

Second, there could be option (in very start of constructor) to skip adding those actions within instantiating of class, something like this (there's probably a better way, this is just an example):

if (!isset($arr['skip_admin_head']) || $arr['skip_admin_head'] == false) {
    add_action('admin_head', array($this, '_global_head'));
}

Third, can those constants that are defined outside of class be class constants? I don't think there's a reason to be outside of it but maybe I am missing something?

Thanks

helgatheviking commented 8 years ago

:+1: I'm not sure I see the purpose of _gloabl_head and _global_foot at all when the wp_enqueue_scripts hook exists and enqueing a small script would ensure that it isn't loaded multiple times. But they could at least be in the constructor.

And those constants can definitely be class constants/variables!

dommermuth commented 8 years ago

I had a problem with the admin_head being written multiple times. My hack to solve this on some legacy code was this:

At approximately line 462 in MetaBox.php I changed this:

add_action( 'admin_head', array( $this, '_global_head' ) );

to this:

if(RUNONCE != 'TRUE'){ define('RUNONCE', 'TRUE'); add_action( 'admin_head', array( $this, '_global_head' ) ); }

Not a graceful solution, but does what I need for now. Thank you Helgatheviking and farinspace for your attention to wp alchemy. I've used it often over the years...

rnovotny commented 8 years ago

+1 - for https://github.com/davidhme/easy-pricing-tables-free for some reason this _global_head is getting called multiple times. This attaches multiple event handlers to the 'add' and 'delete' buttons, so when you click it will add 5 boxes, and when you delete you get 5 confirmations. I mean why is there inline js at all, use wp_enqueue_script.

rnovotny commented 8 years ago

I made a commit that seems to be a better of doing this:

https://github.com/farinspace/wpalchemy/pull/109