downer
downer

Reputation: 27

Change text on the click of a button, JavaScript

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!

Upvotes: 1

Views: 2093

Answers (4)

Ele
Ele

Reputation: 33726

  • 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>

Upvotes: 0

Sebastian Simon
Sebastian Simon

Reputation: 19475

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 (… ?: …).

Upvotes: 0

PSK
PSK

Reputation: 17943

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>

Upvotes: 0

Mitya
Mitya

Reputation: 34556

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

Upvotes: 1

Related Questions