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

Dramatically simplify the check whether we need to load scripts etc. #1871

Merged
merged 13 commits into from
Jan 13, 2015
111 changes: 42 additions & 69 deletions admin/class-config.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,24 +23,6 @@ class WPSEO_Admin_Pages {
*/
var $currentoption = 'wpseo';

/**
* @var array $adminpages Array of admin pages that the plugin uses.
*/
var $adminpages = array(
'wpseo_dashboard',
'wpseo_rss',
'wpseo_files',
'wpseo_permalinks',
'wpseo_internal-links',
'wpseo_import',
'wpseo_titles',
'wpseo_xml',
'wpseo_social',
'wpseo_bulk-editor',
'wpseo_licenses',
'wpseo_network_licenses',
);

/**
* Class constructor, which basically only hooks the init function on the init hook
*/
Expand All @@ -57,8 +39,6 @@ function init() {
wp_redirect( admin_url( 'admin.php?page=wpseo_dashboard' ) );
}

$this->adminpages = apply_filters( 'wpseo_admin_pages', $this->adminpages );

if ( WPSEO_Options::grant_access() ) {
add_action( 'admin_enqueue_scripts', array( $this, 'config_page_scripts' ) );
add_action( 'admin_enqueue_scripts', array( $this, 'config_page_styles' ) );
Expand Down Expand Up @@ -158,10 +138,10 @@ function admin_sidebar() {
/**
* Generates the header for admin pages
*
* @param bool $form Whether or not the form start tag should be included.
* @param string $option The long name of the option to use for the current page.
* @param string $optionshort The short name of the option to use for the current page.
* @param bool $contains_files Whether the form should allow for file uploads.
* @param bool $form Whether or not the form start tag should be included.
* @param string $option The long name of the option to use for the current page.
* @param string $optionshort The short name of the option to use for the current page.
* @param bool $contains_files Whether the form should allow for file uploads.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's up with the spacing ? Doc alignment check missing ?

function admin_header( $form = true, $option = 'yoast_wpseo_options', $optionshort = 'wpseo', $contains_files = false ) {
?>
Expand Down Expand Up @@ -190,7 +170,7 @@ function admin_header( $form = true, $option = 'yoast_wpseo_options', $optionsho
/**
* Generates the footer for admin pages
*
* @param bool $submit Whether or not a submit button and form end tag should be shown.
* @param bool $submit Whether or not a submit button and form end tag should be shown.
* @param bool $show_sidebar Whether or not to show the banner sidebar - used by premium plugins to disable it
*/
function admin_footer( $submit = true, $show_sidebar = true ) {
Expand Down Expand Up @@ -322,36 +302,29 @@ function export_settings( $include_taxonomy ) {
* Loads the required styles for the config page.
*/
function config_page_styles() {
global $pagenow;
if ( $pagenow === 'admin.php' && isset( $_GET['page'] ) && in_array( $_GET['page'], $this->adminpages ) ) {
wp_enqueue_style( 'dashboard' );
wp_enqueue_style( 'thickbox' );
wp_enqueue_style( 'global' );
wp_enqueue_style( 'wp-admin' );
wp_enqueue_style( 'yoast-admin-css', plugins_url( 'css/yst_plugin_tools' . WPSEO_CSSJS_SUFFIX . '.css', WPSEO_FILE ), array(), WPSEO_VERSION );

if ( is_rtl() ) {
wp_enqueue_style( 'wpseo-rtl', plugins_url( 'css/wpseo-rtl' . WPSEO_CSSJS_SUFFIX . '.css', WPSEO_FILE ), array(), WPSEO_VERSION );
}
wp_enqueue_style( 'dashboard' );
wp_enqueue_style( 'thickbox' );
wp_enqueue_style( 'global' );
wp_enqueue_style( 'wp-admin' );
wp_enqueue_style( 'yoast-admin-css', plugins_url( 'css/yst_plugin_tools' . WPSEO_CSSJS_SUFFIX . '.css', WPSEO_FILE ), array(), WPSEO_VERSION );

if ( is_rtl() ) {
wp_enqueue_style( 'wpseo-rtl', plugins_url( 'css/wpseo-rtl' . WPSEO_CSSJS_SUFFIX . '.css', WPSEO_FILE ), array(), WPSEO_VERSION );
}
}

/**
* Loads the required scripts for the config page.
*/
function config_page_scripts() {
global $pagenow;

if ( $pagenow == 'admin.php' && isset( $_GET['page'] ) && in_array( $_GET['page'], $this->adminpages ) ) {
wp_enqueue_script( 'wpseo-admin-script', plugins_url( 'js/wp-seo-admin' . WPSEO_CSSJS_SUFFIX . '.js', WPSEO_FILE ), array(
'jquery',
'jquery-ui-core',
), WPSEO_VERSION, true );
wp_enqueue_script( 'dashboard' );
wp_enqueue_script( 'thickbox' );
}

if ( $pagenow == 'admin.php' && isset( $_GET['page'] ) && in_array( $_GET['page'], array( 'wpseo_social' ) ) ) {
wp_enqueue_script( 'wpseo-admin-script', plugins_url( 'js/wp-seo-admin' . WPSEO_CSSJS_SUFFIX . '.js', WPSEO_FILE ), array(
'jquery',
'jquery-ui-core',
), WPSEO_VERSION, true );
wp_enqueue_script( 'dashboard' );
wp_enqueue_script( 'thickbox' );

if ( 'wpseo_social' === $_GET['page'] ) {
wp_enqueue_media();
wp_enqueue_script( 'wpseo-admin-media', plugins_url( 'js/wp-seo-admin-media' . WPSEO_CSSJS_SUFFIX . '.js', WPSEO_FILE ), array(
'jquery',
Expand All @@ -360,7 +333,7 @@ function config_page_scripts() {
wp_localize_script( 'wpseo-admin-media', 'wpseoMediaL10n', $this->localize_media_script() );
}

if ( $pagenow == 'admin.php' && isset( $_GET['page'] ) && in_array( $_GET['page'], array( 'wpseo_bulk-editor' ) ) ) {
if ( 'wpseo_bulk-editor' === $_GET['page'] ) {
wp_enqueue_script( 'wpseo-bulk-editor', plugins_url( 'js/wp-seo-bulk-editor' . WPSEO_CSSJS_SUFFIX . '.js', WPSEO_FILE ), array( 'jquery' ), WPSEO_VERSION, true );
}
}
Expand Down Expand Up @@ -396,10 +369,10 @@ function get_option( $option ) {
/**
* Create a Checkbox input field.
*
* @param string $var The variable within the option to create the checkbox for.
* @param string $label The label to show for the variable.
* @param bool $label_left Whether the label should be left (true) or right (false).
* @param string $option The option the variable belongs to.
* @param string $var The variable within the option to create the checkbox for.
* @param string $label The label to show for the variable.
* @param bool $label_left Whether the label should be left (true) or right (false).
* @param string $option The option the variable belongs to.
*
* @return string
*/
Expand Down Expand Up @@ -443,8 +416,8 @@ function checkbox( $var, $label, $label_left = false, $option = '' ) {
/**
* Create a Text input field.
*
* @param string $var The variable within the option to create the text input field for.
* @param string $label The label to show for the variable.
* @param string $var The variable within the option to create the text input field for.
* @param string $label The label to show for the variable.
* @param string $option The option the variable belongs to.
*
* @return string
Expand All @@ -463,10 +436,10 @@ function textinput( $var, $label, $option = '' ) {
/**
* Create a textarea.
*
* @param string $var The variable within the option to create the textarea for.
* @param string $label The label to show for the variable.
* @param string $var The variable within the option to create the textarea for.
* @param string $label The label to show for the variable.
* @param string $option The option the variable belongs to.
* @param string $class The CSS class to assign to the textarea.
* @param string $class The CSS class to assign to the textarea.
*
* @return string
*/
Expand All @@ -484,7 +457,7 @@ function textarea( $var, $label, $option = '', $class = '' ) {
/**
* Create a hidden input field.
*
* @param string $var The variable within the option to create the hidden input for.
* @param string $var The variable within the option to create the hidden input for.
* @param string $option The option the variable belongs to.
*
* @return string
Expand All @@ -507,9 +480,9 @@ function hidden( $var, $option = '' ) {
/**
* Create a Select Box.
*
* @param string $var The variable within the option to create the select for.
* @param string $label The label to show for the variable.
* @param array $values The select options to choose from.
* @param string $var The variable within the option to create the select for.
* @param string $label The label to show for the variable.
* @param array $values The select options to choose from.
* @param string $option The option the variable belongs to.
*
* @return string
Expand Down Expand Up @@ -541,8 +514,8 @@ function select( $var, $label, $values, $option = '' ) {
/**
* Create a File upload field.
*
* @param string $var The variable within the option to create the file upload field for.
* @param string $label The label to show for the variable.
* @param string $var The variable within the option to create the file upload field for.
* @param string $label The label to show for the variable.
* @param string $option The option the variable belongs to.
*
* @return string
Expand Down Expand Up @@ -608,9 +581,9 @@ function media_input( $var, $label, $option = '' ) {
/**
* Create a Radio input field.
*
* @param string $var The variable within the option to create the file upload field for.
* @param array $values The radio options to choose from.
* @param string $label The label to show for the variable.
* @param string $var The variable within the option to create the file upload field for.
* @param array $values The radio options to choose from.
* @param string $label The label to show for the variable.
* @param string $option The option the variable belongs to.
*
* @return string
Expand Down Expand Up @@ -649,8 +622,8 @@ function radio( $var, $values, $label, $option = '' ) {
/**
* Create a postbox widget.
*
* @param string $id ID of the postbox.
* @param string $title Title of the postbox.
* @param string $id ID of the postbox.
* @param string $title Title of the postbox.
* @param string $content Content of the postbox.
*/
function postbox( $id, $title, $content ) {
Expand Down
7 changes: 3 additions & 4 deletions wp-seo-main.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
}

/**
* @internal Nobody should be able to overrule the real version number as this can cause serious issues
* @note Nobody should be able to overrule the real version number as this can cause serious issues
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@internal is a PHPDoc tag, @note is not. @internal is used to indicate the info is for developers only
Ref: http://www.phpdoc.org/docs/latest/references/phpdoc/tags/internal.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, PHPStorm strikes through the used constant or function if an @internal comment is added, saying that the function or constant is "internal".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a bug in PHPStorm and should be fixed there, not in our code. @maartenba might be able to clarify.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, @internal means nobody outside the project or module should call this function.

You can disable this inspection which was added in 8.0 (https://youtrack.jetbrains.com/issue/WI-22284)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maartenba Thanks for your feedback.

Well, @internal means nobody outside the project or module should call this function.

Not just that according to the docs:

The @internal tag is used to denote that associated Structural Elements are elements internal to this application or library. It may also be used inside a long description to insert a piece of text that is only applicable for the developers of this software.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, added it to the issue. Feel free to +1 there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maartenba Done ;-)

* with the options, so no if ( ! defined() )
*/
define( 'WPSEO_VERSION', '1.7.1' );
Expand Down Expand Up @@ -375,12 +375,11 @@ function wpseo_admin_init() {
}
}

if ( in_array( $pagenow, array( 'edit-tags.php' ) ) ) {
if ( $pagenow === 'edit-tags.php' ) {
$GLOBALS['wpseo_taxonomy'] = new WPSEO_Taxonomy;
}

if ( in_array( $pagenow, array( 'admin.php' ) ) ) {
// @todo [JRF => whomever] Can we load this more selectively ? like only when $_GET['page'] is one of ours ?
if ( 'admin.php' === $pagenow && strpos( (string) filter_input( INPUT_GET, 'page' ), 'wpseo' ) === 0 ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately we've already run into the fact that the filter extension is not always enabled, so this code cannot be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right. ^%$^$.

$GLOBALS['wpseo_admin_pages'] = new WPSEO_Admin_Pages;

$GLOBALS['WPSEO_i18n'] = register_i18n_promo_class();
Expand Down