Address review comments

src/components/cardbuilder/card.css

@@ -169,7 +169,7 @@ button::-moz-focus-inner {
     color: inherit;
 }
 
-.cardImageContainer.cardScalable {
+.cardScalable .cardImageContainer {
     height: 100%;
     contain: strict;
 }

src/components/cardbuilder/cardBuilder.js

@@ -1534,7 +1534,6 @@ import 'programStyles';
                 case 'MusicAlbum':
                     return '<span class="cardImageIcon material-icons album"></span>';
                 case 'MusicArtist':
-                    return '<span class="cardImageIcon material-icons person"></span>';
                 case 'Person':
                     return '<span class="cardImageIcon material-icons person"></span>';
                 case 'Audio':

src/components/nowPlayingBar/nowPlayingBar.js

@@ -165,7 +165,8 @@ define(['require', 'datetime', 'itemHelper', 'events', 'browser', 'imageLoader',
                         return;
                     }
                     playbackManager.seekPercent(0, currentPlayer);
-                    // This is done automatically by playbackManager, however, setting this here gives instant visual feedback
+                    // This is done automatically by playbackManager, however, setting this here gives instant visual feedback.
+                    // TODO: Check why seekPercentage doesn't reflect the changes inmmediately, so we can remove this workaround.
                     positionSlider.value = 0;
                 } else {
                     playbackManager.previousTrack(currentPlayer);
@@ -181,11 +182,7 @@ define(['require', 'datetime', 'itemHelper', 'events', 'browser', 'imageLoader',
 
         elem.querySelector('.btnShuffleQueue').addEventListener('click', function () {
             if (currentPlayer) {
-                if (playbackManager.getQueueShuffleMode(currentPlayer) === 'Sorted') {
+                playbackManager.toggleQueueShuffleMode(currentPlayer);
-                    playbackManager.setQueueShuffleMode('Shuffle', currentPlayer);
-                } else {
-                    playbackManager.setQueueShuffleMode('Sorted', currentPlayer);
-                }
             }
         });
 
@@ -367,16 +364,23 @@ define(['require', 'datetime', 'itemHelper', 'events', 'browser', 'imageLoader',
 
     function updateRepeatModeDisplay(repeatMode) {
         toggleRepeatButtonIcon.classList.remove('repeat', 'repeat_one');
+        const cssClass = 'repeatButton-active';
 
-        if (repeatMode === 'RepeatAll') {
+        switch (repeatMode) {
+            case 'RepeatAll':
                 toggleRepeatButtonIcon.classList.add('repeat');
-            toggleRepeatButton.classList.add('repeatButton-active');
+                toggleRepeatButton.classList.add(cssClass);
-        } else if (repeatMode === 'RepeatOne') {
+                break;
+            case 'RepeatOne':
                 toggleRepeatButtonIcon.classList.add('repeat_one');
-            toggleRepeatButton.classList.add('repeatButton-active');
+                toggleRepeatButton.classList.add(cssClass);
-        } else {
+                break;
+            case 'RepeatNone':
                 toggleRepeatButtonIcon.classList.add('repeat');
-            toggleRepeatButton.classList.remove('repeatButton-active');
+                toggleRepeatButton.classList.remove(cssClass);
+                break;
+            default:
+                throw new TypeError('invalid value for repeatMode');
         }
     }
 
@@ -530,24 +534,26 @@ define(['require', 'datetime', 'itemHelper', 'events', 'browser', 'imageLoader',
         var nowPlayingItem = state.NowPlayingItem;
 
         var textLines = nowPlayingItem ? nowPlayingHelper.getNowPlayingNames(nowPlayingItem) : [];
-        if (textLines.length > 1) {
-            textLines[1].secondary = true;
-        }
-
-        if (textLines) {
         nowPlayingTextElement.innerHTML = '';
-            let itemText = document.createElement('div');
+        if (textLines) {
             let secondaryText = document.createElement('div');
             secondaryText.classList.add('nowPlayingBarSecondaryText');
+            let itemText = document.createElement('div');
+            if (textLines.length > 1) {
+                textLines[1].secondary = true;
+                if (textLines[1].text) {
+                    let text = document.createElement('a');
+                    text.innerHTML = textLines[1].text;
+                    secondaryText.appendChild(text);
+                }
+            }
+
+            if (textLines[0].text) {
                 if (textLines[0].text) {
                     let text = document.createElement('a');
                     text.innerHTML = textLines[0].text;
                     itemText.appendChild(text);
                 }
-            if (textLines[1].text) {
-                let text = document.createElement('a');
-                text.innerHTML = textLines[1].text;
-                secondaryText.appendChild(text);
             }
             nowPlayingTextElement.appendChild(itemText);
             nowPlayingTextElement.appendChild(secondaryText);
@@ -586,11 +592,11 @@ define(['require', 'datetime', 'itemHelper', 'events', 'browser', 'imageLoader',
                     var userData = item.UserData || {};
                     var likes = userData.Likes == null ? '' : userData.Likes;
                     if (!layoutManager.mobile) {
-                        let contextButton = document.querySelector('.nowPlayingBar').querySelector('.btnToggleContextMenu');
+                        let contextButton = nowPlayingBarElement.querySelector('.btnToggleContextMenu');
                         // We remove the previous event listener by replacing the item in each update event
                         let contextButtonClone = contextButton.cloneNode(true);
                         contextButton.parentNode.replaceChild(contextButtonClone, contextButton);
-                        contextButton = document.querySelector('.nowPlayingBar').querySelector('.btnToggleContextMenu');
+                        contextButton = nowPlayingBarElement.querySelector('.btnToggleContextMenu');
                         let options = {
                             play: false,
                             queue: false,
@@ -637,10 +643,15 @@ define(['require', 'datetime', 'itemHelper', 'events', 'browser', 'imageLoader',
         let context = nowPlayingBarElement;
         let toggleShuffleButton = context.querySelector('.btnShuffleQueue');
 
-        if (shuffleMode === 'Sorted') {
+        switch (shuffleMode) {
+            case 'Sorted':
                 toggleShuffleButton.classList.remove('shuffleQueue-active');
-        } else if (shuffleMode === 'Shuffle') {
+                break;
+            case 'Shuffle':
                 toggleShuffleButton.classList.add('shuffleQueue-active');
+                break;
+            default:
+                throw new TypeError('invalid value for shuffleMode');
         }
     }
 

src/components/playback/playbackmanager.js

@@ -3886,7 +3886,7 @@ define(['events', 'datetime', 'appSettings', 'itemHelper', 'pluginManager', 'pla
                 'GoToSearch',
                 'DisplayMessage',
                 'SetRepeatMode',
-                'SetQueueShuffleMode',
+                'SetShuffleQueue',
                 'PlayMediaSource',
                 'PlayTrailers'
             ];
@@ -3941,9 +3941,7 @@ define(['events', 'datetime', 'appSettings', 'itemHelper', 'pluginManager', 'pla
         return this._playQueueManager.getRepeatMode();
     };
 
-    PlaybackManager.prototype.setQueueShuffleMode = function (value, player) {
+    PlaybackManager.prototype.setQueueShuffleMode = function (value, player = this._currentPlayer) {
-
-        player = player || this._currentPlayer;
         if (player && !enableLocalPlaylistManagement(player)) {
             return player.setQueueShuffleMode(value);
         }
@@ -3952,9 +3950,7 @@ define(['events', 'datetime', 'appSettings', 'itemHelper', 'pluginManager', 'pla
         events.trigger(player, 'shufflequeuemodechange');
     };
 
-    PlaybackManager.prototype.getQueueShuffleMode = function (player) {
+    PlaybackManager.prototype.getQueueShuffleMode = function (player = this._currentPlayer) {
-
-        player = player || this._currentPlayer;
         if (player && !enableLocalPlaylistManagement(player)) {
             return player.getQueueShuffleMode();
         }
@@ -3962,6 +3958,26 @@ define(['events', 'datetime', 'appSettings', 'itemHelper', 'pluginManager', 'pla
         return this._playQueueManager.getShuffleMode();
     };
 
+    PlaybackManager.prototype.toggleQueueShuffleMode = function (player = this._currentPlayer) {
+        let currentvalue;
+        if (player && !enableLocalPlaylistManagement(player)) {
+            currentvalue = player.getQueueShuffleMode();
+            switch (currentvalue) {
+                case 'Shuffle':
+                    player.setQueueShuffleMode('Sorted');
+                    break;
+                case 'Sorted':
+                    player.setQueueShuffleMode('Shuffle');
+                    break;
+                default:
+                    throw new TypeError('current value for shufflequeue is invalid');
+            }
+        } else {
+            this._playQueueManager.toggleShuffleMode();
+        }
+        events.trigger(player, 'shufflequeuemodechange');
+    };
+
     PlaybackManager.prototype.trySetActiveDeviceName = function (name) {
 
         name = normalizeName(name);
@@ -4030,7 +4046,7 @@ define(['events', 'datetime', 'appSettings', 'itemHelper', 'pluginManager', 'pla
             case 'SetRepeatMode':
                 this.setRepeatMode(cmd.Arguments.RepeatMode, player);
                 break;
-            case 'SetQueueShuffleMode':
+            case 'SetShuffleQueue':
                 this.setQueueShuffleMode(cmd.Arguments.ShuffleMode, player);
                 break;
             case 'VolumeUp':

src/components/playback/playqueuemanager.js

@@ -60,16 +60,12 @@ define([], function () {
 
     PlayQueueManager.prototype.shufflePlaylist = function () {
         this._sortedPlaylist = [];
-        let currentPlaylistItem = this._playlist[this.getCurrentPlaylistIndex()];
+        for (const item of this._playlist) {
-        for (let item of this._playlist) {
             this._sortedPlaylist.push(item);
         }
+        const currentPlaylistItem = this._playlist.splice(this.getCurrentPlaylistIndex(), 1)[0];
 
         for (let i = this._playlist.length - 1; i > 0; i--) {
-            if (this._playlist[i] === currentPlaylistItem) {
-                this._playlist.splice(i, 1);
-                continue;
-            }
             const j = Math.floor(Math.random() * i);
             const temp = this._playlist[i];
             this._playlist[i] = this._playlist[j];
@@ -216,7 +212,8 @@ define([], function () {
     };
 
     PlayQueueManager.prototype.setRepeatMode = function (value) {
-        if (value === 'RepeatOne' || value === 'RepeatAll' || value === 'RepeatNone') {
+        const repeatModes = ['RepeatOne', 'RepeatAll', 'RepeatNone'];
+        if (repeatModes.includes(value)) {
             this._repeatMode = value;
         } else {
             throw new TypeError('invalid value provided for setRepeatMode');
@@ -228,12 +225,28 @@ define([], function () {
     };
 
     PlayQueueManager.prototype.setShuffleMode = function (value) {
-        if (value === 'Sorted') {
+        switch (value) {
-            this.sortShuffledPlaylist();
+            case 'Shuffle':
-        } else if (value === 'Shuffle') {
                 this.shufflePlaylist();
-        } else {
+                break;
-            throw new TypeError('invalid value provided for setShuffleMode');
+            case 'Sorted':
+                this.sortShuffledPlaylist();
+                break;
+            default:
+                throw new TypeError('invalid value provided to setShuffleMode');
+        }
+    };
+
+    PlayQueueManager.prototype.toggleShuffleMode = function () {
+        switch (this._shuffleMode) {
+            case 'Shuffle':
+                this.setShuffleMode('Sorted');
+                break;
+            case 'Sorted':
+                this.setShuffleMode('Shuffle');
+                break;
+            default:
+                throw new TypeError('current value for shufflequeue is invalid');
         }
     };
 

src/components/remotecontrol/remotecontrol.css

@@ -208,7 +208,6 @@
 
 .layout-mobile .playlistSectionButton .btnSavePlaylist {
     margin: 0;
-    padding-right: 0;
     border-radius: 0;
 }
 

src/components/remotecontrol/remotecontrol.js

@@ -123,7 +123,7 @@ define(['browser', 'datetime', 'backdrop', 'libraryBrowser', 'listView', 'imageL
                     var artistsSeries = '';
                     var albumName = item.Album;
                     if (item.ArtistItems != null) {
-                        for (let artist of item.ArtistItems) {
+                        for (const artist of item.ArtistItems) {
                             let artistName = artist.Name;
                             let artistId = artist.Id;
                             artistsSeries += `<a class="button-link emby-button" is="emby-linkbutton" href="itemdetails.html?id=${artistId}&amp;serverId=${nowPlayingServerId}">${artistName}</a>`;
@@ -135,7 +135,7 @@ define(['browser', 'datetime', 'backdrop', 'libraryBrowser', 'listView', 'imageL
                         // For some reason, Chromecast Player doesn't return a item.ArtistItems object, so we need to fallback
                         // to normal item.Artists item.
                         // TODO: Normalise fields returned by all the players
-                        for (let artist of item.Artists) {
+                        for (const artist of item.Artists) {
                             artistsSeries += `<a>${artist}</a>`;
                             if (artist !== item.Artists.slice(-1)[0]) {
                                 artistsSeries += ', ';
@@ -176,7 +176,6 @@ define(['browser', 'datetime', 'backdrop', 'libraryBrowser', 'listView', 'imageL
                 maxHeight: 300 * 2
             }) : null;
 
-            console.debug('updateNowPlayingInfo');
             let contextButton = context.querySelector('.btnToggleContextMenu');
             // We remove the previous event listener by replacing the item in each update event
             let contextButtonClone = contextButton.cloneNode(true);
@@ -355,18 +354,30 @@ define(['browser', 'datetime', 'backdrop', 'libraryBrowser', 'listView', 'imageL
         function updateRepeatModeDisplay(repeatMode) {
             var context = dlg;
             let toggleRepeatButtons = context.querySelectorAll('.repeatToggleButton');
+            const cssClass = 'repeatButton-active';
+            let innHtml = '<span class="material-icons repeat"></span>';
+            let repeatOn = undefined;
 
-            for (let toggleRepeatButton of toggleRepeatButtons) {
+            switch (repeatMode) {
-                if ('RepeatAll' == repeatMode) {
+                case 'RepeatAll':
-                    toggleRepeatButton.innerHTML = "<span class='material-icons repeat'></span>";
+                    break;
-                    toggleRepeatButton.classList.add('repeatButton-active');
+                case 'RepeatOne':
-                } else if ('RepeatOne' == repeatMode) {
+                    innHtml = '<span class="material-icons repeat_one"></span>';
-                    toggleRepeatButton.innerHTML = "<span class='material-icons repeat_one'></span>";
+                    break;
-                    toggleRepeatButton.classList.add('repeatButton-active');
+                case 'RepeatNone':
-                } else {
+                    repeatOn = null;
-                    toggleRepeatButton.innerHTML = "<span class='material-icons repeat'></span>";
+                    break;
-                    toggleRepeatButton.classList.remove('repeatButton-active');
+                default:
+                    throw new TypeError('invalid value for repeatMode');
             }
+
+            for (const toggleRepeatButton of toggleRepeatButtons) {
+                if (repeatOn === null) {
+                    toggleRepeatButton.classList.remove(cssClass);
+                } else {
+                    toggleRepeatButton.classList.add(cssClass);
+                }
+                toggleRepeatButton.innerHTML = innHtml;
             }
         }
 
@@ -516,10 +527,15 @@ define(['browser', 'datetime', 'backdrop', 'libraryBrowser', 'listView', 'imageL
             let shuffleButtons = context.querySelectorAll('.btnShuffleQueue');
 
             for (let shuffleButton of shuffleButtons) {
-                if ('Sorted' === shuffleMode) {
+                switch (shuffleMode) {
+                    case 'Sorted':
                         shuffleButton.classList.remove('shuffleQueue-active');
-                } else if ('Shuffle' === shuffleMode) {
+                        break;
+                    case 'Shuffle':
                         shuffleButton.classList.add('shuffleQueue-active');
+                        break;
+                    default:
+                        throw new TypeError('invalid shuffle mode');
                 }
             }
             onPlaylistUpdate();
@@ -708,14 +724,10 @@ define(['browser', 'datetime', 'backdrop', 'libraryBrowser', 'listView', 'imageL
                     playbackManager.fastForward(currentPlayer);
                 }
             });
-            for (let shuffleButton of context.querySelectorAll('.btnShuffleQueue')) {
+            for (const shuffleButton of context.querySelectorAll('.btnShuffleQueue')) {
                 shuffleButton.addEventListener('click', function () {
                     if (currentPlayer) {
-                        if (playbackManager.getQueueShuffleMode(currentPlayer) === 'Sorted') {
+                        playbackManager.toggleQueueShuffleMode(currentPlayer);
-                            playbackManager.setQueueShuffleMode('Shuffle', currentPlayer);
-                        } else {
-                            playbackManager.setQueueShuffleMode('Sorted', currentPlayer);
-                        }
                     }
                 });
             }
@@ -728,7 +740,8 @@ define(['browser', 'datetime', 'backdrop', 'libraryBrowser', 'listView', 'imageL
                             return;
                         }
                         playbackManager.seekPercent(0, currentPlayer);
-                        // This is done automatically by playbackManager. However, setting this here gives instant visual feedback
+                        // This is done automatically by playbackManager. However, setting this here gives instant visual feedback.
+                        // TODO: Check why seekPercentage doesn't reflect the changes inmmediately, so we can remove this workaround.
                         positionSlider.value = 0;
                     } else {
                         playbackManager.previousTrack(currentPlayer);
@@ -794,7 +807,7 @@ define(['browser', 'datetime', 'backdrop', 'libraryBrowser', 'listView', 'imageL
                 } else {
                     context.querySelector('.playlist').classList.add('hide');
                     context.querySelector('.btnSavePlaylist').classList.add('hide');
-                    if (showMuteButton && showVolumeSlider) {
+                    if (showMuteButton || showVolumeSlider) {
                         context.querySelector('.volumecontrol').classList.remove('hide');
                     }
                     if (layoutManager.mobile) {

src/plugins/chromecastPlayer/plugin.js

@@ -652,7 +652,7 @@ define(['appSettings', 'userSettings', 'playbackManager', 'connectionManager', '
                 'SetSubtitleStreamIndex',
                 'DisplayContent',
                 'SetRepeatMode',
-                'SetQueueShuffleMode',
+                'SetShuffleQueue',
                 'EndSession',
                 'PlayMediaSource',
                 'PlayTrailers'
@@ -897,7 +897,7 @@ define(['appSettings', 'userSettings', 'playbackManager', 'connectionManager', '
             options: {
                 ShuffleMode: value
             },
-            command: 'SetQueueShuffleMode'
+            command: 'SetShuffleQueue'
         });
     };
 

src/plugins/sessionPlayer/plugin.js

@@ -508,7 +508,7 @@ define(['playbackManager', 'events', 'serverNotifications', 'connectionManager']
 
     SessionPlayer.prototype.setQueueShuffleMode = function (mode) {
 
-        sendCommandByName(this, 'SetQueueShuffleMode', {
+        sendCommandByName(this, 'SetShuffleQueue', {
             ShuffleMode: mode
         });
     };

src/scripts/serverNotifications.js

@@ -65,7 +65,7 @@ define(['connectionManager', 'playbackManager', 'syncPlayManager', 'events', 'in
             case 'SetRepeatMode':
                 playbackManager.setRepeatMode(cmd.Arguments.RepeatMode);
                 break;
-            case 'SetQueueShuffleMode':
+            case 'SetShuffleQueue':
                 playbackManager.setQueueShuffleMode(cmd.Arguments.ShuffleMode);
                 break;
             case 'VolumeUp':