Reputation: 27
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
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 = "+" } }
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.
addEventListener
function.target
as data attribute data-target
to your button, i.e data-target='one'
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
Reputation: 19475
You can eliminate lots of mistakes and errors by following best practices:
onclick
attributesstyle
changesAlso, 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
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
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');
Upvotes: 1