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

remove redundant code #33

Closed
derpixler opened this issue Jan 28, 2016 · 2 comments
Closed

remove redundant code #33

derpixler opened this issue Jan 28, 2016 · 2 comments

Comments

@derpixler
Copy link
Contributor

The handling of admin pages are very prone for errors because the code repeats oneself.

Look at the tabs, we have the same markup on 5 different files:
https://github.com/inpsyde/search-and-replace/tree/3.0.1/src/inc/templates

    <h2 class="nav-tab-wrapper">
        <a class="nav-tab" href="<?php echo admin_url() ?>/tools.php?page=sqlesc_html_export"><?php esc_html_e( 'Backup Database', 'insr' ); ?></a>
        <a class="nav-tab" href="<?php echo admin_url() ?>/tools.php?page=replace_domain"><?php esc_html_e( 'Replace Domain/URL', 'insr' ); ?></a>
        <a class="nav-tab" href="<?php echo admin_url() ?>/tools.php?page=inpsyde_search_replace"><?php esc_html_e( 'Search and replace', 'insr' ); ?></a>
        <a class="nav-tab" href="<?php echo admin_url() ?>/tools.php?page=sql_import"><?php esc_html_e( 'Import SQL file', 'insr' ); ?></a>
        <a class="nav-tab nav-tab-active" href="<?php echo admin_url() ?>/tools.php?page=credits"><?php esc_html_e( 'Credits', 'insr' ); ?></a>
    </h2>

Go a head to src/inc/Init.php here we have identical functions.
https://github.com/inpsyde/search-and-replace/blob/3.0.1/src/inc/Init.php

    /**
     *callback function for db backup  page
     */
    public function show_db_backup_page() {

        $export_admin = new DbBackupAdmin();
        $export_admin->show_page();
    }

    /**
     *callback function for replace domain page
     */
    public function show_replace_domain_page() {

        $export_admin = new ReplaceDomainAdmin();
        $export_admin->show_page();
    }

    /**
     *callback function for import page
     */
    public function show_import_page() {

        $import_admin = new SqlImportAdmin();
        $import_admin->show_page();
    }

    /**
     *callback function for import page
     */
    public function show_credits_page() {

        $import_admin = new CreditsAdmin();
        $import_admin->show_page();
    }

TODO: Reduce code and implement better tab handling.

@Chrico
Copy link
Contributor

Chrico commented Feb 9, 2016

URLs with GET-Params should be printed out with WP-Core-functions:

$tools_url = admin_url( 'tools.php' );
echo add_query_arg( 'page', 'db_export', $tools_url );

Additionally the duplicated admin_url()-call should be removed.


Instead of using multiple methods within the Init-class we should introduce a new class (e.G. PageManager) which contains all pages and handles the rendering of them. The page-classes should implement an interface.

PageInternface

interface PageInterface {
   public function get_menu_title();
   public function get_page_title();
   public function render();
}

PageManager

class PageManager {

    /**
     * @var PageInterface[]
     */
    private $pages;

    public function add( PageInterface $page ) {
        $slug                   = sanitize_title_with_dashes( $page->get_menu_title() );
        $this->pages[ $slug ]   = $page
    }

    public function register()  {
        $cap = apply_filters( 'insr-capability', 'install_plugins' );
        foreach ( $this->pages as $slug => $page ) {
            add_submenu_page( 
                'tools.php', 
                $page->get_page_title(),
                $page->get_menu_title(),
                $cap,
                $slug
                array( $this, 'render' )
            );
        }
    }

    public function render() {
        $url            = admin_url( 'tools.php' );
        $current_page   = $_GET[ 'page' ] ) ? $_GET[ 'page' ] : key( $this->pages );

        $output = '<ul class="tab__list">';
        foreach( $this->pages as $slug => $page ) :
            $class = $current_page === $slug ? 'tab__item--is-active' : '';
            $output .= sprintf( 
                '<li class="tab__item %1$s"><a href="%2$s">%3$s</a></li>' 
                esc_attr( $class ),
                add_query_arg( 'page', $slug, $url ),
                $page->get_page_title()
            );
        endforeach;
        $output .= '</ul>';

        echo $output;
        echo '<div class="tab__content">';
        $this->pages[ $current_page ]->render();
        echo '</div>';
    }

}

Usage

class ReplaceDomainAdmin implements PageInterface {
    public function get_page_title() { __( 'Replace Domain', '..' ); }
    public function get_menu_title() { /* snip */ }
    public function render() { /* snip */  }
}


$manager = new PageManager();
$manger->add( new ReplaceDomainAdmin() );
add_action( 'admin_menu', array( $manager , 'register' ) );

Edit: The naming should also be improved. The class ReplaceDomainAdmin should be moved to an own sub-namespace (within a folder).

Example: Folder: inc/Settings/ReplaceDomain.php | Namespace: Inpsyde\SearchReplace\Settings.

@derpixler
Copy link
Contributor Author

solved

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

No branches or pull requests

2 participants