0

This is the first time I’ve thought about moving my events outside of the normal HTML onClick=”” event but I cant seem to find any references as to how I would do this with a li list.

Basically I’m trying to get the number associated with the scrollToArtical(#) in to myElement.onclick. How would you rewrite this so that the event is in the .js file.

I’ve tried variations of to get at the element but these don’t work:

var objScrollToNav = document.getElementById("id_ScrollToNav").children;
var objScrollToNav = document.querySelector("#id_ScrollToNav a"); 

Any help would be greatly appreciated – CES My old code is:

<ul id="id_ScrollToNav" role="list">
    <li class="sectionNavOff"><a onclick="scrollToArticle(0)" role="link">•</a></li>
    <li class="sectionNavOn"><a onclick="scrollToArticle(1)" role="link">•</a></li>
    <li class="sectionNavOff"><a onclick="scrollToArticle(2)" role="link">•</a></li>
</ul>
Brian Tompsett - 汤莱恩
  • 5,195
  • 62
  • 50
  • 120
CES
  • 71
  • 3

3 Answers3

1

Use document.querySelectorAll to get an array-like list, then loop over them. To keep a reference to the index, make sure you also pass the index into a new closure (the addEvent function below creates a new closure).

function scrollToArticle(index) { console.log("Scrolling to:", index); }

// Select all the elements.
var links = document.querySelectorAll("#id_ScrollToNav a");

// This function adds event listener, and holds a reference to the index.
function addEvent(el, index) {
  el.addEventListener("click", function() {
    scrollToArticle(index);
  });
}

// Loop over the elements.
for (var i = 0; i < links.length; i++) {
  addEvent(links[i], i);
}
<ul id="id_ScrollToNav" role="list">
    <li class="sectionNavOff"><a role="link">•</a></li>
    <li class="sectionNavOn"><a role="link">•</a></li>
    <li class="sectionNavOff"><a role="link">•</a></li>
</ul>
Alexander O'Mara
  • 52,993
  • 16
  • 139
  • 151
1

Since your li elements can be gathered up into an array and arrays have indexes, you really don't need to pass a hard-coded number to your function. You can just pass the index of the li that is being clicked to the function.

Also, don't use <a> elements when they are not directly navigating you anywhere. This can cause problems for people who use screen readers. Instead, set up the click event directly on the li elements and eliminate the a elements completely.

Lastly, don't use inline HTML event attributes (onclick). That is how we did event handlers 20 years ago and, unfortunately, this technique just won't die. There are many reasons not to use them. Instead, follow modern standards and separate your JavaScript from your HTML.

// Get all the li elements into an array
var items = Array.prototype.slice.call(document.querySelectorAll("#id_ScrollToNav > li"));

// Loop over the list items
items.forEach(function(item, index){
  // Assign each item a click event handler that uses the index of the current item
  item.addEventListener("click", function(){ scrollToArticle(index) });
});

// Just for testing
function scrollToArticle(articleNumber){
 console.log(articleNumber);
}
#id_ScrollToNav > li {
  cursor:pointer;
}
<ul id="id_ScrollToNav" role="list">
  <li class="sectionNavOff" role="link">•</li>
  <li class="sectionNavOn" role="link">•</li>
  <li class="sectionNavOff" role="link">•</li>
</ul>
Scott Marcus
  • 57,085
  • 6
  • 34
  • 54
  • I like the answer. What about @Andreas proposal for event delegation? It's even easier now that you look up for children, not children of children. – RaphaMex Dec 13 '17 at 19:51
  • @RaphaMex I don't see how delegation would help here at all. We know exactly what elements need event handlers. Delegation is for times when it's not entirely clear which items need event handling. – Scott Marcus Dec 13 '17 at 19:56
  • I was thinking that if CES wanted now to create hundreds of sections, or have dynamic addition of sections, delegation would help. Yet, it's more about what I want, because CES did not ask for it :-) – RaphaMex Dec 13 '17 at 20:04
  • @ScottMarcus Event delegation is not only for a dynamically changing number of elements. It can also reduce the number of added event handlers. _n_ handlers for _n_ elements w/o vs _one_ handler for _n_ elements with event delegation. – Andreas Dec 13 '17 at 21:06
  • @Andreas True, but this use case doesn't beg for that solution. While not a bad idea, it's not directly relevant to the question or the answer. – Scott Marcus Dec 13 '17 at 21:07
0

To add to the above, use data- attributes to separate css styles from javascript (meaning, html class tags should be used for html/css things only).

<li data-element="sectionNavOff">
<li data-element="sectionNavOn">

There are some minor downsides to using data- tags, mainly speed, but many enterprise applications and frameworks (e.g. Bootstrap) tend to believe the upside to separating styles from javascript completely outweighs the downsides. If I knew whether or not you use jQuery I could give you a detailed usage example.

KayakinKoder
  • 2,312
  • 1
  • 16
  • 29