Responsive menu highlights page with JavaScript

Question

This is a responsive navigation menu (pure css), that highlights the page with a short JavaScript I found online. I have it all working, but the CSS is totally a mess. The menu is actually a PHP .inc file, so that it can be used site wide.

  $(document).ready(function(){
      var str=location.href.toLowerCase();
    $('nav li a').each(function() {
            if (str.indexOf(this.href.toLowerCase()) > -1) {
                    $("li.highlight").removeClass("highlight");
                    $(this).parent().addClass("highlight"); 
               }
                                        }); 
    $('li.highlight').parents().each(function(){

                if ($(this).is('li')){
                    $(this).addClass("highlight"); 
                    }                             
                                              });
   })

/* nav menu css----*/
nav {padding: 50px 0 30px 0; font-size: 24px; border: none; color: #7E6B6E;}

nav ul {list-style: none;}

nav li {float: left;}

nav li a {
    padding: 5px 5px;
    color: #DBA182;
    display: block;
    font-weight: normal;
    border-bottom: 3px solid #fff;}

nav li.highlight a {
    color: #DB002E;
    display: block;
    border-bottom: 3px solid #DB002E;}

nav ul li:hover a {
    color: #DB002E;
    display: block;
    font-weight: normal;
    border-bottom: 3px solid #DB002E;}

nav ul li.highlight a {
    color: #DB002E;
    display: block;
    border-bottom: 3px solid #DB002E;}

nav ul li.highlight:hover a {
    color: #DB002E;
    display: block;
    border-bottom: 3px solid #DB002E;}

nav li ul {
    background: #F1EDED;
    display: none;
    position: absolute;
    z-index: 200;}

nav li:hover ul {
    display: block;}

nav li li {
    display: block;
    float: none;
    padding: 0;
    width: auto;
    color: #7E6B6E;
    border-bottom: 1px solid #FFF;}

nav li:hover li a {
    background: none;
    color: #7E6B6E;
    border-bottom: 1px solid #FFF;}

nav li.highlight li a {
    background: none;
    color: #7E6B6E;
    border-bottom: 1px solid #FFF;}

nav li.highlight:hover li a {
    background: none;
    color: #7E6B6E;
    border-bottom: 1px solid #FFF;}

nav li ul a {
    display: block;
    font-size: 18px;
    color: #7E6B6E;
    border-bottom: 1px solid #FFF;}

nav li ul a:hover, nav li ul li:hover a {
    background: #efdacf;
    color: #DB002E;
    border-bottom: 1px solid #FFF;}

nav li.highlight ul a:hover, nav li.highlight ul li:hover a {
    background: #efdacf;
    color: #DB002E;
    border-bottom: 1px solid #FFF;}

nav li.highlight ul.childNav li.highlight {
    position: relative;
    float: none;
    width: auto;
    border-bottom: 1px solid #FFF;}

nav li.highlight ul.childNav li.highlight a {
    position: relative;
    float: none;
    width: auto;
    color: #FFF;
    background: #E8C2AD;
    border-bottom: 1px solid #DB002E;}

nav li.highlight ul.childNav li.highlight a:hover {
    position: relative;
    float: none;
    width: auto;
    color: #FFF;
    background: #E8C2AD;
    border-bottom: 1px solid #DB002E;}

.show-menu {color: #fff; background: #E5DACF; text-align: center; padding: 10px 5px; display: none; cursor: pointer;}

/*Hide checkbox*/
input[type=checkbox] {display: none;}

/*Show menu when invisible checkbox is checked*/
input[type=checkbox]:checked ~ #menu {display: block;}

/*media query */
@media handheld, only screen and (max-width: 767px) {

/* nav Make dropdown links appear inline*/

nav {
    padding: 10px 0 10px 0;
}
nav ul, nav ul.childNav {
    position: static;
    display: none;
    background-color: #F8F6F3;
}

nav li {
}

/* nav Make all menu links full width*/
nav ul li, nav ul.childNav, nav li a {
    width: 100%;
    display: block;
}

/*nav Display 'show menu' link*/
nav .show-menu {
    display: block;
}


}
/* end nav */

<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<nav>
<label for="show-menu" class="show-menu">Menu</label>
    <input type="checkbox" id="show-menu" role="button">
        <ul id="menu">
    <li><a href="/">home</a></li>
    <li><a href="#">about</a>
      <ul class="childNav">
        <li><a href="#">about link 2</a></li>
        <li><a href="#">about link 3</a></li>
      </ul>
    </li>
    <li><a href="#">services</a>
      <ul class="childNav">
        <li><a href="#">services link 2</a></li>
        <li><a href="#">services link 3</a></li>
      </ul>
    </li>
    <li><a href="#">contact</a></li>
  </ul>
</nav>

Answer 1

OK, I think there's quite a few issues with this CSS but there's not really much benefit to your learning if I just take your input here and refactor it. Instead I'm going to paste your code on JSFiddle and suggest improvements.

So the first thing that I can already suggest is to properly format your code. In your original post all of the JavaScript is not aligned properly, so I took the liberty of reformatting it. As you can see now it is a lot clearer; I suggest that you come up with a system that looks good to you for future reference, to keep your code looking clean & consistent.

As for the CSS, I'd just like to reference some line numbers on the fiddle:

Lines 2, 4, 6, 106, 109 & 112

The way that you formatted here isn't beneficial if you need to add properties later on. I recommend that you follow this pattern for all of your selectors:

h1 {
    color: blue;
}

Line 8

Specifying the li inside this selector doesn't really give you any benefit, unless you have an anchor tag that is not inside a list item. Instead, do nav a.

Lines 15-34

These really need rewriting. Look into grouping your selectors by common properties, like so:

nav ul li.highlight a,
nav ul li.highlight:hover a,
nav li.highlight a,
nav ul li:hover a {
    color: #DB002E;
    display: block;
    border-bottom: 3px solid #DB002E;
}

nav ul li:hover a {
    display: block;
}

Line 49

width: auto is the default, it should be omitted here.

Line 50-51, 55-56 & 60-61, etc

Again, you should consolidate your rules together.

nav li li {
    display: block;
    float: none;
    padding: 0;
}

nav li li,
nav li:hover li a,
nav li.highlight li a {
    color: #7E6B6E;
    border-bottom: 1px solid #FFF;
}

nav li:hover li a,
nav li.highlight li a {
    background: none;
}

The same goes for the rest of the rules that are identical.

Line 120

This can be written, with shorthand, like so:

nav {
    padding: 10px 0;
}

A good mnemonic to remember is TRBL - or top, right, bottom, left. In shorthand you specify TB and RL when the values are the same.

Line 128-129

This does not do what you think it does; an empty block will not zero out a rule, it will still be applied. If you want to remove the float that you defined earlier in the file do float: none here.

In general

Remember that properties are cascaded down to other elements, so it may turn out that half of these rules that you are writing don't need to be written, as they might already be covered with a selector. Indeed, if you look at lines 15-34 again, you might prefer writing the first selector like so:

nav li.highlight a,
nav li:hover a {
    /* ... */
}

The over-qualification of your selectors should be a pattern to be avoided. Don't make selectors that mirror the DOM; instead use the least tags possible to create your desired effect.

Oh, and this is a point that I'm not sure has anything to do with the refactor; I'm just looking at the end result and you have big left hand side gaps on the menu. I think it would be preferable to reset the padding on the ul itself so that when you hover the list item, the background colour is the full width. So on line 4:

nav ul {
    list-style: none;
    padding-left: 0;
}

I hope this helps.