Audio player for a website

Question

I'm fairly new to jQuery and have decided to make a jQuery audio player for a website.

There are two versions of the way it selects the audio:

1. Here
2. Here

I removed some basic, not connected with the player functions. What do you think of it?

$(document).ready(function() {



  function secondsTimeSpanToHMS(s) {
         // this function is not mine
    var m = Math.floor(s / 60); //Get remaining minutes
    s -= m * 60;
    return (m < 10 ? '0' + m : m) + ":" + (s < 10 ? '0' + Math.floor(s) : Math.floor(s)); //zero padding on minutes and seconds
  }



  // INDEX PLAYER

  $('.play').click(function() {




    var $this = $(this);
    var $master = $('.master');
    var id = $this.attr('id').replace(/btn/, '');

    $(".play").not(this).not('.master').each(function() { 
      if ($(this).hasClass("active"))
        $(this).click();
    });

    $this.toggleClass('active');

    var audio = $('audio[id^="sound"]')[id - 1];


    if ($this.hasClass('active')) {

      $master.attr('id', $(this).attr('id'));


      $this.addClass('playing');

      audio.play();

      $('#songlength').text(secondsTimeSpanToHMS(audio.duration));



      if (!$this.hasClass("master")) {
        if (!$(".master").hasClass("playing")){
          $master.addClass("playing");
          $master.addClass("active");

      }




      audio.ontimeupdate = function() {  
        $('.progress').css("width", (audio.currentTime / audio.duration * 100 + '%'));

        $('#duration').text(secondsTimeSpanToHMS(audio.currentTime));
      }

    } else {    
      $this.removeClass('playing');  
      audio.pause();

        if (!$this.hasClass("master")) {  
          if ($(".master").hasClass("playing"))
            $master.removeClass("playing"); 
        }
    }



    $('#slider').bind('click', function(ev) {

      var $div = $(ev.target);
      var $display = $div.find('#progcont');
      var vW = $(window).width(); 
      var pW = $("#progcont").width();
      var offset = $div.offset();
      var x = ev.clientX - offset.left;
      var ProcRatio = x / pW;
      audio.currentTime = ProcRatio * audio.duration;

    });



  });


  //INDEX.HTML BIG PLAY BUTTON


  $(".master").click(function() {
    var $this = $(this);
    var ID = $this.attr('id');
    console.log(ID);

    if ($this.hasClass("playing")) {
      console.log("ZROBI PLAY");
      $(".play").not(this).each(function() {
        if ($(this).attr("id") == ID)
          $(this).click();
      });

    } else {
      console.log("ZROBI PAUSE");
      $(".play").not(this).each(function() {
        if ($(this).hasClass('active')) {
          $(this).click();
        }
      });
    }
  });


  //PLAY ON MUZYKA
  $('.play2').click(function() {



    var $this = $(this);
    var audio = $(this).find('audio').get(0);



    $(".play2").not(this).each(function() {
      if ($(this).hasClass("active"))
        $(this).click();
    });

    $this.toggleClass('active');
    $('#bottomPlayer').css('bottom', '0px');


    if ($this.hasClass('active')) { //LAUNCHES THE TRACK



      $this.delay(6).addClass('playing');


      audio.play();
      audio.currentTime = 0;
      $('#songlength').text(secondsTimeSpanToHMS(audio.duration));

      audio.ontimeupdate = function() {
        $('.progress').css("width", audio.currentTime / audio.duration * 100 + '%');

        $('#duration').text(secondsTimeSpanToHMS(audio.currentTime));
      }

    } else {
      $this.removeClass('playing');
      audio.pause();
      $('#bottomPlayer').css('bottom', '-50px');
    }




    $('#slider').bind('click', function(ev) {

      var $div = $(ev.target);
      var $display = $div.find('#progcont');
      var vW = $(window).width();
      var pW = $("#progcont").width();
      var offset = $div.offset();
      var x = ev.clientX - offset.left;
      var ProcRatio = x / pW;

      audio.currentTime = ProcRatio * audio.duration;

    });


  });


  $('#pauseAll').click(function() {
    $(".play2").each(function() {
      if ($(this).hasClass("active"))
        $(this).click();
    });
    $('#bottomPlayer').css('bottom', '-50px');
  });



});

Answer 1

There are some problems with style and consistency. The indentation is all over the place and there is too much whitespace. Quotes are inconsistent; choose single or double quotes and stick to one. Some one-line only statements have braces, and others don't, better to always add the braces. You are relying on type casting here:

var id = $this.attr('id').replace(/btn/, ''); // `id` is a string
...
$('audio[id^="sound"]')[id - 1]; // but you use it as a number

An id is unique in the page, so this selector is overspecified:

var $display = $div.find('#progcont'); // same as `$('#progcont')`

PascalCase is the convention for JavaScript constructors, so this is confusing:

var ProcRatio = x / pW;

I'd suggest not wrapping your objects in jQuery when it is not necessary, like in this case:

if ($(this).attr("id") == ID) // same as `if (this.id === ID)`

Or this case:

$master.attr('id', $(this).attr('id')); // same as `$master.attr('id', this.id)`

Don't use bind in jQuery because it is deprecated, use on instead:

$('#slider').bind('click', function(ev) {

I don't think this works as you think it does:

$this.delay(6).addClass('playing');

delay works if there is a queue already, but there isn't one.

Avoid changing CSS in JS whenever possible. For example, you are repeating this code twice:

$('#bottomPlayer').css('bottom', '-50px');

Instead, create a class and toggle it on and off to make it reusable.

Answer 2

Your ternary looks funny, I'd personally declare a function to do the zero padding more readably:

function zeroPad(num){
    return num < 10 ? '0' + num : num
}

...

return zeroPad(m) + ":" + zeroPad(Math.floor(s))

Answer 3

Thoughts and ideas ...

Major ideas

• Currently confusing because $('.play') includes the $(".master") button. Yet there are click handlers for both $('.play') and $(".master"). It would make more sense to make the master button a proxy for the currently active play button.
• Association of elements by id is a bad idea - if possible, traverse the DOM to find audio element relative to its play button (as you do with the play2 buttons but not the play buttons).
• $master.attr('id', $(this).attr('id')) is illegal. No two elements should have the same id. May well cause a problem. (However not relevant if proxy idea is adopted).
• When attaching event handlers inside event handlers - ensure there's no accumulation of effect by calling .off() before .on(), eg $("#slider").off('click').on('click', ...);
• .delay(6) looks like a dangerous workaround. Is the delay correct in all browsers on all computers all of the time?
• The ontimeupdate handlers are never detached, suggesting that they can be attached once, at the outset, to all audio elements. Properly phrased, a single handler should suffice.
• Avoid bulky code repetition by creating utility functions, eg a setAudioTime() function.
• Classes 'active' and 'playing' need to be added/removed independently of each other, or there's not point having both. Suggest adding "active" unconditionally and toggling "playing".
• Should clicks on the $('.play') buttons affect the state of the $('.play2') buttons/audio and vice versa?

Small ideas

• Brackets and braces out of balance. Assumption needed.
• Avoid bulky tests by better exploiting jQuery selectors.
• .removeClass()'s can address multiple classes in one hit.
• Use .on() not .bind().

$(document).ready(function() {
    // *** Utility functions ***
    function secondsTimeSpanToHMS(s) {
        // this function is not mine
        var m = Math.floor(s / 60); //Get remaining minutes
        s -= m * 60;
        return (m < 10 ? '0' + m : m) + ":" + (s < 10 ? '0' + Math.floor(s) : Math.floor(s)); //zero padding on minutes and seconds
    }
    function setAudioTime(ev) {
        this.currentTime = this.duration * (ev.clientX - $(ev.target).offset().left) / $("#progcont").width();
    };

    // INDEX PLAYER
    $('.play').not(".master").on('click', function() {
        var $this = $(this);
        // Make the master a proxy for this play button
        var $master = $('.master').off('click').on('click', function() {
            $this.click();
        }).addClass('active');
        //stop currently playing audio and remove classes playing/active from all play buttons, exp this and master.
        $(".play").not(this).not($master).find(".playing").click().end().removeClass('playing active'); // probably?
        /* Here, control state of .play2 buttons/audio ??? */  
        $this.addClass('active').toggleClass('playing');
        var id = $this.attr('id').replace(/btn/, '');
        var audio = $('audio[id^="sound"]')[id - 1];
        if ($this.is('.playing')) {
            audio.play();
            $master.addClass('playing');
            $('#songlength').text(secondsTimeSpanToHMS(audio.duration));
        } else {
            audio.pause();
            $master.removeClass('playing');
        }
        $('#slider').off('click').on('click', setAudioTime.bind(audio));
    });

    //INDEX.HTML BIG PLAY BUTTON
    //All this code disappears because the master is now a proxy for the currently active play button.

    //PLAY ON MUZYKA
    $('.play2').on('click', function() {
        var $this = $(this);
        var audio = $this.find('audio').get(0);
        $(".play2").not(this).find(".playing").click().end().removeClass('playing active'); // probably?
        /* Here, control state of .play buttons/audio ??? */  
        $this.addClass('active').toggleClass('playing');
        $('#bottomPlayer').css('bottom', '0px');
        if ($this.hasClass('playing')) { //LAUNCHES THE TRACK
            audio.play();
            audio.currentTime = 0;
            $('#songlength').text(secondsTimeSpanToHMS(audio.duration));
        } else {
            $this.removeClass('playing');
            audio.pause();
            $('#bottomPlayer').css('bottom', '-50px');
        }
        $('#slider').off('click').on('click', setAudioTime.bind(audio));
    });
    $('#pauseAll').on('click', function() {
        $(".play2.playing").click();
    });
    $('audio[id^="sound"]').add('.play2 audio').get().ontimeupdate = function setProgess() {
        $('.progress').css('width', (this.currentTime / this.duration * 100 + '%'));
        $('#duration').text(secondsTimeSpanToHMS(this.currentTime));
    };
});

Not sure this is 100% correct. Heaps af assumptions were necessary, so will almost undoubtedly need debugging.

You may well choose to adopt some of these ideas and not others.

Answer 4

A few things not pointed out in the above answers:

1) You should place your code in a closure by creating an IIFE. This will create a private scope for your code so that you don't pollute the global scope.

(function( $ ) {
  //code here
})( jQuery );

In this example, I pass $ in so that jQuery can be safely referenced using it throughout your code.

2) Use 'use strict';. Strict mode helps prevent a lot of common issues.

(function( $ ) {
  'use strict';
  //code here
})( jQuery );

3) Cache your jQuery selections. Selecting elements is one of the slowest things that jQuery does. So creating variables at the top of your code to hold all of your selections. That way you can refer to this variable everywhere in your code.

(function( $ ) {
  'use strict';
  var $master = $('.master');
  //code here
})( jQuery );

4) You should create named functions instead of using anonymous functions. This will help with debugging via stack traces. Anonymous functions are much harder to debug.

(function( $ ) {
  'use strict';
  var $master = $('.master');
  function masterClickHandler() {
   //code here
  }
  $master.on('click', masterClickHandler);
})( jQuery );

5) You don't need all of your code in document.ready. If you follow the above advice, you can just call your named functions that have already been created. This is another slight performance boost.

(function( $ ) {
  'use strict';
  var $master;
  function masterClickHandler() {
   //code here
  }
  $(function(){  //shortcut for document.ready
    $master = $('.master');
    $master.on('click', masterClickHandler);
  });
})( jQuery );

Hope that helps.