Sliding marker on link hover with jQuery

Question

The following code is for a vertical menu, where I needed a marker to slide down next to a link upon hovering over said link. Therefore I needed to change CSS properties of a HTML element (the top-margin in this case).

What I've done in jQuery is add a class ('.movedown1' or '.movedown2') to the marker div ('#slide') when the mouse hovers over one of the links ('#menulink2' or '#menulink3'), and remove the class when the mouse stops hovering over the link.

It works!...But is it a clean method to add/change CSS properties of a HTML element? Does it follow best practices for jQuery? I've only used jQuery a few times in the past so I'm not familiar with best practices.

Codepen

HTML(Pug):

#slide

#menu
  a.menulink#menulink1(href="#")
    p RED
  a.menulink#menulink2(href="#")
    p BLUE
  a.menulink#menulink3(href="#")
    p GREEN

CSS (Sass)

a
  color: white
  text-decoration: none

#slide
  width: 10px
  height: 30px
  background-color: red
  position: absolute
  border-radius: 15px
  transition: 0.3s ease-in-out


.movedown1
  margin-top: 40px
  background-color: blue !important

.movedown2
  margin-top: 80px
  background-color: green !important

#menu
  margin-left: 30px

.menulink
  height: 40px
  width: 70px
  display: block

.menulink p
  height: 30px
  width: 70px
  margin: 0px
  border-radius: 15px
  display: flex
  align-items: center
  justify-content: center

#menulink1 p
  background: red
#menulink2 p
  background: blue
#menulink3 p
  background: green

jQuery

$('#menulink2').hover(
  function(){
  $('#slide').addClass('movedown1');
  },
  function(){
  $('#slide').removeClass('movedown1')  

});

$('#menulink3').hover(
  function(){
  $('#slide').addClass('movedown2');
  },
  function(){
  $('#slide').removeClass('movedown2')  

});

Answer 1

JavaScript

This code violates the Don't Repeat Yourself principle because there is some redundancy in the code to handle the hover events, but for a beginner it might be challenging to know how to abstract the common code.

One way to reduce the redundancy is to define a mapping of id values to classes to toggle. This can be done with a plain-old JavaScript Object, like below, or with a Map() object.

const targetClassMapping = {
  'menulink2': 'movedown1',
  'menulink3': 'movedown2'
};

Then the code can utilize .hover() on any element with the class name menulink with a single event handler that checks the id attribute (using the jQuery .attr() method) of the event target and if the id matches a key in that mapping it then calls .toggleClass() on the slider element. Additionally, $('slider') can be stored in a variable and used when needed instead of querying the DOM each time.

const slide = $('#slide');
$('.menulink').hover(function(){
  const id = $(this).attr('id');
  if (id && id in targetClassMapping) {
    slide.toggleClass(targetClassMapping[id]);
  }
});

This could also be done without jQuery, though the code wouldn't be quite as succinct.

const slider = document.getElementById('slide');
const targetClassMapping = {
  'menulink2': 'movedown1',
  'menulink3': 'movedown2'
};
['mouseover', 'mouseout'].forEach(type => document.addEventListener(type, handleMouseEventOnMenuLink));
function handleMouseEventOnMenuLink(event) {
  if (event.target.parentNode) {
    const id = event.target.parentNode.id;
    if (id in targetClassMapping) {
      const method = event.type === 'mouseover' ? 'add' : 'remove';
      slider.classList[method](targetClassMapping[id]);
    }
  }
} 

CSS / SASS

I see a couple lines with !important.

.movedown1
  margin-top: 40px
  background-color: blue !important

.movedown2
  margin-top: 80px
  background-color: green !important

It is wise to not depend on !important if possible, to avoid confusion. You can remove the !important from those background color styles if you make the selectors more specific than just #slide - e.g. #slide.movedown1 and #slide.movedown2.

Answer 2

It is possible by changing the location of your #slide element so that it is a sibling of menulink. I would also remove the paragraph tags as they are redundant and reduce the dependence on id tags. Rsather use classes.

Here is how I would implement it (though you could also use a flex layout)

#menu
  a.menulink(href="#") RED
  a.menulink(href="#") BLUE
  a.menulink(href="#") GREEN
  .slide

.slide
  width: 10px
  height: 30px
  background-color: red
  position: absolute
  top: 8px
  border-radius: 15px
  transition: 0.3s ease-in-out

.menulink:hover:nth-child(2) ~ .slide
  margin-top: 40px
  background-color: blue

.menulink:hover:nth-child(3) ~ .slide
  margin-top: 80px
  background-color: green

#menu
  margin-left: 30px

.menulink
  display: block
  margin: 5px 16px
  width: 70px
  display: block
  height: 30px
  line-height: 30px
  border-radius: 15px
  color: white
  text-decoration: none
  text-align: center

.menulink:nth-child(1)
  background: red
.menulink:nth-child(2)
  background: blue
.menulink:nth-child(3)
  background: green
```