5

I have a simple menu made of <ul><li> elements and a class="active" in place to mark the current page. A variable is passed by $_get[] to select the specific page by url: ?pg=PAGE.

I am fairly new to php and still learning. This works just fine, but i feel there ought to be a simpler and shorter way.

<ul class="nav">
  <li <?php if ($_GET['pg'] == "PAGE1") { echo "class=\"active\""; } ?>><a href="?pg=PAGE1">FIRST PAGE</a></li>
  <li <?php if ($_GET['pg'] == "PAGE2") { echo "class=\"active\""; } ?>><a href="?pg=PAGE2">SECOND PAGE</a></li>
</ul>
1
  • 1
    You're doing N operations to print N links. Doing any less is called telepathy 0:-) anything else is optimization of the code itself, not the logic. Apr 25, 2013 at 22:19
4
<?php
    $pages = array(
        'PAGE1' => 'FIRST PAGE',
        'PAGE2' => 'SECOND PAGE');
?>

<ul class="nav">
  <?php foreach ($pages as $pageId => $pageTitle): ?>
  <li <?=(($_GET['pg'] == $pageId) ? 'class="active"' : '')?>><a href="?pg=<?=$pageId?>"><?=$pageTitle?></a></li>
  <?php endforeach; ?>
</ul>

http://php.net/manual/en/control-structures.foreach.php

Don't repeat yourself -- both li-s are very similar, the only difference is in page ID and title. This approach will really help once you have more than two pages.

Try to keep PHP and HTML as separate as possible -- this will make your life easier once you decide to keep them in separate files (and you will sometimes).

4
  • I can never understand when code uses so many <?php and <? tags...no fewer than 10 within your <ul> block. Makes the code so much harder to read than a simple foreach block with an echo statement in it.
    – Madbreaks
    Apr 25, 2013 at 22:30
  • In this example, yes, probably. But in real code where you have much more HTML, mixed with JavaScript and CSS, keeping all this inside PHP echo is a nightmare.
    – hudolejev
    Apr 25, 2013 at 22:33
  • You mix HTML, JS and CSS in a single view? That does sound like a nightmare.
    – Madbreaks
    Apr 25, 2013 at 22:37
  • Not me. I'm usually the one who tries to get rid of that where possible. Converting PHP echo-s to something template-like is the first step usually, HTML coder can do the further cleanup.
    – hudolejev
    Apr 25, 2013 at 22:48
1
<?php

$navItems = [
  '/' => 'Home',
  '/about' => 'About',
  '/contact' => 'Contact'
];

function navItemClass($itemUri) {
  if ($_SERVER[REQUEST_URI] == $itemUri) {
    return 'is-active';
  }
  return '';
}

?>

...

<ul>
<?php foreach ($navItems as $uri => name): ?>
  <li class="<?php echo navItemClass($uri); ?>">
    <a href="<?php echo $uri; ?>">
      <?php echo $name; ?>
    </a>
  </li>
<?php endforeach; ?>
</ul>
0

Use ternary operator:

<?
$pg = $_GET['pg'];
?>

<ul class="nav">
    <li <?=($pg=="PAGE1"?'class="active"':'')?>><a href="?pg=PAGE1">FIRST PAGE</a></li>
    <li <?=($pg=="PAGE2"?'class="active"':'')?>><a href="?pg=PAGE2">SECOND PAGE</a></li>
</ul>
4
  • It assumes short tags are enabled. And no self-respecting shop I've worked for does so. ;)
    – Madbreaks
    Apr 25, 2013 at 22:24
  • @Madbreaks interesting, in my experience 95% short tags ON
    – michi
    Apr 25, 2013 at 22:26
  • Short tags are not recommended: php.net/manual/en/language.basic-syntax.phptags.php -- doesn't matter these are enabled on 95% of machines, your code will still fail on other 5%. Full tags will always work.
    – hudolejev
    Apr 25, 2013 at 22:42
  • Just for the record, most projects I've worked on have short tags enabled for legacy reasons but have also coding conventions that does not allow to use short tags in new code.
    – hudolejev
    Apr 25, 2013 at 22:43
0

You could use javascript/Jquery to check the value of $_get[] and use that to add a class to the li tag dynamically. This will remove the need to have the check for each li item, but perhaps might be a bit more complicated for you since you stated you are just new to this.

Anyhow since you are learning, it is good to start looking in that direction as well. PHP and JS go hand in hand most of the time...

2
  • Op's looking for how to improve server-side code. Suggesting jQuery as a solution borders on ridiculous.
    – Madbreaks
    Apr 25, 2013 at 22:27
  • 4
    whoa, chill down @Madbreaks. Bad day?
    – raidenace
    Apr 25, 2013 at 22:28
0

You could put your pages into an array and make this much easier:

<ul class="nav">
<?
$pages = array();
$pages[] = array("PAGE1","FIRST PAGE");
$pages[] = array("PAGE2","SECOND PAGE");

foreach($pages as $page) { ?>
    <li <?=(($_GET['pg'] == $page[0])?'class=\"active\"':'')?>><a href="?pg=<?=$page[0]?>"><?=$page[1]?></a></li>
<? } ?>
</ul>
0
<?php
$server = $_SERVER['REQUEST_URI'];
$pages = array(
    'all_page' => '/university/index.php/pages/all_page',
    'page_add' => '/university/index.php/pages/page_add',
    'menus' => '/university/index.php/pages/index',
    'menu' => '/university/index.php/pages/',
    'sliderAdd' => '/university/index.php/Slider/sliderAdd',
    'sliderView' => '/university/index.php/Slider/sliderView',
    'Future' => '/university/index.php/Slider/Future',
    'contact_us' => '/university/index.php/pages/contact_us',
    'newsAdd' => '/university/index.php/News/newsAdd',
    'newsView' => '/university/index.php/News/index',
    'eventsAdd' => '/university/index.php/Events/eventsAdd',
    'eventsView' => '/university/index.php/Events/index',
    'emailPhone' => '/university/index.php/Slider/emailPhone',
    'Logos' => '/university/index.php/Slider/Logo',
    'footer_change' => '/university/index.php/Slider/footer_change'
);
?>
<div class="nab">
    <div class="sidebar-collapse">
        <ul class="nav metismenu" id="side-menu">
            <li class="nav-header">
                <div class="dropdown profile-element"> <span style="margin-left:-15px;"> 
                        <img alt="image"  src="<?php echo base_url(); ?>upload/image-22082017080213.png" height="60" />
                    </span>

                </div>
                <div class="logo-element">
                    USMS
                </div>
            </li>
            <!--            <li>
                            <a href="<?php //echo site_url('dashboard/index');       ?>"><i class="fa fa-th-large"></i> <span class="nav-label">Dashboard</span></a>
                        </li>-->
            <li<?php
extract($pages);
if ($all_page == $server || $page_add == $server) {
    ?> class="active" <?php } ?>>
                <a href=""><i class="fa fa-file-code-o"></i> <span class="nav-label">Pages </span> <span class="fa arrow"></span></a>
                <ul class="nav nav-second-level collapse">
                    <li <?php if ($page_add == $server) { ?> class="active" <?php } ?>><a href="<?php echo site_url('pages/page_add'); ?>">Pages Add </a></li>
                    <li <?php if ($all_page == $server) { ?> class="active" <?php } ?>><a href="<?php echo site_url('pages/all_page'); ?>">All Pages</a></li>


                </ul>
            </li>
            <li <?php if ($sliderAdd == $server || $sliderView == $server) { ?> class="active" <?php } ?>>
                <a href=""><i class="fa fa-film"></i> <span class="nav-label">Slider</span> <span class="fa arrow"></span></a>
                <ul class="nav nav-second-level collapse">
                    <li  <?php if ($sliderAdd == $server) { ?> class="active" <?php } ?>><a href="<?php echo site_url('Slider/sliderAdd'); ?>">Slider Add </a></li>
                    <li  <?php if ($sliderView == $server) { ?> class="active" <?php } ?>><a href="<?php echo site_url('Slider/sliderView'); ?>">Slider View</a></li>


                </ul>
            </li>

            <li  <?php if ($Future == $server) { ?> class="active" <?php } ?>> 
                <a href="<?php echo site_url('Slider/Future'); ?>"><i class="fa fa-cubes"></i> <span class="nav-label">Future</span></a>
            </li>




            <li <?php if ($contact_us == $server) { ?> class="active" <?php } ?>>
                <a href="<?php echo site_url('pages/contact_us'); ?>"><i class="fa fa-map-marker"></i> <span class="nav-label">Contact Us</span></a>
            </li>

            <li <?php if ($newsAdd == $server || $newsView == $server) { ?> class="active" <?php } ?>>
                <a href=""><i class="fa fa-newspaper-o"></i> <span class="nav-label">News</span> <span class="fa arrow"></span></a>
                <ul class="nav nav-second-level collapse">


                    <li<?php if ($newsAdd == $server) { ?> class="active" <?php } ?>><a href="<?php echo site_url('News/newsAdd'); ?>">News Add</a></li>
                    <li <?php if ($newsView == $server) { ?> class="active" <?php } ?>><a href="<?php echo site_url('News/index'); ?>">All News</a></li>`
`
                </ul>
            </li>


            <li <?php if ($eventsAdd == $server || $eventsView == $server) { ?> class="active" <?php } ?>>
                <a href=""><i class="fa fa-university"></i> <span class="nav-label">Events</span> <span class="fa arrow"></span></a>
                <ul class="nav nav-second-level collapse">


                    <li <?php if ($eventsAdd == $server) { ?> class="active" <?php } ?>><a href="<?php echo site_url('Events/eventsAdd'); ?>">Events Add</a></li>
                    <li <?php if ($eventsView == $server) { ?> class="active" <?php } ?>><a href="<?php echo site_url('Events/index'); ?>">All Events</a></li>
                </ul>
            </li>

            <li <?php if ($menus == $server || $menu == $server || $emailPhone == $server || $Logos == $server || $footer_change == $server) { ?> class="active" <?php } ?> >>
                <a href=""><i class="fa fa-gear"></i> <span class="nav-label">Setting</span> <span class="fa arrow"></span></a>
                <ul class="nav nav-second-level collapse">


                    <li <?php if ($emailPhone == $server) { ?> class="active" <?php } ?>><a href="<?php echo site_url('Slider/emailPhone'); ?>">Header Email And Phone Number#</a></li>
                    <li <?php if ($Logos == $server) { ?> class="active" <?php } ?>> <a href="<?php echo site_url('Slider/Logo'); ?>">Header Logo</a></li>
                    <li <?php if ($menus == $server || $menu == $server) { ?> class="active" <?php } ?> ><a href="<?php echo site_url('pages/index'); ?>">Menus</a></li>
                    <li <?php if ($footer_change == $server) { ?> class="active" <?php } ?>><a href="<?php echo site_url('Slider/footer_change'); ?>">Footer </a></li>
                </ul>
            </li>
        </ul>

    </div>
</div>
0

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.