0

I am struggling to fully grasp how I can make my JavaScript DRY.

Right now I am naming each id with the addition of a number to keep it unique and then repeating the same code just replacing the id. This is awful.

I want to sick with Vanilla JS rather than jQuery. I have tried to understand this but I'm finding it difficult to understand how it's used in regards to the code examples.

I was hoping the community could help with this basic example that I understand when non-DRY, so could follow when it's made DRY much better.

Thanks!

const content1 = document.getElementById("content1")
const trigger1 = document.getElementById("trigger1")

trigger1.addEventListener("click", () => {
  if(content1.getAttribute("data-toggle") === "closed") {
    content1.setAttribute("data-toggle", "open")
    content1.classList.remove("hide")
  } else {
    content1.setAttribute("data-toggle", "closed")
    content1.classList.add("hide")  
  }
})

const content2 = document.getElementById("content2")
const trigger2 = document.getElementById("trigger2")

trigger2.addEventListener("click", () => {
  if(content2.getAttribute("data-toggle") === "closed") {
    content2.setAttribute("data-toggle", "open")
    content2.classList.remove("hide")
  } else {
    content2.setAttribute("data-toggle", "closed")
    content2.classList.add("hide")  
  }
})


const content3 = document.getElementById("content3")
const trigger3 = document.getElementById("trigger3")

trigger3.addEventListener("click", () => {
  if(content3.getAttribute("data-toggle") === "closed") {
    content3.setAttribute("data-toggle", "open")
    content3.classList.remove("hide")
  } else {
    content3.setAttribute("data-toggle", "closed")
    content3.classList.add("hide")  
  }
})
.hide {
 display:none;
}

.open {
  padding: 8px;
  background-color:blue;
  margin: 20px;
}
<div class="open" id="trigger1">
  Open 1
</div>

<div class="content-1 hide" id="content1" data-toggle="closed">
  Content 1
</div>

<div class="open" id="trigger2">
  Open 2
</div>


<div class="content-2 hide" id="content2" data-toggle="closed">
  Content 2
</div>

<div class="open" id="trigger3">
  Open 3
</div>


<div class="content-3 hide" id="content3" data-toggle="closed">
  Content 3
</div>
GoldenGonaz
  • 1,116
  • 2
  • 17
  • 42
  • 1
    You could look at creating your DOM (elements) through JavaScript rather than with HTML. Then you might not even need to assign id attributes at all - or if you do, you can generate them automatically. – eddiewould Aug 15 '18 at 12:50
  • 1
    i.e. `document.createElement(), element.appendChild()` etc – eddiewould Aug 15 '18 at 12:51
  • You could create one event handler instead of redefining (nearly) the same anonymous function multiple times. [Here's an example](https://stackoverflow.com/a/27781016/1288) that shows how to pass parameters to an even handler, if that's where you're stuck. – Bill the Lizard Aug 15 '18 at 12:55

2 Answers2

1

Just use a data attribute to target what you are going to toggle and loop over to add the event listeners.

document.querySelectorAll('[data-toggles]').forEach(function(elem) {
  elem.addEventListener('click', function() {
    var id = this.dataset.toggles;
    document.getElementById(id).classList.toggle('hidden')
  })
})
.hidden {
  display: none;
}
<div data-toggles="content1">
  Open 1
</div>

<div class="hidden" id="content1">
  Content 1
</div>


<div data-toggles="content2">
  Open 2
</div>

<div class="hidden" id="content2">
  Content 2
</div>

You could also use event delegation to add one handler, but do not think it is very beneficial in this case.

epascarello
  • 204,599
  • 20
  • 195
  • 236
0

How about looping through your IDs?

var ids = [1, 2, 3];
ids.forEach(function(contentId) {
    var content = document.getElementById("content" + contentId);
    var trigger = document.getElementById("trigger" + contentId);
    trigger.addEventListener("click", () => {
        if(content.getAttribute("data-toggle") === "closed") {
            content.setAttribute("data-toggle", "open");
            content.classList.remove("hide");
        } else {
            content.setAttribute("data-toggle", "closed");
            content.classList.add("hide");
        }
    });
});
wholevinski
  • 3,658
  • 17
  • 23