Jquery Function only working correctly first click

Question

I have this function for playing music and pausing music. It is just simple and light weight for small samples. I just wanted it to have play and stop. It seems to work fine, but the second time you play it, it doesn't switch the classes so pausing can take place.

Here is the jquery;

        var isPlaying = false;
            function playMusic(){
                    var audioElement = document.createElement('audio');
                    var audioElementSrc = $(this).attr('data-audio-src');               

                    if (isPlaying != true)
                    {
                        isPlaying = true;
                        audioElement.setAttribute('src', audioElementSrc);
                        $.get();
                        audioElement.addEventListener("loadeddata", function(){
                        audioElement.play();                
                        }, true);
                         audioElement.addEventListener("ended", function() {
                         isPlaying = false;    
                        });  
                        $(this).addClass('pause');
                        $(this).removeClass('play'); 
                        $('.pause').click(function() {
                        audioElement.pause();
                        isPlaying = false
                        $(this).removeClass('pause');
                        $(this).addClass('play');
                        });             
                    }
                    else
                    {
                        return false;
                    }           
                }

            $(function(e){
                $('.play').click(playMusic);  
            });

jsfiddle Here with everything. You can see the stop appear, then click it and when you play again it doesn't swap the classes.

Why do you use addEventListener when you have .on and why add inside the click? — mplungjan, Nov 09 '14 at 20:36

score 2 · Accepted Answer · answered Nov 09 '14 at 20:45

You've managed to complicate this a lot with the plain javascript function and replacing the audio element every time the play button is clicked.

Use a little more jQuery, and store the audio element, and it's a lot easier

$(function (e) {
    $('.play').click(function() {
        var self = $(this).toggleClass('pause play');

        if (!this.audio) {
            this.audio = $('<audio />', {src : self.data('audio-src')}).on({
                loadeddata : function() {
                    this.play();
                },
                ended : function() {
                    self.toggleClass('pause play');
                }
            }).get(0);
        }else{
            this.audio[this.audio.paused ? 'play' : 'pause']();
        }
    });
});

FIDDLE

Wow! Yes I started from a totally different direction, and if I would have just kept jquery and the end need in mind it would have been much easier! Thank You! — dimmlight, Nov 09 '14 at 20:51

Ernesto · Answer 2 · 2014-11-09T20:50:56.313

0

you should not do the

$('.pause').click(function() {
   audioElement.pause();
   isPlaying = false
   $(this).removeClass('pause');
   $(this).addClass('play');
   });

code there, you should add the pause at the beginign just one time with a delegate like

$(".songPlayer").delegate('.pause','click', function());

the problem here is taht the event on pause is getting calling a lot of times because is getting attached one time per click

edited Nov 09 '14 at 20:50

answered Nov 09 '14 at 20:35

Ernesto

301
1
6

.on is delegating just fine. .delegate is deprecated and you have too many dots in the selector – mplungjan Nov 09 '14 at 20:36
yeah he can use .on too but, http://api.jquery.com/delegate/ <--i dont know where it says that delegate is deprecated, and i didn't realize my dot after de "songplayer" sorry – Ernesto Nov 09 '14 at 20:50
Ah you are correct, I am surprised. Anyway since 1.7 it is on: http://stackoverflow.com/questions/8359085/delegate-vs-on – mplungjan Nov 10 '14 at 05:31

score 0 · Answer 3 · answered Nov 09 '14 at 20:42

I noticed this in the play code:

$('.pause').click(function() {
  audioElement.pause();
  isPlaying = false
  $(this).removeClass('pause');
  $(this).addClass('play');
});

This code changes an element with class .pause, but that is overriding the function that it is in! Also your main if statement looks like this:

if (isPlaying != true)
{
// Play code here           
}
else
{
return false;
}

But what you need is this:

    if (isPlaying != true)
    {
    // Play code here           
    }
    else
    {
    // Pause code here
    }

So your new code would look like this:

 var isPlaying = false;
            function playMusic(){
                    var audioElement = document.createElement('audio');
                    var audioElementSrc = $(this).attr('data-audio-src');               

                    if (isPlaying != true)
                    {
                        isPlaying = true;
                        audioElement.setAttribute('src', audioElementSrc);
                        $.get();
                        audioElement.addEventListener("loadeddata", function(){
                        audioElement.play();                
                        }, true);
                         audioElement.addEventListener("ended", function() {
                         isPlaying = false;    
                        });  
                        $(this).addClass('pause');
                        $(this).removeClass('play');            
                    }
                    else
                    {
                        audioElement.pause(); // This part moved from above
                        isPlaying = false
                        $(this).removeClass('pause');
                        $(this).addClass('play');
                    }           
                }

            $(function(e){
                $('.play').click(playMusic);  
            });

Jquery Function only working correctly first click

3 Answers3