Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question about is_admin #585

Open
xoex opened this issue Feb 28, 2023 · 3 comments
Open

Question about is_admin #585

xoex opened this issue Feb 28, 2023 · 3 comments

Comments

@xoex
Copy link

xoex commented Feb 28, 2023

I was looking at the code, asked myself, isn't it better to wrap admin hooks in is_admin() ?
for example in this code :

private function define_admin_hooks() {

		$plugin_admin = new Ghorekeshi_Admin( $this->get_plugin_name(), $this->get_version() );

		$this->loader->add_action( 'admin_enqueue_scripts', $plugin_admin, 'enqueue_styles' );
		$this->loader->add_action( 'admin_enqueue_scripts', $plugin_admin, 'enqueue_scripts' );
	}

If wrap inside code in a if (is_admin()) {} so we don't run administrative hooks when non-admin pages and posts are loaded?

@JadeResources
Copy link

The hook admin_enqueue_scripts will only run on admin anyway so the is_admin() is a pointless check. The hook enqueue_scripts does the opposite and will only run on the front end.

@xoex
Copy link
Author

xoex commented Mar 4, 2023

yes it's true, but a lot of unused action and hooks can be added, what if we change this part of code :

	public function __construct() {
		if ( defined( 'PLUGIN_NAME_VERSION' ) ) {
			$this->version = PLUGIN_NAME_VERSION;
		} else {
			$this->version = '1.0.0';
		}
		$this->plugin_name = 'plugin-name';

		$this->load_dependencies();
		$this->set_locale();
		$this->define_admin_hooks();
		$this->define_public_hooks();
	}

to this :

	public function __construct() {
		if ( defined( 'PLUGIN_NAME_VERSION' ) ) {
			$this->version = PLUGIN_NAME_VERSION;
		} else {
			$this->version = '1.0.0';
		}
		$this->plugin_name = 'plugin-name';

		$this->load_dependencies();
		$this->set_locale();
		if (is_admin()) {
			$this->define_admin_hooks();
		}
		$this->define_public_hooks();
	}

So we don't run create a new instance of admin class and we don't add non necessary admin hooks and filters to public pages.

@gerardreches
Copy link

Most actions and filters that you will use inside define_admin_hooks() will or should only be executed when in the admin dashboard. There could even be some use cases in which you don't want to check is_admin(), such as a custom admin action that is triggered through a form on the frontend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants