Setting the position of a navigation highlight
Clash 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>
javascript functional-programming
add a comment |Â
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>
javascript functional-programming
add a comment |Â
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>
javascript functional-programming
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>
javascript functional-programming
asked May 23 at 4:24
Vince
1473
1473
add a comment |Â
add a comment |Â
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.
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
add a comment |Â
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.
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
add a comment |Â
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.
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
add a comment |Â
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.
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.
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
add a comment |Â
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
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
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
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password