1

I have a simple JavaScript expanding and collapsing text on the click of a button. What I want to do is add a "+" sign next to the button and change it to "-" when the text is collapsed and vice-versa.

This is my HTML:

<button onclick="expand('one')">First text</button><span class="plusminus">+</span>
<div class="text" id="one">
      A bunch of text here
</div>


<p><button onclick="expand('two')">Second text</button><span class="plusminus">+</span>
<div class="text" id="two">
      More text here
</div>


<p><button onclick="expand('three')">Third text</button><span class="plusminus">+</span>
<div class="text" id="three">
      And some text here
</div>

This is my CSS:

.text {
  display: none;
}  

And this is JavaScript:

function expand(textId) {
    var e = document.getElementById(textId);
    var sign = document.getElementsByClassName("plusminus");
    if (e.style.display === "block") {
        e.style.display = "none";
        sign.innerText = "+";
    } else {
        e.style.display = "block";
        sign.innerText = "-";
    }
}

The expand/collapse works, but not changing the sign... What am I doing wrong?

Thanks a lot!

2
  • Several problems here. Firstly, your markup is invalid as you're not closing the p tags. Secondly, your selector is currently selecting ALL plusmin elements, not the one relative to the area being expanded/collapsed. Finally, inline DOM-zero events via HTML attributes are pretty archaic. Consider centralised event handling via addEventListener Commented Feb 7, 2018 at 12:57
  • Thanks for your comments! I think I know how to fix it, will try later. Commented Feb 7, 2018 at 13:06

4 Answers 4

1

First of all, let's improve the code by converting to centralised event handling via addEventListener rather than multiple, inline event attributes muddying the HTML.

In preparation for this, we need to transfer the flag ("one", "two") etc to a data attribute to each button. So the HTML becomes:

<p>
    <button data-which="three">Third text</button>
    <span class="plusminus">+</span>
</p> <!-- <-- note missing closing P tag in your original markup -->

So in an external JS file you'd do something like:

var btns = document.querySelectorAll('button[data-which]');
[].forEach.call(btns, function(btn) {
    btn.addEventListener('click', function() {
        expand(this.getAttribute('data-which'));
    }, false);
});

Your other problem is that the expand() function currently targets ALL plusminus elements, not the one relative to the area being expanded/collapsed. Let's fix that:

var sign = document.querySelector('#'+textId).parentNode.querySelector('.plusminus');

Fiddle

Sign up to request clarification or add additional context in comments.

Comments

0

document.getElementsByClassName returns an array. You are treating it as a single object.

Your implementation of document.getElementsByClassName is not correct, you have multiple spans with same class, you need to pick the correct one using index. keeping mind in your existing code I have done a small correction in your expand function.

function expand(textId, index) {
    var e = document.getElementById(textId);
    var sign =  document.getElementsByClassName("plusminus")[index];
    if (e.style.display === "block") {
        e.style.display = "none";
        sign.innerText = "+";
    } else {
        e.style.display = "block";
        sign.innerText = "-";
    }
}
.text {
  display: none;
}  
<!DOCTYPE html>
<html>

  <head>
   
  </head>

  <body>
    <button onclick="expand('one',0)">First text</button><span class="plusminus">+</span>
<div class="text" id="one">
      A bunch of text here
</div>


<p><button onclick="expand('two',1)">Second text</button><span class="plusminus">+</span>
<div class="text" id="two">
      More text here
</div>


<p><button onclick="expand('three',2)">Third text</button><span class="plusminus">+</span>
<div class="text" id="three">
      And some text here
</div>
  </body>

</html>

Comments

0

You can eliminate lots of mistakes and errors by following best practices:

  • Don’t use onclick attributes
  • Use event delegation
  • Use CSS classes to toggle styles instead of direct style changes

Also, close <p>s properly and keep in mind that <div>s can’t be inside a <p>.

Now, your most important issue was that you misunderstood what getElementsByClassName returns: an HTMLCollection, rather than a single element. innerText doesn’t mean anything on it.

document.addEventListener("click", function expand(e) {
  if (e.target.dataset.expand) {
    let textElement = document.getElementById(e.target.dataset.expand),
      expandIndicator = e.target.parentElement.getElementsByClassName("plusminus")[0];

    expandIndicator.innerText = (textElement.classList.toggle("show")
      ? "-"
      : "+");
  }
});
.text {
  display: none;
}

.show {
  display: block;
}
<p><button data-expand="one">First text</button><span class="plusminus">+</span></p>
<div id="one" class="text">
  A bunch of text here
</div>

<p><button data-expand="two">Second text</button><span class="plusminus">+</span></p>
<div id="two" class="text">
  More text here
</div>

<p><button data-expand="three">Third text</button><span class="plusminus">+</span></p>
<div id="three" class="text">
  And some text here
</div>

Instead of onclick attributes you could use data-* attributes to store the ID of the element you want to show. By event delegation (testing for e.target’s properties), you can select the correct textElement.

To find the correct expandIndicator you can go back to your parent element (<p>) and find an element with class="plusminus" from there. Remember to select the first one ([0]), since you’ll still get an HTMLCollection.

You could alternatively use expandIndicator = e.target.nextElementSibling, if the <span class="plusminus"> always comes after the button — here, the class name doesn’t event matter.

Finally, instead of testing for the style property, consider using class toggling instead. .classList.toggle conveniently returns true or false depending on whether the class has been set or not, which can directly be used in a ternary expression (… ?: …).

Comments

0
  • You can use the list of class classList to check if class text was applied to the element.

  • Use the nextElementSibling to get the minus/plus element.

function expand(button, textId) {
  var e = document.getElementById(textId);
  if (e.classList.contains('text')) {
    e.classList.remove('text');
    button.nextElementSibling.innerText = "-"
  } else {
    e.classList.add('text');
    button.nextElementSibling.innerText = "+"
  }
}

Look at this code snippet

function expand(button, textId) {
  var e = document.getElementById(textId);
  if (e.classList.contains('text')) {
    e.classList.remove('text');
    button.nextElementSibling.innerText = "-"
  } else {
    e.classList.add('text');
    button.nextElementSibling.innerText = "+"
  }
}
.text {
  display: none;
}
<button onclick="expand(this, 'one')">First text</button><span class="plusminus">+</span>
<div class="text" id="one">
  A bunch of text here
</div>
<p/>
<button onclick="expand(this, 'two')">Second text</button><span class="plusminus">+</span>
<div class="text" id="two">
  More text here
</div>
<p/>

<button onclick="expand(this, 'three')">Third text</button><span class="plusminus">+</span>
<div class="text" id="three">
  And some text here
</div>

See? now, your minus/plus element is changing.

Better approach

  • Bind a click event to the button using addEventListener function.
  • Add a target as data attribute data-target to your button, i.e data-target='one'

Look at this code snippet

document.querySelectorAll('button').forEach(function(b) {
  b.addEventListener('click', function(e) {
    expand(e.currentTarget, e.currentTarget.dataset.target);
  });
});


function expand(button, textId) {
  var e = document.getElementById(textId);
  if (e.classList.contains('text')) {
    e.classList.remove('text');
    button.nextElementSibling.innerText = "-"
  } else {
    e.classList.add('text');
    button.nextElementSibling.innerText = "+"
  }
}
.text {
  display: none;
}
<button data-target='one'>First text</button><span class="plusminus">+</span>
<div class="text" id="one">
  A bunch of text here
</div>
<p/>
<button data-target='two'>Second text</button><span class="plusminus">+</span>
<div class="text" id="two">
  More text here
</div>
<p/>

<button data-target='three'>Third text</button><span class="plusminus">+</span>
<div class="text" id="three">
  And some text here
</div>

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.