0

I'm following the top answer to this post here.

Direct link to the JSFiddle.

I tried implemented the same strategy except one part doesn't work, specifically the last line. My structure is a little different so I had to change it.

Original:

 menuItems
  .parent().removeClass("active")
  .end().filter("[href='#"+id+"']").parent().addClass("active");

I had to get rid of parent() because my setup is a little different, but that breaks it.

 menuItems
  .removeClass("active")
  .end().filter("[href='#"+id+"']").addClass("active");

It half works if I keep the first .parent(), it just doesn't remove the class like I'd like it too. So my question is, what is wrong with the syntax of my version, that does not include .parent()?

Here is my HTML structure:

<div class="content-wrap>
  <a class="side-link>Menu item 1</a>
  <a class="side-link>Menu item 2</a>
  <a class="side-link>Menu item 3</a>
</div>
James Mitchell
  • 2,387
  • 4
  • 29
  • 59

1 Answers1

0

In your jQuery when you are searching for element to highlight you need to step one step up in DOM so you can find sibling elements:

   menuItems
     .removeClass("active").parent()
     .end().filter("[href='#"+id+"']").addClass("active");

Working example:

// Cache selectors
var lastId,
    topMenu = $("#top-menu"),
    topMenuHeight = topMenu.outerHeight()+15,
    // All list items
    menuItems = topMenu.find("a"),
    // Anchors corresponding to menu items
    scrollItems = menuItems.map(function(){
      var item = $($(this).attr("href"));
      if (item.length) { return item; }
    });

// Bind click handler to menu items
// so we can get a fancy scroll animation
menuItems.click(function(e){
  var href = $(this).attr("href"),
      offsetTop = href === "#" ? 0 : $(href).offset().top-topMenuHeight+1;
  $('html, body').stop().animate({ 
      scrollTop: offsetTop
  }, 300);
  e.preventDefault();
});

// Bind to scroll
$(window).scroll(function(){
   // Get container scroll position
   var fromTop = $(this).scrollTop()+topMenuHeight;
   
   // Get id of current scroll item
   var cur = scrollItems.map(function(){
     if ($(this).offset().top < fromTop)
       return this;
   });
   
   // Get the id of the current element
   cur = cur[cur.length-1];
   var id = cur && cur.length ? cur[0].id : "";
   
   if (lastId !== id) {
       lastId = id;
       // Set/remove active class
       menuItems
         .removeClass("active").parent()
         .end().filter("[href='#"+id+"']").addClass("active");
   }                   
});
body {
    height: 6000px;
    font-family: Helvetica, Arial;
}

#top-menu {
    position: fixed;
    z-index: 1;
    background: white;
    left: 0;
    right: 0;
    top: 0;
}


#top-menu a {
    display: block;
    padding: 5px 25px 7px 25px;
    width: 6em;
    text-align: center;
    -webkit-transition: .5s all ease-out;
    -moz-transition: .5s all ease-out;
    transition: .5s all ease-out;
    border-top: 3px solid white;
    color: #aaa;
    text-decoration: none;
    float: left;
}

#top-menu a:hover {
    color: #000;
}

#top-menu a.active {
    border-top: 3px solid #333;
    color: #333;
}

#foo {
    position: absolute;
    top: 0px;
}

#bar {
    position: absolute;
    top: 800px;
}

#baz {
    position: absolute;
    top: 1200px;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="content-wrap" id="top-menu">
  <a class="side-link active" href="#">Menu item 1</a>
  <a class="side-link" href="#bar">Menu item 2</a>
  <a class="side-link" href="#baz">Menu item 3</a>
</div>


<a id="foo">Foo</a>


<a id="bar">Bar</a>


<a id="baz">Baz</a>
Samich
  • 29,157
  • 6
  • 68
  • 77