Setting the position of a navigation highlight

Question

How can I make my function better and favor functional programming style?

On this single-page site I have a fixed navigation list. Each item in the list has a transparent background and I have a #nav-highlight div that I move with JavaScript and animate with CSS for an interesting effect while scrolling through the page.

Everything works, but I'm not happy with my implementation. There's one part that doesn't make sense without the comments to explain it and I need to use continue and break for it to work this way. I'd prefer to move more toward a functional style, but I couldn't figure out how to do this without a for loop.

Here's the relevant function. You can see the code in context at GitHub and the page is hosted here:

function setHighlightPosition(nav) {
  const documentEnd = document.documentElement.scrollHeight - window.innerHeight;
  const scrollTop = window.scrollY;
  const scrollMiddle = scrollTop + (window.innerHeight / 2);
  const highlight = document.getElementById('nav-highlight');

  let highlightItem = 0;
  for (let i = 0; i < nav.positions.length; i += 1) {
    const item = nav.positions[i];
    if (!item.targeted) continue;

    const target = document.getElementById(item.id);
    const position = target.offsetTop;

    // If the target is close to the top of the window, set the highlighted item
    // and don't check any more.
    if (
      (Math.abs(position - scrollTop) < 25) ||
      (scrollTop < position && position - scrollTop < 50)
    ) {
      highlightItem = i;
      break;
    }

    // Check the position of a target near the middle of the window if there
    // were no targets near the top of the window.
    if (scrollMiddle > position) {
      highlightItem = i;
    }
  }

  if (scrollTop === documentEnd) {
    highlightItem = nav.positions.length - 1;
  }

  highlight.style.transform = `translateY(${nav.positions[highlightItem].position}px)`;
  highlight.dataset.position = highlightItem;
}

Here's the affected HTML

<nav id="menu">
  <ul>
    <li class="menu-handle"><a href="#"><i class="fas fa-bars"></i></a></li>
    <li><a class="top" href="#top"><i class="fas fa-angle-up"></i></a></li>
    <li><a href="#objective">Objective</a></li>
    <li><a href="#bio">Bio</a></li>
    <li><a href="#skills">Skills</a></li>
    <li><a href="#history">History</a></li>
    <li><a href="#certification">Certification</a></li>
    <li><a href="#connect">Connect</a></li>
  </ul>
  <div id="nav-highlight" data-position="0"></div>
</nav>

Answer 1

Refactoring

Disclaimer: Sorry, I didn't have a solid chunk of time to test the code, so please consider it as a direction rather than an exact prescription.

---

It's always possible to break away from loop execution interfering keywords like continue and break. It is also almost always possible to quickly rewrite the code in a "more functional" way. Question is, what exactly is the motivation.

First, in functional world it all about function. And even though my code below does not use function composition, I'm adding getPosition(), isCloseToTop(), and isScrollAfterPosition() to simplify a few conditions. Another benefit of that is that these local functions' names are replacing the ugly comments.

Second, nav.positions.findIndex(...) is used to find out whether there's an item close to top. This is made as an independent step in the code and IMO it's more expressive this way.

If there's no item close to the top of window, we can try to find the last position before the scroll. Again, now it's implemented as a dedicated step (via map() and filter()).

Hopefully, it's now closer to what you wanted the code to be.

function setHighlightPosition(nav) {
  const scrollTop = window.scrollY;
  const scrollMiddle = scrollTop + (window.innerHeight / 2);

  const getPosition = item => document.getElementById(item.id).offsetTop;
  const isCloseToTop = position =>
    Math.abs(position - scrollTop) < 25 ||
    scrollTop < position && position - scrollTop < 50;
  const isScrollAfterPosition = position => scrollMiddle > position;

  const indexOfItemCloseToTop = nav.positions.findIndex(item => item.targeted && isCloseToTop(getPosition(item)));

  if (indexOfItemCloseToTop >= 0) {
    highlightItem = indexOfItemCloseToTop;
  } else {
    const positionsBeforeScroll = nav.positions
      .map((item, itemIndex) => (item.targeted && isScrollAfterPosition(getPosition(item))) ? itemIndex : null)
      .filter(index => index !== null);
    highlightItem = positionsBeforeScroll[positionsBeforeScroll.length - 1];
  }

  const documentEnd = document.documentElement.scrollHeight - window.innerHeight;
  if (scrollTop === documentEnd) {
    highlightItem = nav.positions.length - 1;
  }

  const highlight = document.getElementById('nav-highlight');
  highlight.style.transform = `translateY(${nav.positions[highlightItem].position}px)`;
  highlight.dataset.position = highlightItem;
}

BTW, I moved variable declarations around so that the declarations are located next to their first usages. This reduces brain cycles and eliminates the need of scrolling the code up and down.