yeswework / fabrica-dev-kit

A toolkit for faster, smoother WordPress 5 development
https://fabri.ca/
MIT License
274 stars 27 forks source link

Create "admin_menu" #10

Closed maxperei closed 7 years ago

maxperei commented 7 years ago

hi there, i've got difficulties while displaying a function inside a new admin menu here is my code in project.php :

public function __construct() {
        add_action('admin_menu', array($this, 'custom_settings_menu'));
}
public function custom_settings_menu() {
        add_menu_page('Highlight', 'Highlight', 'manage_options', 'highlight_posts', 'highlight_settings_page');
}
public function highlight_settings_page() { /* ... */ }

the new entry is ok but it returns this warning : Warning: call_user_func_array() expects parameter 1 to be a valid callback, function 'highlight_settings_page' not found or invalid function name in /var/www/html/wp-includes/class-wp-hook.php on line 298

what am i supposed to to ? i just want to create a form in the backoffice like i used to to in the functions.php with add_menu_page

thanks

maxperei commented 7 years ago

i should write :

public function __construct() {
        add_action('admin_menu', array($this, 'custom_settings_menu'));
}
public function custom_settings_menu() {
        add_menu_page('Highlight', 'Highlight', 'manage_options', 'highlight_posts', array($this, 'highlight_settings_page'));
}
public function highlight_settings_page() { /* ... */ }

sorry guys for the inconvenience but sometimes just saying it can help to resolve ^^

andrewstaffell commented 7 years ago

Hey @maxperei, glad you were able to figure that out!

We recommend that you put hooks which only affect the backoffice/wp-admin in dev/src/includes/admin.php. Hooks in project.php will be executed by front-end requests as well, which is no big deal in the case of add_menu_page(), but it's redundant so may as well be avoided for efficiency.

In this case, the hook should probably go in admin.php after the conditional return on line 22 (if (wp_doing_ajax()) { return; }) because it doesn't need to run for AJAX calls either.

If this structural aspect wasn't clear from the documentation and you think there's a way we could make it clearer, let us know.

Thanks for trying FDK and happy to hear any further feedback.

maxperei commented 7 years ago

hi andrew, again thanks for your time, i really wanted to work in a better "wordpress environment" and FDK nailed it 👌 you're completely right, and the doc is pretty clear about that, so i'll change it right away — anyway, there are a few things that i'm not comfortable with : i like the way the project is organized with gulp and webpack but when i wanted to enqueue_scripts in admin.php i had to add/change some code in gulpfile.js and maybe i went wrong by doing this ; i mean it works now but i feel not very satisfied about that however i have no regrets by trying this out and for sure you've got my full support ! i think i'll probably have other questions like this one, and i'll try to open not too much shameless issues

andrewstaffell commented 7 years ago

@maxperei it doesn't sound right to me that you've had to modify gulpfile.js just to get enqueue_scripts to work. Do you want to explain exactly what you were trying to do here including modifications, or email me at info@fabri.ca and I'll take a look and see if there's an easier solution.

maxperei commented 7 years ago

what i'm trying to do is to load separately from main.js two scripts (chosen.js and admin.js) intended to be executed only in the backoffice, so here my code in admin.php :

public function __construct() 
{
    add_action('admin_enqueue_scripts', array($this, 'custom_settings_page_scripts'));
}

public function custom_settings_page_scripts()
{
        wp_enqueue_script('chosen', get_stylesheet_directory_uri().'/js/chosen.jquery.min.js', array('jquery'), '1.6.1', true);
        wp_enqueue_script('admin', get_stylesheet_directory_uri() . '/js/admin.js', array('jquery', 'chosen'), '1.0', true);
}

but as i previously said i had to modify gulpfile.js because function scripts() concatenate all scripts into one file thanks to webpack, i missed something for sure...