Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is using PHP if/else within JS bad practice?

I'm building a custom template for WordPress and in a couple places I've used PHP if else statements like the following example within the JS in the footer. It works fine but I'm wondering if this is considered "bad practice" and if so what is a better way to handle it?

<script type="text/javascript">
var $submenu = $('.submenu');
// SUBMENU animation
<?php if ( in_category('Collection') == false ) { ?> // if not a Collection subpage
    $('.menu li a').each(function() { 
        if ( $(this).text() == 'Collection' ) { // If is Collection link show submenu on hover
            $(this).mouseenter(function() { 
                $submenu.slideDown(); 
            });
        } else { // else close submenu on hover over other menu links
            $(this).mouseenter(function() { 
                $submenu.slideUp(); 
            });
        }
    });
    $('.nav').mouseleave(function() { // close submenu
        $submenu.slideUp(); 
    });
<?php } else { ?> // If a Collection subpage always show subnav
    $submenu.show();
<?php } ?>
</script>
like image 914
lizzmo Avatar asked Apr 03 '13 22:04

lizzmo


2 Answers

Whilst there isn't anything really wrong with mixing PHP and JavaScript, I personally find it quite awkward to read and modify, plus it makes moving that code around tricky. For example if you decided to export that JavaScript to an external file, which has numerous benefits:

<script src="myjs.js.php"></script>

This becomes clunky if your JavaScript needs to know certain values in order to calculate in_category('Collection') as you have to start using GET parameters (unless you are depending on session variables, which can get quite compex and unpredictable, especially through asset requests):

<script src="myjs.js.php?random_vars_required=123"></script>

Another point to be wary of is when having a JavaScript file that changes it's content depending on server-side logic, you have to be careful with what the browser is caching (to avoid these type of problems, it basically means you have to change the request URL for each possible outcome of the js file). i.e.

<script src="myjs.js.php?collection=true"></script>
<script src="myjs.js.php?collection=false"></script>

Another downside is by mixing PHP with JS you are likely to end up duplicating the PHP code in numerous places which goes against the DRY principal. This is why the suggested "export data to a javascript variable" is a much nicer idea. However it's best to avoid variables in the global js namespace if possible. Avoiding the global namespace can prove tricky though if you need to share the logic across multiple JavaScript files and don't wish to export your variables at the top of every file.

another possibility

If the logic you are testing is purely boolean in nature, and it also centres around page classification (or sub-region classification), the following is quite a nice way to handle what you are trying to achieve. It's nice mainly because it keeps your PHP and HTML together, and your JS separate.

The following should be placed in whatever template you use to generate your outer HTML:

<?php

    $classes = array();
    if ( in_category('Collection') ) {
      $classes[] = 'collection';
    }
    $classes = implode(' ', $classes);

?>
<!-- 
  obviously you'd render the rest of the html markup
  I've removed it for simplicity
//-->
<body class="<?php echo $classes; ?>"></body>

Then in your JavaScript / jQuery:

if ( $('body.collection').length ) {
  /// if collection sub page
}
else {
  /// else do otherwise
}

If you'd rather not add a class to your body element, you could always define your boolean check based on something that already exists on one version of the page and not on the other. Although personally I like to keep things clean and only resort to those kind of checks when I know the HTML markup is not going to change much in the future.

Nearly all browsers that the greater world should be worrying about today support multiple classes on elements. So this means even if you have multiple things you wish to check for, as long as it makes sense, you can place these classes on your html or body tag and use jQuery's Sizzle implementation to find them for you.

like image 100
Pebbl Avatar answered Oct 28 '22 02:10

Pebbl


Building javascript server-side is probably something we've all done, despite the main arguments for not doing so - namely that the js can't be (easily) validated (with eg. jsLint), and can't (easily) be put into a .js file - there's no point allowing the browser to cache just one of two or more possible versions of the script.

You could consider trading off server-side branching for client-side branching, which arguably makes the code more readable but, more importantly, is an intermediate step to my final suggestion (bear with me) :

var $submenu = $('.submenu');
// SUBMENU animation
var isCollection = <?php echo in_category('Collection') ? 'false' : 'true' ?>;
if ( !isCollection ) { // if not a Collection subpage
    $('.menu li a').each(function() { 
        if ( $(this).text() == 'Collection' ) { // If is Collection link show submenu on hover
            $(this).mouseenter(function() {
                $submenu.slideDown(); 
            });
        } else { // else close submenu on hover over other menu links
            $(this).mouseenter(function() {
                $submenu.slideUp(); 
            });
        }
    });
    $('.nav').mouseleave(function() { // close submenu
        $submenu.slideUp();
    });
} else { // If a Collection subpage always show subnav
    $submenu.show();
}

However, if the boolean isCollection could be determined by another means (eg. by enquiring some aspect of the DOM such as a data-xxx attribute), then you're cooking with gas. Only one version of the js script would be necessary; it could be easily validated with jsLint; and could be moved into a .js file if desired.

Of course you need to set the data-xxx attribute (or whatever) elsewhere in the server-side code (complete with an explanatory comment), which is a possible downside, but maybe not a big one.

Maybe not all js would be amenable to this approach but I think the example in the question would be.

To my mind, this is a viable way ahead on this occasion.

like image 42
Beetroot-Beetroot Avatar answered Oct 28 '22 00:10

Beetroot-Beetroot