Audio player for a website

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:

• Here

• 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();
v

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.

StackExchange Code Review Q#113162, answer score: 4