Reputation: 69
this is html
i'm a complete beginner as i started learning js since last two month, please help me to solve this problem
<h1>Best Song Collection</h1>
<div class="songItem">
<span class="songName">love you zindagi</span>
<span class="btn"><i class="far fa-play-circle playbtn"></i></span>
<span class="btn"><i class="far fa-pause-circle pausebtn"></i></span>
</div>
<div class="songItem">
<span class="songName">love you zindagi</span>
<span class="btn"><i class="far fa-play-circle playbtn"></i></span>
<span class="btn"><i class="far fa-pause-circle pausebtn"></i></span>
</div>
</div>
</div>
js
let pausebtn = document.querySelector(".pausebtn");
let playbtn = document.querySelector(".playbtn")
let btn = document.querySelectorAll(".btn");
function change(element){
if(element.classList==="fa-play-circle"){
element.classList.remove("fa-play-circle");
element.classList.add("fa-pause-circle");
}
}
btn.addEventListner('click',change());
Upvotes: 3
Views: 2760
Reputation: 129
First of all, if you pass a callback function, do not call it. There you need to do it as so btn.addEventListner('click', change);
. (Also, there is a typo in addEventListener)
Second of all, I would change the logic of both your HTML and JS. There is no need to keep two spans inside each .songItem div
, you can keep only one and just change the class that is responsible for the icon when a user clicks on the button. You will have less code and it will be more readable. Also, you don't need to put a i
tag inside a span, you can pass the icons class directly to the span. What is more, it is more convenient to use const
instead of let
, because you do not intend to change the value of the variables.
You can achieve it by the code written below, I also attach a codepen with a working example.
const pauseIconClassName = 'fa-pause-circle'
const playIconClassName = 'fa-play-circle'
const btns = document.querySelectorAll('.btn')
function onChange (event) {
// get the button elememt from the event
const buttonElement = event.currentTarget
// check if play button class is present on our button
const isPlayButton = buttonElement.classList.contains(playIconClassName)
// if a play button, remove the play button class and add pause button class
if (isPlayButton) {
buttonElement.classList.remove(playIconClassName)
buttonElement.classList.add(pauseIconClassName)
// if a pause button, remove pause button class and add play button class
} else {
buttonElement.classList.remove(pauseIconClassName)
buttonElement.classList.add(playIconClassName)
}
// You can also use .toggle function on classList as mentioned by the person in other answer
}
// query selector all returns a list of nodes, therefore we need to iterate over it and attach an event listener to each button seperatly
btns.forEach(btn => {
btn.addEventListener('click', onChange)
})
<link href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.0.0-beta3/css/all.min.css" rel="stylesheet"/>
<h1>Best Song Collection</h1>
<div class="songItem">
<span class="songName">love you zindagi</span>
<span class="btn far fa-play-circle"></span>
</div>
<div class="songItem">
<span class="songName">love you zindagi</span>
<span class="btn far fa-play-circle"></span>
</div>
Upvotes: 3
Reputation: 13222
You probably want to toggle the button so I made an example for that. When you press the play button it will show the pause and when you press the pause button it shows play.
When the button is clicked both fa-play-circle
and fa-pause-circle
are toggled to alter the button icon when clicked.
You made a few mistakes in your code.
addEventListner()
contains a typo. It should be addEventListener()
document.querySelectorAll(".btn").forEach(element => element.addEventListener('click', (event) => {
let iElement = event.currentTarget.querySelector('i');
iElement.classList.toggle("fa-play-circle");
iElement.classList.toggle("fa-pause-circle");
}));
<link href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.0.0-beta3/css/all.min.css" rel="stylesheet"/>
<div class="songItem">
<span class="songName">love you zindagi</span>
<span class="btn"><i class="far fa-play-circle playbtn"></i></span>
</div>
<div class="songItem">
<span class="songName">love you zindagi</span>
<span class="btn"><i class="far fa-play-circle playbtn"></i></span>
</div>
Upvotes: 2
Reputation: 459
At first glance, it looks like a syntax issue. Try to not invoke a function and as args, you should receive an event. So it will look something like this:
let pausebtn = document.querySelector(".pausebtn");
let playbtn = document.querySelector(".playbtn")
let btn = document.querySelectorAll(".btn");
function change(event){
if(event.target.classList==="fa-play-circle"){
event.target.classList.remove("fa-play-circle");
event.target.classList.add("fa-pause-circle");
}
}
btn.addEventListner('click', change);
EDIT: In HTML you have both buttons for play and pause, you should have just one and change the icon with js toggle.
Semantic tip, use button tag for buttons.
Upvotes: 0
Reputation: 10111
pass change to click listener, don't call change function.
btn.addEventListner('click', change);
const pausebtn = document.querySelector(".pausebtn");
const playbtn = document.querySelector(".playbtn");
const btn = document.querySelectorAll(".btn");
function change(element) {
if (element.classList === "fa-play-circle") {
element.classList.remove("fa-play-circle");
element.classList.add("fa-pause-circle");
}
}
btn.addEventListner("click", change);
Upvotes: 0