From 200559efb8636eeba092f5d217e2a5015eb729dc Mon Sep 17 00:00:00 2001 From: jcass Date: Sat, 7 Jan 2017 14:55:12 +0200 Subject: [PATCH] User correct track insert position when invoking 'PLAY_NEXT' from dialog. --- .../static/css/webclient.css | 5 + mopidy_musicbox_webclient/static/index.html | 8 +- .../static/js/controls.js | 114 ++++++++------ .../static/js/functionsvars.js | 1 + mopidy_musicbox_webclient/static/mb.appcache | 2 +- tests/js/dummy_tracklist.js | 45 ++++-- tests/js/test_controls.js | 143 ++++++++++++++---- 7 files changed, 226 insertions(+), 92 deletions(-) diff --git a/mopidy_musicbox_webclient/static/css/webclient.css b/mopidy_musicbox_webclient/static/css/webclient.css index 0220b3b6..f01fe7f0 100644 --- a/mopidy_musicbox_webclient/static/css/webclient.css +++ b/mopidy_musicbox_webclient/static/css/webclient.css @@ -416,6 +416,7 @@ span.hostInfo { .ui-icon-playAll:after, .ui-icon-play:after, .ui-icon-playNext:after, +.ui-icon-insert:after, .ui-icon-add:after, .ui-icon-addAll:after, .ui-icon-remove:after { @@ -435,6 +436,10 @@ span.hostInfo { content: '\f149'; } +.ui-icon-insert:after { + content: '\f177'; +} + .ui-icon-add:after { content: '\f196'; } diff --git a/mopidy_musicbox_webclient/static/index.html b/mopidy_musicbox_webclient/static/index.html index 5aa6eb18..9afe5c99 100644 --- a/mopidy_musicbox_webclient/static/index.html +++ b/mopidy_musicbox_webclient/static/index.html @@ -160,11 +160,11 @@

Artists

  • Play
    • -
  • - Insert Track After This One +
  • + Insert a Track Here
  • - Remove from Queue + Remove from Queue
  • Show Album @@ -196,7 +196,7 @@

    Artists

    - diff --git a/mopidy_musicbox_webclient/static/js/controls.js b/mopidy_musicbox_webclient/static/js/controls.js index c643afc2..04b3a69d 100644 --- a/mopidy_musicbox_webclient/static/js/controls.js +++ b/mopidy_musicbox_webclient/static/js/controls.js @@ -62,15 +62,16 @@ switch (action) { case PLAY_NOW: case PLAY_NEXT: - if (action === PLAY_NOW || typeof index === 'undefined' || index === '') { - // Use current track as reference point for insertion. - mopidy.tracklist.index().then(function (currentIndex) { - controls._addTrackAtIndex(action, mopidy, trackUris, currentIndex) - }) - } else { - // Use provided index for insertion. - controls._addTrackAtIndex(action, mopidy, trackUris, index) - } + mopidy.tracklist.index().then(function (currentIndex) { + if (currentIndex === null && action === PLAY_NEXT) { + // Tracklist is empty, start playing new tracks immediately. + action = PLAY_NOW + } + controls._addTrackAtIndex(action, mopidy, trackUris, currentIndex) + }) + break + case INSERT_AT_INDEX: + controls._addTrackAtIndex(action, mopidy, trackUris, index) break case ADD_THIS_BOTTOM: case ADD_ALL_BOTTOM: @@ -89,12 +90,14 @@ throw new Error('Unexpected tracklist action identifier: ' + action) } - if (window[$(document.body).data('on-track-click')] === DYNAMIC) { - // Save last 'action' - will become default for future 'onClick' events - var previousAction = $.cookie('onTrackClick') - if (typeof previousAction === 'undefined' || action !== previousAction) { - $.cookie('onTrackClick', action, { expires: 365 }) - updatePlayIcons('', '', controls.getIconForAction(action)) + if (action !== INSERT_AT_INDEX) { // TODO: Add support for 'INSERT_AT_INDEX' to allow user to insert tracks in any playlist. + if (window[$(document.body).data('on-track-click')] === DYNAMIC) { + // Save last 'action' - will become default for future 'onClick' events + var previousAction = $.cookie('onTrackClick') + if (typeof previousAction === 'undefined' || action !== previousAction) { + $.cookie('onTrackClick', action, { expires: 365 }) + updatePlayIcons('', '', controls.getIconForAction(action)) + } } } @@ -125,6 +128,8 @@ return 'fa fa-play-circle' case PLAY_NOW: return 'fa fa-play-circle-o' + case INSERT_AT_INDEX: + return 'fa fa-long-arrow-left' case PLAY_NEXT: return 'fa fa-level-down' case ADD_THIS_BOTTOM: @@ -143,6 +148,7 @@ switch (parseInt(action)) { case PLAY_NOW: case PLAY_NEXT: + case INSERT_AT_INDEX: case ADD_THIS_BOTTOM: // Process single track trackUris.push(trackUri) @@ -159,19 +165,19 @@ }, _addTrackAtIndex: function (action, mopidy, trackUris, index) { - var pos - if (index !== null) { - if (action === PLAY_NOW) { - // Insert at provided index. - pos = index - } else if (action === PLAY_NEXT) { - // Insert after provided index. - pos = index + 1 - } - } else { - // Insert at top of queue + if (typeof index === 'undefined' || index === '') { + throw new Error('No index provided for insertion.') + } + + var pos = index + if (pos === null) { pos = 0 } + + if (action === PLAY_NEXT) { + pos = pos + 1 + } + mopidy.tracklist.add({at_position: pos, uris: trackUris}).then(function (tlTracks) { if (action === PLAY_NOW) { // Start playback immediately. mopidy.playback.stop().then(function () { @@ -203,7 +209,7 @@ /** ********************************* * remove a track from the queue * ***********************************/ - removeTrack: function (tlid) { + removeTrack: function (tlid, mopidy) { toast('Deleting...') tlid = tlid || $('#popupQueue').data('tlid') @@ -236,7 +242,7 @@ $(this).removeData('tlid') }) var trackName = popupData[$('#popupQueue').data('track')].name - $('#select-add').append('') + $('#select-add').append('') } if (typeof songdata.track.uri !== 'undefined' && songdata.track.uri !== '') { $('#getPlayingBtn').button('enable') @@ -252,35 +258,47 @@ $('#popupAddTrack').popup('open') }, - addTrack: function (trackUri) { - if (parseInt($('#select-add').val()) === ADD_THIS_BOTTOM) { - controls.addTrackToBottom(trackUri) + addTrack: function (trackUri, mopidy) { + var selection = parseInt($('#select-add').val()) + + if (selection === ADD_THIS_BOTTOM) { + controls.addTrackToBottom(trackUri, mopidy) + + } else if (selection === PLAY_NEXT) { + controls.insertTrack(trackUri, mopidy) + + } else if (selection === INSERT_AT_INDEX) { + var tlid = $('#popupAddTrack').data('tlid') + controls.insertTrack(trackUri, mopidy, tlid) } else { - controls.insertTrack(trackUri) + throw new Error('Unkown tracklist action selection option: ' + selection) } }, - insertTrack: function (trackUri) { - if (typeof trackUri !== 'undefined' && trackUri !== '') { - var tlid = $('#popupAddTrack').data('tlid') - if (typeof tlid !== 'undefined' && tlid !== '') { - mopidy.tracklist.index({tlid: parseInt(tlid)}).then(function (index) { - controls.playTracks(PLAY_NEXT, mopidy, trackUri, 'undefined', index) - }) - } else { - // No tlid provided, insert after current track. - controls.playTracks(PLAY_NEXT, mopidy, trackUri, 'undefined') - } - $('#popupAddTrack').popup('close') + insertTrack: function (trackUri, mopidy, tlid) { + if (typeof trackUri === 'undefined' || trackUri === '') { + throw new Error('No track URI provided to insert.') + } + + if (typeof tlid !== 'undefined' && tlid !== '') { + mopidy.tracklist.index({tlid: parseInt(tlid)}).then(function (index) { + controls.playTracks(INSERT_AT_INDEX, mopidy, trackUri, CURRENT_PLAYLIST_TABLE, index) + }) + } else { + // No tlid provided, insert after current track. + controls.playTracks(PLAY_NEXT, mopidy, trackUri, CURRENT_PLAYLIST_TABLE) } + $('#popupAddTrack').popup('close') return false }, - addTrackToBottom: function (trackUri) { - if (typeof trackUri !== 'undefined' && trackUri !== '') { - controls.playTracks(ADD_THIS_BOTTOM, mopidy, trackUri, 'undefined') - $('#popupAddTrack').popup('close') + addTrackToBottom: function (trackUri, mopidy) { + if (typeof trackUri === 'undefined' || trackUri === '') { + throw new Error('No track URI provided to add.') } + + controls.playTracks(ADD_THIS_BOTTOM, mopidy, trackUri, CURRENT_PLAYLIST_TABLE) + $('#popupAddTrack').popup('close') return false }, diff --git a/mopidy_musicbox_webclient/static/js/functionsvars.js b/mopidy_musicbox_webclient/static/js/functionsvars.js index 2ad23b72..a06a2a7d 100644 --- a/mopidy_musicbox_webclient/static/js/functionsvars.js +++ b/mopidy_musicbox_webclient/static/js/functionsvars.js @@ -69,6 +69,7 @@ ADD_THIS_BOTTOM = 2 ADD_ALL_BOTTOM = 3 PLAY_ALL = 4 DYNAMIC = 5 +INSERT_AT_INDEX = 6 // the first part of Mopidy extensions which serve radio streams var radioExtensionsList = ['somafm', 'tunein', 'dirble', 'audioaddict'] diff --git a/mopidy_musicbox_webclient/static/mb.appcache b/mopidy_musicbox_webclient/static/mb.appcache index b952b450..738559a1 100644 --- a/mopidy_musicbox_webclient/static/mb.appcache +++ b/mopidy_musicbox_webclient/static/mb.appcache @@ -1,6 +1,6 @@ CACHE MANIFEST -# 2017-01-06:v2 +# 2017-01-07:v1 NETWORK: * diff --git a/tests/js/dummy_tracklist.js b/tests/js/dummy_tracklist.js index ee096ff1..0919f84f 100644 --- a/tests/js/dummy_tracklist.js +++ b/tests/js/dummy_tracklist.js @@ -31,10 +31,13 @@ throw new Error('DummyTracklist.add does not support deprecated "tracks" and "uri" parameters.') } + var position = params.at_position // Add tracks to end of tracklist if no position is provided - params.at_position = params.at_position || this._tlTracks.length + if (typeof position === 'undefined') { + position = Math.max(0, this._tlTracks.length) + } + var tlTrack - var tlTracks = [] for (var i = 0; i < params.uris.length; i++) { tlTrack = { tlid: this._nextTlid++, @@ -42,11 +45,10 @@ uri: params.uris[i] } } - tlTracks.push(tlTrack) - this._tlTracks.splice(params.at_position + i, 0, tlTrack) + this._tlTracks.splice(position++, 0, tlTrack) } - return $.when(tlTracks) + return $.when(this._tlTracks) } /* Clears the tracklist */ @@ -54,6 +56,21 @@ this._tlTracks = [] } + /* Remove the matching tracks from the tracklist */ + DummyTracklist.prototype.remove = function (criteria) { + this.filter(criteria).then( function (matches) { + for (var i = 0; i < matches.length; i++) { + for (var j = 0; j < this._tlTracks.length; j++) { + if (this._tlTracks[j].track.uri === matches[i].track.uri) { + this._tlTracks.splice(j, 1) + } + } + } + }.bind(this)) + + return $.when(this._tlTracks) + } + /** * Retuns a list containing tlTracks that contain the provided * criteria.uri or has ID criteria.tlid. @@ -89,9 +106,9 @@ /* Retuns index of the currently 'playing' track. */ DummyTracklist.prototype.index = function (params) { if (!params) { - if (this._tlTracks.length > 1) { - // Always just assume that the second track is playing - return $.when(1) + if (this._tlTracks.length > 0) { + // Always just assume that the first track is playing + return $.when(0) } else { return $.when(null) } @@ -101,7 +118,17 @@ return $.when(i) } } - return $.when(0) + return $.when(null) + } + + /* Returns the tracks in the tracklist */ + DummyTracklist.prototype.get_tl_tracks = function () { + return $.when(this._tlTracks) + } + + /* Returns the length of the tracklist */ + DummyTracklist.prototype.get_length = function () { + return $.when(this._tlTracks.length) } return DummyTracklist diff --git a/tests/js/test_controls.js b/tests/js/test_controls.js index 7a4e95ff..381be8e6 100644 --- a/tests/js/test_controls.js +++ b/tests/js/test_controls.js @@ -13,8 +13,8 @@ describe('controls', function () { var mopidy var div_element var QUEUE_TRACKS = [ // Simulate an existing queue with three tracks loaded. - {uri: 'track:tlTrackMock1'}, - {uri: 'track:tlTrackMock2'}, // <-- Currently playing track + {uri: 'track:tlTrackMock1'}, // <-- Currently playing track + {uri: 'track:tlTrackMock2'}, {uri: 'track:tlTrackMock3'} ] var NEW_TRACKS = [ // Simulate the user browsing to a folder with three tracks inside it. @@ -48,11 +48,8 @@ describe('controls', function () { mopidy.tracklist.clear() clearSpy.reset() mopidy.tracklist.add({uris: getUris(QUEUE_TRACKS)}) - }) - - afterEach(function () { - mopidy.playback.play.reset() addSpy.reset() + mopidy.playback.play.reset() }) after(function () { @@ -62,62 +59,62 @@ describe('controls', function () { describe('#playTracks()', function () { it('PLAY_ALL should clear tracklist first before populating with tracks', function () { - customTracklists[BROWSE_TABLE] = NEW_TRACKS - controls.playTracks(PLAY_ALL, mopidy, NEW_TRACKS[0].uri, BROWSE_TABLE) + customTracklists[CURRENT_PLAYLIST_TABLE] = NEW_TRACKS + controls.playTracks(PLAY_ALL, mopidy, NEW_TRACKS[0].uri, CURRENT_PLAYLIST_TABLE) assert(clearSpy.called) }) it('should not clear tracklist for events other than PLAY_ALL', function () { - customTracklists[BROWSE_TABLE] = NEW_TRACKS - controls.playTracks(PLAY_NOW, mopidy, NEW_TRACKS[0].uri, BROWSE_TABLE) + customTracklists[CURRENT_PLAYLIST_TABLE] = NEW_TRACKS + controls.playTracks(PLAY_NOW, mopidy, NEW_TRACKS[0].uri, CURRENT_PLAYLIST_TABLE) assert(clearSpy.notCalled) }) it('should raise exception if trackUri parameter is not provided and "track" data attribute is empty', function () { assert.throw(function () { controls.playTracks('', mopidy) }, Error) - controls.playTracks(PLAY_ALL, mopidy, NEW_TRACKS[0].uri, BROWSE_TABLE) + controls.playTracks(PLAY_ALL, mopidy, NEW_TRACKS[0].uri, CURRENT_PLAYLIST_TABLE) assert(mopidy.playback.play.calledWithMatch({tlid: mopidy.tracklist._tlTracks[0].tlid})) }) it('should raise exception if playListUri parameter is not provided and "track" data attribute is empty', function () { assert.throw(function () { controls.playTracks('', mopidy, NEW_TRACKS[0].uri) }, Error) - controls.playTracks(PLAY_ALL, mopidy, NEW_TRACKS[0].uri, BROWSE_TABLE) + controls.playTracks(PLAY_ALL, mopidy, NEW_TRACKS[0].uri, CURRENT_PLAYLIST_TABLE) assert(mopidy.playback.play.calledWithMatch({tlid: mopidy.tracklist._tlTracks[0].tlid})) }) it('should raise exception if unknown tracklist action is provided', function () { var getTrackURIsForActionStub = sinon.stub(controls, '_getTrackURIsForAction') // Stub to bypass earlier exception - assert.throw(function () { controls.playTracks('99', mopidy, NEW_TRACKS[0].uri, BROWSE_TABLE) }, Error) + assert.throw(function () { controls.playTracks('99', mopidy, NEW_TRACKS[0].uri, CURRENT_PLAYLIST_TABLE) }, Error) getTrackURIsForActionStub.restore() }) it('should use "track" and "list" data attributes as fallback if parameters are not provided', function () { $('#popupTracks').data('track', 'track:trackMock1') // Simulate 'track:trackMock1' being clicked. - $('#popupTracks').data('list', BROWSE_TABLE) - customTracklists[BROWSE_TABLE] = NEW_TRACKS + $('#popupTracks').data('list', CURRENT_PLAYLIST_TABLE) + customTracklists[CURRENT_PLAYLIST_TABLE] = NEW_TRACKS controls.playTracks(PLAY_ALL, mopidy) assert(mopidy.playback.play.calledWithMatch({tlid: mopidy.tracklist._tlTracks[0].tlid})) }) it('PLAY_NOW, PLAY_NEXT, and ADD_THIS_BOTTOM should only add one track to the tracklist', function () { - controls.playTracks(PLAY_NOW, mopidy, NEW_TRACKS[0].uri) - assert(addSpy.calledWithMatch({at_position: 1, uris: [NEW_TRACKS[0].uri]}), 'PLAY_NOW did not add correct track') - addSpy.reset() - + controls.playTracks(PLAY_NOW, mopidy, NEW_TRACKS[0].uri, CURRENT_PLAYLIST_TABLE) + assert(addSpy.calledWithMatch({at_position: 0, uris: [NEW_TRACKS[0].uri]}), 'PLAY_NOW did not add correct track') + mopidy.tracklist.clear() mopidy.tracklist.add({uris: getUris(QUEUE_TRACKS)}) - - controls.playTracks(PLAY_NEXT, mopidy, NEW_TRACKS[0].uri) - assert(addSpy.calledWithMatch({at_position: 2, uris: [NEW_TRACKS[0].uri]}), 'PLAY_NEXT did not add correct track') addSpy.reset() + controls.playTracks(PLAY_NEXT, mopidy, NEW_TRACKS[0].uri, CURRENT_PLAYLIST_TABLE) + assert(addSpy.calledWithMatch({at_position: 1, uris: [NEW_TRACKS[0].uri]}), 'PLAY_NEXT did not add correct track') + mopidy.tracklist.clear() mopidy.tracklist.add({uris: getUris(QUEUE_TRACKS)}) + addSpy.reset() - controls.playTracks(ADD_THIS_BOTTOM, mopidy, NEW_TRACKS[0].uri) + controls.playTracks(ADD_THIS_BOTTOM, mopidy, NEW_TRACKS[0].uri, CURRENT_PLAYLIST_TABLE) assert(addSpy.calledWithMatch({uris: [NEW_TRACKS[0].uri]}), 'ADD_THIS_BOTTOM did not add correct track') }) @@ -134,12 +131,12 @@ describe('controls', function () { }) it('PLAY_NEXT should insert track after currently playing track by default', function () { - controls.playTracks(PLAY_NEXT, mopidy, NEW_TRACKS[0].uri) - assert(addSpy.calledWithMatch({at_position: 2, uris: [NEW_TRACKS[0].uri]}), 'PLAY_NEXT did not insert track at correct position') + controls.playTracks(PLAY_NOW, mopidy, NEW_TRACKS[0].uri) + assert(addSpy.calledWithMatch({at_position: 0, uris: [NEW_TRACKS[0].uri]}), 'PLAY_NEXT did not insert track at correct position') }) it('PLAY_NEXT should insert track after reference track index, if provided', function () { - controls.playTracks(PLAY_NEXT, mopidy, NEW_TRACKS[0].uri, 'undefined', 0) + controls.playTracks(PLAY_NEXT, mopidy, NEW_TRACKS[0].uri, '', 0) assert(addSpy.calledWithMatch({at_position: 1, uris: [NEW_TRACKS[0].uri]}), 'PLAY_NEXT did not insert track at correct position') }) @@ -151,7 +148,7 @@ describe('controls', function () { it('PLAY_NOW should always insert track at current index', function () { controls.playTracks(PLAY_NOW, mopidy, NEW_TRACKS[0].uri) - assert(addSpy.calledWithMatch({at_position: 1, uris: [NEW_TRACKS[0].uri]}), 'PLAY_NOW did not insert track at correct position') + assert(addSpy.calledWithMatch({at_position: 0, uris: [NEW_TRACKS[0].uri]}), 'PLAY_NOW did not insert track at correct position') addSpy.reset() mopidy.tracklist.clear() @@ -162,7 +159,7 @@ describe('controls', function () { it('only PLAY_NOW and PLAY_ALL should trigger playback', function () { controls.playTracks(PLAY_NOW, mopidy) - assert(mopidy.playback.play.calledWithMatch({tlid: mopidy.tracklist._tlTracks[1].tlid}), 'PLAY_NOW did not start playback of correct track') + assert(mopidy.playback.play.calledWithMatch({tlid: mopidy.tracklist._tlTracks[0].tlid}), 'PLAY_NOW did not start playback of correct track') mopidy.playback.play.reset() mopidy.tracklist.clear() @@ -260,9 +257,9 @@ describe('controls', function () { }) it('should get tracks from "playlistUri" for PLAY_ALL, and ADD_ALL_BOTTOM', function () { - customTracklists[BROWSE_TABLE] = NEW_TRACKS + customTracklists[CURRENT_PLAYLIST_TABLE] = NEW_TRACKS - var tracks = controls._getTrackURIsForAction(PLAY_ALL, NEW_TRACKS[0], BROWSE_TABLE) + var tracks = controls._getTrackURIsForAction(PLAY_ALL, NEW_TRACKS[0], CURRENT_PLAYLIST_TABLE) assert.equal(tracks.length, NEW_TRACKS.length) for (var i = 0; i < tracks.length; i++) { assert.equal(tracks[i], NEW_TRACKS[i].uri) @@ -277,4 +274,90 @@ describe('controls', function () { assert.equal(controls._getTrackURIsForAction('0', 'mockUri')[0], 'mockUri') }) }) + + describe('#insertTrack()', function () { + it('should raise exception if no uri is provided', function () { + assert.throw(function () { controls.insertTrack() }, Error) + }) + + it('should insert track after currently playing track by default', function () { + var tracklistLength = QUEUE_TRACKS.length + var insertUri = NEW_TRACKS[0].uri + + controls.insertTrack(insertUri, mopidy) + + mopidy.tracklist.get_length().then(function (length) { + assert.equal(length, tracklistLength + 1) + }) + + mopidy.tracklist.index().then( function (index) { + mopidy.tracklist.get_tl_tracks().then( function (tlTracks) { + assert.equal(tlTracks[index + 1].track.uri, insertUri) + }) + }) + }) + + it('should insert track at provided index', function () { + var tracklistLength = QUEUE_TRACKS.length + var insertUri = NEW_TRACKS[0].uri + + mopidy.tracklist.get_tl_tracks().then(function (tlTracks) { + controls.insertTrack(insertUri, mopidy, tlTracks[1].tlid) + }) + + mopidy.tracklist.get_length().then(function (length) { + assert.equal(length, tracklistLength + 1) + }) + + mopidy.tracklist.get_tl_tracks().then( function (tlTracks) { + assert.equal(tlTracks[1].track.uri, insertUri) + }) + }) + }) + + describe('#addTrackToBottom()', function () { + it('should raise exception if no uri is provided', function () { + assert.throw(function () { controls.addTrackToBottom() }, Error) + }) + + it('should add track at bottom of tracklist', function () { + var tracklistLength = QUEUE_TRACKS.length + var insertUri = NEW_TRACKS[0].uri + + controls.addTrackToBottom(insertUri, mopidy) + + mopidy.tracklist.get_length().then(function (length) { + assert.equal(length, tracklistLength + 1) + }) + + mopidy.tracklist.get_tl_tracks().then(function (tlTracks) { + assert.equal(tlTracks[tlTracks.length - 1].track.uri, insertUri) + }) + }) + }) + + describe('#removeTrack()', function () { + it('should remove track', function () { + var tracklistLength = QUEUE_TRACKS.length + var deleteUri = QUEUE_TRACKS[1].uri + + mopidy.tracklist.get_tl_tracks().then(function (tlTracks) { + controls.removeTrack(tlTracks[1].tlid, mopidy) + }) + + mopidy.tracklist.get_length().then(function (length) { + assert.equal(length, tracklistLength - 1) + }) + + mopidy.tracklist.get_tl_tracks().then(function (tlTracks) { + var found = false + for (var i = 0; i < tlTracks.length; i++) { + if (tlTracks[i].track.uri === deleteUri) { + found = true + } + } + assert(!found) + }) + }) + }) })