HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

First attempt - Wordpress PHP Settings API wrapper

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
settingsphpattemptwordpresswrapperfirstapi

Problem

I've been working on what is essentially my first proper attempt at making a useful PHP wrapper class for the Wordpress Settings API. It works great so far and I plan to add more methods as I go so that I eventually build up a class that can be used in pretty much any situation.

As this is my first proper PHP class, I'm looking for suggestions of ways that I could refactor or do things more efficiently. Perhaps places where I could write code differently/use different functions to improve it.

The code is also on my GitHub repo, so I'd appreciate any constructive feedback.

```
pages as $page )
{
if ($page['type']=='menu')
{
add_menu_page(
__( $page['title'] ),
__( $page['title'] ),
'administrator',
$page['id'],
array( &$this, 'displayOptionsPage' )
);
}
else if ($page['type'] == 'submenu')
{
add_submenu_page(
$page['parent'],
__( $page['title'] ),
__( $page['title'] ),
'administrator',
$page['id'],
array( &$this, 'displayOptionsPage' )
);
}
}

}

/**
* Add the settings sections
*
* @since 1.0
*/
private function addOptionsSections()
{
foreach( $this->sections as $section )
{
add_settings_section(
$section['id'],
__($section['title']),
'', // Add the description for the section in later
$section['page']
);
}
}

/**
* Add the settings fields
*
* @since 1.0
*/
private function addOptionsFields()
{
foreach ($this->fields as $section => $field)
{
foreach ($field as $option)
{

Solution

The first small thing that pops out is array( &$this, 'method_name' ). That's PHP 4 and you can totally drop the & (WordPress requires PHP 5.2.4+).

The second is that if your going to use localization, better to put all in place throughout your code, ie: $translated = __( 'Hello World!', 'mytextdomain' );.

And finally, and maybe that's micro-optimization, it's said that === is faster than ==.

Notes regarding the code samples bellow:

  • I try to follow WordPress Coding Standards.



  • I prefer your coding style, but will shorten some things for brevity.



Interface

I'd really consider moving the theme options to a second level menu. We tend to think that our product deserves the spotlight, but, from a users point of view, theme settings are something that you do once in a while, maybe only once. The first thing that I do when I setup a site is removing stuff from my first level menu, as I value that real estate a lot.

The method addOptionsPages() would be:

public function add_options_pages() { 
    foreach ( $this->pages as $page ) {
        if ( $page['type'] == 'menu' )
            add_theme_page( $args );
        else if ( $page['type'] == 'submenu' )
            add_submenu_page(
                null, # <-- Invisible page attached to /wp-admin/options.php?page=THIS-SUBMENU
                __( $page['title'], 'mytextdomain' ),
                __( $page['title'], 'mytextdomain' ),
                'administrator',
                $page['id'],
                array( $this, 'display_options_page' )
            );
    }
}


And adjust the method displayOptionsTabs() adding the full URL in %5$s:

$output .= sprintf(
    '%3$s',
    $page['id'],
    $currentTab,
    $page['title'],
    $activeClass,
    ( $page['type'] == 'menu' ) ? admin_url( 'themes.php') : admin_url( 'options.php')
);


You'll note that there'll be an issue with the current admin menu indicator, it can be solved like this or this.

Given that all this ends in a jQuery fix, you can consider only add_theme_page() and deal with the Tabs display doing a switch( $_GET['tab'] ) {}.

Scripts and styles

To target our own pages, the add_(sub)menu_page functions return the hook name that we can use like:

$hook = add_menu_page( $args );
add_action( "admin_print_scripts-$hook", array( $this, 'print_scripts' );

public function print_scripts(){
    ?>
     /* declare styles */ 
    /* run scripts */ 
    <?php
}


And to enqueue, and using your code:

private $hooks = array();

public function add_options_pages() { 
    foreach ( $this->pages as $page ) {
        if ($page['type']=='menu')
            $this->hooks[] = add_theme_page( $args );
        else if ($page['type'] == 'submenu')
            $this->hooks[] = add_submenu_page( $args );
    }
    add_action( 'admin_enqueue_scripts', array( $this, 'enqueue' ) );
}

public function enqueue( $hook ){
    if( !in_array( $hook, $this->hooks ) )
        return;
    wp_enqueue_script( 'my-scp', plugins_url( '/js/my-script.js', __FILE__) ); # Adjust to use with themes
    wp_enqueue_style( 'my-stl', plugins_url( '/css/my-style.js', __FILE__) ); 
}


Settings API

The only thing I'm missing is validation and sanitization in your register_setting( $option_group, $option_name, $sanitize_callback );. Two excellent references by leading WP developers:

-
WordPress Settings API Tutorial, by Otto Wood

// validate our options
function plugin_options_validate( $input ) {
    $newinput['text_string'] = trim( $input['text_string'] );
    if( !preg_match( '/^[a-z0-9]{32}$/i', $newinput['text_string'] ) )
        $newinput['text_string'] = '';
    return $newinput;
}



Take special note of the fact that I don’t return the original array. [...] In short, $input is untrusted data, but the returned $newinput should be trusted data.

-
The WordPress Settings API, by Konstantin Kovshenin

function my_settings_validate( $input ) {
    $output = get_option( 'my-settings' );        
    if ( is_email( $input['email'] ) )
        $output['email'] = $input['email'];
    else
        add_settings_error( 'my-settings', 'invalid-email', 'You have entered an invalid e-mail address.' );        
    return $output;
}



Here’s the trick with validation. We’ll use the same technique as sanitization, but we will return the current option value, if the new input value was not good enough.

You'd probably have to add a validation type inside your $PBThemeOptions->fields;

Database

In setOptionsDefaults() method, you're saving one option for each item in the fields array (example-options.php in your repo):

$fields = array(
    // Example Section One
    'sandbox_theme_menu' => array( /* etc */ ),
    // Example Section Two
    'sandbox_theme_display_options' => array( /* etc */ )
);


Those are only two items, but as good practice it's recommended that we store all our plugin/theme settings inside one option only. Truth be told, as you have it it

Code Snippets

public function add_options_pages() { 
    foreach ( $this->pages as $page ) {
        if ( $page['type'] == 'menu' )
            add_theme_page( $args );
        else if ( $page['type'] == 'submenu' )
            add_submenu_page(
                null, # <-- Invisible page attached to /wp-admin/options.php?page=THIS-SUBMENU
                __( $page['title'], 'mytextdomain' ),
                __( $page['title'], 'mytextdomain' ),
                'administrator',
                $page['id'],
                array( $this, 'display_options_page' )
            );
    }
}
$output .= sprintf(
    '<a href="%5$s?page=%1$s&tab=%2$s" class="nav-tab%4$s" id="%2$s-tab">%3$s</a>',
    $page['id'],
    $currentTab,
    $page['title'],
    $activeClass,
    ( $page['type'] == 'menu' ) ? admin_url( 'themes.php') : admin_url( 'options.php')
);
$hook = add_menu_page( $args );
add_action( "admin_print_scripts-$hook", array( $this, 'print_scripts' );

public function print_scripts(){
    ?>
    <style> /* declare styles */ </style>
    <script>/* run scripts */ </style>
    <?php
}
private $hooks = array();

public function add_options_pages() { 
    foreach ( $this->pages as $page ) {
        if ($page['type']=='menu')
            $this->hooks[] = add_theme_page( $args );
        else if ($page['type'] == 'submenu')
            $this->hooks[] = add_submenu_page( $args );
    }
    add_action( 'admin_enqueue_scripts', array( $this, 'enqueue' ) );
}

public function enqueue( $hook ){
    if( !in_array( $hook, $this->hooks ) )
        return;
    wp_enqueue_script( 'my-scp', plugins_url( '/js/my-script.js', __FILE__) ); # Adjust to use with themes
    wp_enqueue_style( 'my-stl', plugins_url( '/css/my-style.js', __FILE__) ); 
}
// validate our options
function plugin_options_validate( $input ) {
    $newinput['text_string'] = trim( $input['text_string'] );
    if( !preg_match( '/^[a-z0-9]{32}$/i', $newinput['text_string'] ) )
        $newinput['text_string'] = '';
    return $newinput;
}

Context

StackExchange Code Review Q#49724, answer score: 7

Revisions (0)

No revisions yet.