Setting the position of a navigation highlight

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
3
down vote

favorite












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)

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


highlight.style.transform = `translateY($nav.positions[highlightItem].positionpx)`;
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>






share|improve this question

























    up vote
    3
    down vote

    favorite












    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)

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


    highlight.style.transform = `translateY($nav.positions[highlightItem].positionpx)`;
    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>






    share|improve this question





















      up vote
      3
      down vote

      favorite









      up vote
      3
      down vote

      favorite











      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)

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


      highlight.style.transform = `translateY($nav.positions[highlightItem].positionpx)`;
      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>






      share|improve this 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)

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


      highlight.style.transform = `translateY($nav.positions[highlightItem].positionpx)`;
      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>








      share|improve this question










      share|improve this question




      share|improve this question









      asked May 23 at 4:24









      Vince

      1473




      1473




















          1 Answer
          1






          active

          oldest

          votes

















          up vote
          2
          down vote



          accepted










          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


          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.






          share|improve this answer

















          • 1




            Thank you very much. Your code gave me ideas that helped me to make it much better. My motivation is simple... I want to be a better programmer and this page, more than any project page done for a client, should be an example of my continuous improvement. If your curious, this diff shows the changes I made after learning from your code.
            – Vince
            May 24 at 1:23










          Your Answer




          StackExchange.ifUsing("editor", function ()
          return StackExchange.using("mathjaxEditing", function ()
          StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
          StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
          );
          );
          , "mathjax-editing");

          StackExchange.ifUsing("editor", function ()
          StackExchange.using("externalEditor", function ()
          StackExchange.using("snippets", function ()
          StackExchange.snippets.init();
          );
          );
          , "code-snippets");

          StackExchange.ready(function()
          var channelOptions =
          tags: "".split(" "),
          id: "196"
          ;
          initTagRenderer("".split(" "), "".split(" "), channelOptions);

          StackExchange.using("externalEditor", function()
          // Have to fire editor after snippets, if snippets enabled
          if (StackExchange.settings.snippets.snippetsEnabled)
          StackExchange.using("snippets", function()
          createEditor();
          );

          else
          createEditor();

          );

          function createEditor()
          StackExchange.prepareEditor(
          heartbeatType: 'answer',
          convertImagesToLinks: false,
          noModals: false,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: null,
          bindNavPrevention: true,
          postfix: "",
          onDemand: true,
          discardSelector: ".discard-answer"
          ,immediatelyShowMarkdownHelp:true
          );



          );








           

          draft saved


          draft discarded


















          StackExchange.ready(
          function ()
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f194985%2fsetting-the-position-of-a-navigation-highlight%23new-answer', 'question_page');

          );

          Post as a guest






























          1 Answer
          1






          active

          oldest

          votes








          1 Answer
          1






          active

          oldest

          votes









          active

          oldest

          votes






          active

          oldest

          votes








          up vote
          2
          down vote



          accepted










          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


          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.






          share|improve this answer

















          • 1




            Thank you very much. Your code gave me ideas that helped me to make it much better. My motivation is simple... I want to be a better programmer and this page, more than any project page done for a client, should be an example of my continuous improvement. If your curious, this diff shows the changes I made after learning from your code.
            – Vince
            May 24 at 1:23














          up vote
          2
          down vote



          accepted










          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


          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.






          share|improve this answer

















          • 1




            Thank you very much. Your code gave me ideas that helped me to make it much better. My motivation is simple... I want to be a better programmer and this page, more than any project page done for a client, should be an example of my continuous improvement. If your curious, this diff shows the changes I made after learning from your code.
            – Vince
            May 24 at 1:23












          up vote
          2
          down vote



          accepted







          up vote
          2
          down vote



          accepted






          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


          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.






          share|improve this answer













          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


          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.







          share|improve this answer













          share|improve this answer



          share|improve this answer











          answered May 23 at 7:55









          Igor Soloydenko

          2,697827




          2,697827







          • 1




            Thank you very much. Your code gave me ideas that helped me to make it much better. My motivation is simple... I want to be a better programmer and this page, more than any project page done for a client, should be an example of my continuous improvement. If your curious, this diff shows the changes I made after learning from your code.
            – Vince
            May 24 at 1:23












          • 1




            Thank you very much. Your code gave me ideas that helped me to make it much better. My motivation is simple... I want to be a better programmer and this page, more than any project page done for a client, should be an example of my continuous improvement. If your curious, this diff shows the changes I made after learning from your code.
            – Vince
            May 24 at 1:23







          1




          1




          Thank you very much. Your code gave me ideas that helped me to make it much better. My motivation is simple... I want to be a better programmer and this page, more than any project page done for a client, should be an example of my continuous improvement. If your curious, this diff shows the changes I made after learning from your code.
          – Vince
          May 24 at 1:23




          Thank you very much. Your code gave me ideas that helped me to make it much better. My motivation is simple... I want to be a better programmer and this page, more than any project page done for a client, should be an example of my continuous improvement. If your curious, this diff shows the changes I made after learning from your code.
          – Vince
          May 24 at 1:23












           

          draft saved


          draft discarded


























           


          draft saved


          draft discarded














          StackExchange.ready(
          function ()
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f194985%2fsetting-the-position-of-a-navigation-highlight%23new-answer', 'question_page');

          );

          Post as a guest













































































          Popular posts from this blog

          Greedy Best First Search implementation in Rust

          Function to Return a JSON Like Objects Using VBA Collections and Arrays

          C++11 CLH Lock Implementation