1
\$\begingroup\$

After a few weeks of searching and learning ajax, pagination php, product filter, I created ajax php product filter with pagination.

1. index.php

<?php require_once('../../../private/initialize.php'); ?>

<?php include(SHARED_PATH . '/staff_header.php'); ?>

<!-- Filtering -->
<section id="filtering">
<h3>Product Filter with Pagination</h3>

 <div class="content-box-lg">
  <div class="container-fluid">
   <div class="row">
    <!-- Left Panel-->   
        <div class="col-lg-3">  
        <h5>Category</h5>

        <ul class="list-group">
        <?php 
        $sql = "SELECT DISTINCT(category) FROM photographs ";
        $sql .= "ORDER BY category ASC";
        $photos = Photograph::find_by_sql($sql);

        foreach($photos as $row) { ?>
        <li class="list-group-item">
            <div class="form-check">
                <label class="form-check-label">
                    <input type="checkbox" class="common_selector category" value="<?php echo h($row->category); ?>" > &nbsp; 
                     <?php echo h($row->category); ?>
                </label>
            </div>
        </li>
        <?php } ?>      
        </ul>
        <br>

        <h5 class="text-info">Color</h5>
        <ul class="list-group">

        <?php 
        $sql = "SELECT DISTINCT(color) FROM photographs ";
        $sql .= "ORDER BY color ASC";
        $photos = Photograph::find_by_sql($sql);

        foreach($photos as $row) { ?>
        <li class="list-group-item">
            <div class="form-check">
                <label class="form-check-label">
                    <input type="checkbox" class="common_selector color" value="<?php echo h($row->color); ?>" > &nbsp; 
                     <?php echo h($row->color); ?>
                </label>
            </div>
        </li>
        <?php } ?>      

        </ul>


     </div>

    <!-- Center - load data-->  
    <div class="col-lg-9">

        <!-- Text Change -->
        <h5 class="text-center" id="textChange">All products</h5>
        <hr>

        <div class="row filter_data">
        <!-- load data -->  

        </div>
    </div>


   </div>
  </div>
 </div>
</section>


<style>
#loading
{
    text-align: center; 
    background: url('loader.gif') no-repeat center; 
    height: 150px;
    z-index: 999;
}
</style>

    <?php include(SHARED_PATH . '/staff_footer_filter.php'); ?>

2. fetch_data.php

<?php require_once('../../../private/initialize.php'); ?>
<?php

//fetch_data.php

$per_page = 8;
$current_page = !empty($_POST['page']) ? (int)$_POST['page'] : '1';
$offset = ($current_page - 1) * $per_page;

if(isset($_POST["action"])) {   
    $sql1 = "SELECT COUNT(*) ";
    $sql1 .= "FROM photographs ";
    $sql1 .= "WHERE category !='' ";

    if(isset($_POST["category"])) {
        $category = implode("','", $_POST['category']);
        $sql1 .= "AND category IN('".$category."') ";
    }

    if(isset($_POST["color"])) {
        $color = implode("','", $_POST['color']);
        $sql1 .= "AND color IN('".$color."')";
    }

    $result = Photograph::find_by_sql77($sql1);     
    $row = $result->fetch_array();
    $result->free();    
    $total_count = $row[0];

    $sql = "SELECT * ";
    $sql .= "FROM photographs ";
    $sql .= "WHERE category !='' ";

    if(isset($_POST["category"])) {
        $category_filter = implode("','", $_POST['category']);
        $sql .= "AND category IN('".$category_filter."') ";
        }

    if(isset($_POST["color"])) {
        $color_filter = implode("','", $_POST["color"]);
        $sql .= "AND color IN('".$color_filter."') ";
        }
    $sql .= "LIMIT {$per_page} ";
    $sql .= "OFFSET {$offset}";

    $photos = Photograph::find_by_sql($sql);

    if($total_count > 0) {

        foreach($photos as $photo) { ?>

        <!--https://stackoverflow.com/questions/41794746/col-xs-not-working-in-bootstrap-4-->
        <div class="col-md-3 col-sm-6 col-xs-12">
            <!-- Item 01 -->
            <div class="portfolio-item">

                <a href="show.php?id=<?php echo h(u($photo->id)); ?>">
                    <!-- img-fluid bootstrap 4-->
                    <img src="<?php echo h($photo->image_path_public()); ?>" class="img-fluid"/>
                </a>

                <div class="portfolio-item-overlay">

                    <div class="portfolio-item-details text-center">

                        <!-- Item Category -->
                        <h3>
                            <?php echo h($photo->category); ?>
                        </h3>

                        <!-- Item Name -->
                        <p>
                            <?php echo h($photo->caption); ?>
                        </p>

                    </div>
                </div>
            </div>
        </div>

      <?php } 
    } else {
        echo '<br>' . 'no data found';
    }
}

?>

<div class="container-fluid">
  <div class="row">
    <div class="col-lg-12">
      <div class="pagination justify-content-center pagination-sm">

      <?php

      $total_pages = ceil($total_count / $per_page);

      function total_pages($total_count, $per_page) {
        return ceil($total_count / $per_page); 
      } 

      function previous_page($current_page) {
        $prev = $current_page - 1;
        return ($prev > 0) ? $prev : false;
      }

      function next_page($total_pages, $current_page) {
        $next = $current_page + 1;
        return ($next <= $total_pages) ? $next : false;
      } 

        if ($total_pages > 1) {

            if (previous_page($current_page) != false) {
                echo "<span class='pagination_link page-link' style='cursor:pointer;'  id='" . previous_page($current_page) . "'>Poprzedni</span>";
            }

            for ($i = max( 1, $current_page - 5 ); $i <= min( $current_page + 5, total_pages($total_count, $per_page)); $i++ ) {
                if ($current_page == $i) {
                    echo "<span class=\"selected page-link\">{$i}</span>";
                } else {
                    echo "<span class='pagination_link page-link' style='cursor:pointer;' id='" . $i . "'>" . $i . "</span>";

                }
            }

            if (next_page($total_pages, $current_page) != false) {
                echo "<span class='pagination_link page-link' style='cursor:pointer;' id='" . next_page($total_pages, $current_page) . "'>Następny</span>";
            }
        }
        ?>

        </div>
     </div>
  </div>
</div>

3. staff_footer_filter.php

    <!-- Footer  -->
    <footer class="text-center">
        <div class="container">
            <div class="row">
                <div class="col-md-12">
                    <p>
                        Copyright &copy; <?php echo date('Y'); ?> <span>Mr. Robot</span>
                    </p>
                </div>
            </div>
        </div>

    </footer>
    <!-- Footer Ends -->

    <!-- JQuery -->
    <script src="<?php echo url_for('js/jquery.min.js'); ?>"></script>

    <!-- JQuery-UI -->
    <script src="<?php echo url_for('js/jquery-ui/jquery-ui.js'); ?>"></script>

    <!-- Popper JS -->
    <script src="<?php echo url_for('js/popper.min.js'); ?>"></script>

    <!-- Bootstrap JS -->
    <script src="<?php echo url_for('js/bootstrap4/bootstrap.min.js'); ?>"></script>

    <!-- Custom JS -->
    <script src="<?php echo url_for('js/script.js'); ?>"></script>


<script>


$(document).ready(function(){

    filter_data(1);

    function filter_data(page)
    {
        $('.filter_data').html('<div id="loading" style="" ></div>');
        var action = 'fetch_data';
        var category = get_filter('category');
        var color = get_filter('color');
        $.ajax({
            url:"fetch_data.php",
            method:"POST",
            data:{action:action, category:category, color:color, page:page},
            success:function(data){
                $('.filter_data').html(data);
                //$('#pagination_link').html(data.pagination_link);
            }
        });
    }

    $(document).on('click', '.pagination_link', function(event) // this line binds the handler to your pagination
    {
        event.preventDefault(); //this prevents the link being loaded as a new page, the default behaviour
        var page = $(this).attr("id"); //we get the url of the page to load in by ajax
        filter_data(page);
        //--------------------------------------------------//
        history.pushState(null, null, $(this).attr('id'));
        historyedited = true;
        //--------------------------------------------------//
        return event.preventDefault();
    });

        window.addEventListener('popstate', e => {
            filter_data(e.state.page);
            console.log('popstate!');   
    });


        history.replaceState({page: null}, 'Default state', './');


    function get_filter(class_name)
    {
        var filter = [];
        $('.'+class_name+':checked').each(function(){
            filter.push($(this).val());
        });
        return filter;
    }

    $('.common_selector').click(function(){
        filter_data(1);
        $("#textChange").text("Filtered products");
    });


});
</script>
</body>
</html>

<?php
  db_disconnect($database);
?>
\$\endgroup\$
1
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. If you have new code you want reviewed, please post a new question. They're free today. \$\endgroup\$ – Mast Apr 16 '20 at 9:35
3
\$\begingroup\$
  • h() is a poor choice for a function name. It makes no effort to inform the developer about what exact process it executes. We are no longer in the digital age where saving keystroke/characters is a valuable pursuit. This function should be renamed to make its action abundantly clear.

  • You should never call the same function on the same data more than once. You should save the outcome to a variable and access that cached value as many times as needed. This eliminates needless function calls.

  • As a general rule, try to avoid declaring single-use variables. There are some situations that argue against this advice (like when declaring the variable helps to describe a value that is somewhat cryptic), so this is a grey area.

  • Use css styling to manage spacing between elements/text (not &nbsp; and <br>) -- this will keep your script cleaner and easier to fine-tune.

  • You have redundant code building your un-ordered lists. You should also only present photo filtering lists if there are actually list items to go inside of them -- be sure to make this check even if you know that there will be some.

    foreach (['category', 'color'] as $listName) {
        $listItems = [];
        foreach (Photograph::find_by_sql("SELECT DISTINCT({$listName}) FROM photographs ORDER BY {$listName}") as $row) {
            $value = h($row->$listName);
            $listItems[] = '<li class="list-group-item">
                <div class="form-check">
                    <label class="form-check-label">
                        <input type="checkbox" class="common_selector ' . $value . '" value="' . $value . '">' . $value . '
                    </label>
                </div>
            </li>';
        }
    
        if ($listItems) {
            echo '<h5>' . ucfirst($listName) . '</h5>';
            echo '<ul class="list-group">' . implode("\n", $listItems) . '</ul>';
        }
    }
    
  • You should endeavor to move your js and css declaration to external files and simply include them where desired.

  • I recommend that declare your php function at the top of your file to keep it out of your markup.

  • previous_page() can be rewritten as the following because you are performing a falsey check on the return value anyhow.

    function previous_page($current_page) {
        return max(0, $current_page - 1);  // return will never be lower than 0
    }
    
  • if (previous_page($current_page) != false) { is written more concisely as
    if (previous_page($current_page)) {

  • style='cursor:pointer;' should be moved from inline declarations to an external stylesheet.

  • I don't like the single-use declarations of:

    var action = 'fetch_data';
    var category = get_filter('category');
    var color = get_filter('color');
    

    Just apply those values directly to the data: declaration.

  • Instead of using data to describe the response html, perhaps use the term response or responseHTML so make the script more intuitive and avoid confusion.

  • If you are going to declare: var page = $(this).attr("id");, (I prefer let over var), then use page everywhere downscript. Asking jquery to refetch $(this).attr("id"); is unnecessary work.

  • Disconnecting from your database at the end of your script is unnecessary because php is going to do this for you automatically. There is no benefit to writing it out.

\$\endgroup\$
10
  • \$\begingroup\$ Thank you for your response, I added: Update1 h(); u(); find_by_sql and find_by_sql77 \$\endgroup\$ – Mantykora 7 Apr 16 '20 at 6:54
  • \$\begingroup\$ Your custom functions which act as wrappers for native functions provide no value to your script. Just remove them entirely and call the native functions only. I don't understand why a method would have 77 in it. \$\endgroup\$ – mickmackusa Apr 16 '20 at 7:08
  • \$\begingroup\$ "var page = $(this).attr("id");" can you explain to me or give me an example because I don't understand that? \$\endgroup\$ – Mantykora 7 Apr 16 '20 at 7:29
  • \$\begingroup\$ history.pushState(null, null, page); is the same thing, right? \$\endgroup\$ – mickmackusa Apr 16 '20 at 8:07
  • 1
    \$\begingroup\$ I don't want to open a chat box, I am spending time with my family now, so I won't be replying further. Change your query, let MySQL do the counting. Use SELECT COUNT(1) FROM ... \$\endgroup\$ – mickmackusa Apr 16 '20 at 8:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.