From 072e20b5851d929d5ea9f6a66ddcf9279e4ebe66 Mon Sep 17 00:00:00 2001 From: callum Date: Sat, 27 Aug 2022 13:50:58 +0100 Subject: [PATCH 1/4] Fixed: itemContextMenu opened from bottom media control bar applying operations to the wrong item when items have no image metadata. Can manifest as the wrong song being added to a playlist, deleted, or receiving user supplied metadata, etc. --- src/components/nowPlayingBar/nowPlayingBar.js | 82 +++++++++---------- 1 file changed, 37 insertions(+), 45 deletions(-) diff --git a/src/components/nowPlayingBar/nowPlayingBar.js b/src/components/nowPlayingBar/nowPlayingBar.js index 4277100168..0b1363add5 100644 --- a/src/components/nowPlayingBar/nowPlayingBar.js +++ b/src/components/nowPlayingBar/nowPlayingBar.js @@ -24,6 +24,7 @@ import { appRouter } from '../appRouter'; let currentTimeElement; let nowPlayingImageElement; + let nowPlayingImageUrl; let nowPlayingTextElement; let nowPlayingUserData; let muteButton; @@ -488,7 +489,6 @@ import { appRouter } from '../appRouter'; return null; } - let currentImgUrl; function updateNowPlayingInfo(state) { const nowPlayingItem = state.NowPlayingItem; @@ -524,54 +524,46 @@ import { appRouter } from '../appRouter'; height: imgHeight })) : null; - let isRefreshing = false; - - if (url !== currentImgUrl) { - currentImgUrl = url; - isRefreshing = true; - - if (url) { - imageLoader.lazyImage(nowPlayingImageElement, url); - nowPlayingImageElement.style.display = null; - nowPlayingTextElement.style.marginLeft = null; - } else { - nowPlayingImageElement.style.backgroundImage = ''; - nowPlayingImageElement.style.display = 'none'; - nowPlayingTextElement.style.marginLeft = '1em'; - } + if (url && url !== nowPlayingImageUrl) { + nowPlayingImageUrl = url; + imageLoader.lazyImage(nowPlayingImageElement, url); + nowPlayingImageElement.style.display = null; + nowPlayingTextElement.style.marginLeft = null; + } else { + nowPlayingImageElement.style.backgroundImage = ''; + nowPlayingImageElement.style.display = 'none'; + nowPlayingTextElement.style.marginLeft = '1em'; } if (nowPlayingItem.Id) { - if (isRefreshing) { - const apiClient = ServerConnections.getApiClient(nowPlayingItem.ServerId); - apiClient.getItem(apiClient.getCurrentUserId(), nowPlayingItem.Id).then(function (item) { - const userData = item.UserData || {}; - const likes = userData.Likes == null ? '' : userData.Likes; - if (!layoutManager.mobile) { - let contextButton = nowPlayingBarElement.querySelector('.btnToggleContextMenu'); - // We remove the previous event listener by replacing the item in each update event - const contextButtonClone = contextButton.cloneNode(true); - contextButton.parentNode.replaceChild(contextButtonClone, contextButton); - contextButton = nowPlayingBarElement.querySelector('.btnToggleContextMenu'); - const options = { - play: false, - queue: false, - stopPlayback: true, - clearQueue: true, - positionTo: contextButton - }; - apiClient.getCurrentUser().then(function (user) { - contextButton.addEventListener('click', function () { - itemContextMenu.show(Object.assign({ - item: item, - user: user - }, options)); - }); + const apiClient = ServerConnections.getApiClient(nowPlayingItem.ServerId); + apiClient.getItem(apiClient.getCurrentUserId(), nowPlayingItem.Id).then(function (item) { + const userData = item.UserData || {}; + const likes = userData.Likes == null ? '' : userData.Likes; + if (!layoutManager.mobile) { + let contextButton = nowPlayingBarElement.querySelector('.btnToggleContextMenu'); + // We remove the previous event listener by replacing the item in each update event + const contextButtonClone = contextButton.cloneNode(true); + contextButton.parentNode.replaceChild(contextButtonClone, contextButton); + contextButton = nowPlayingBarElement.querySelector('.btnToggleContextMenu'); + const options = { + play: false, + queue: false, + stopPlayback: true, + clearQueue: true, + positionTo: contextButton + }; + apiClient.getCurrentUser().then(function (user) { + contextButton.addEventListener('click', function () { + itemContextMenu.show(Object.assign({ + item: item, + user: user + }, options)); }); - } - nowPlayingUserData.innerHTML = ''; - }); - } + }); + } + nowPlayingUserData.innerHTML = ''; + }); } else { nowPlayingUserData.innerHTML = ''; } From 44a5d7bb8d6a61682862850b9d0836d32d08cdb0 Mon Sep 17 00:00:00 2001 From: callum Date: Sat, 3 Sep 2022 21:46:01 +0100 Subject: [PATCH 2/4] Ensure nowPlayingBar updates correctly when navigating from a song with no image back to a previous song with an image. --- src/components/nowPlayingBar/nowPlayingBar.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/nowPlayingBar/nowPlayingBar.js b/src/components/nowPlayingBar/nowPlayingBar.js index 0b1363add5..823f8fd178 100644 --- a/src/components/nowPlayingBar/nowPlayingBar.js +++ b/src/components/nowPlayingBar/nowPlayingBar.js @@ -530,6 +530,7 @@ import { appRouter } from '../appRouter'; nowPlayingImageElement.style.display = null; nowPlayingTextElement.style.marginLeft = null; } else { + nowPlayingImageUrl = null; nowPlayingImageElement.style.backgroundImage = ''; nowPlayingImageElement.style.display = 'none'; nowPlayingTextElement.style.marginLeft = '1em'; From 689a65cc92e3e7b39925a767886e85de61b06516 Mon Sep 17 00:00:00 2001 From: callum Date: Sat, 3 Sep 2022 21:54:09 +0100 Subject: [PATCH 3/4] Style. Prefer to use nowPlayingImageUrl to load image in order to make intent clear. --- src/components/nowPlayingBar/nowPlayingBar.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/nowPlayingBar/nowPlayingBar.js b/src/components/nowPlayingBar/nowPlayingBar.js index 823f8fd178..779aaa52e7 100644 --- a/src/components/nowPlayingBar/nowPlayingBar.js +++ b/src/components/nowPlayingBar/nowPlayingBar.js @@ -526,7 +526,7 @@ import { appRouter } from '../appRouter'; if (url && url !== nowPlayingImageUrl) { nowPlayingImageUrl = url; - imageLoader.lazyImage(nowPlayingImageElement, url); + imageLoader.lazyImage(nowPlayingImageElement, nowPlayingImageUrl); nowPlayingImageElement.style.display = null; nowPlayingTextElement.style.marginLeft = null; } else { From ebfd28d396b41459b87fcdbb227af23e3f1c5cb2 Mon Sep 17 00:00:00 2001 From: callum Date: Sat, 10 Sep 2022 20:21:37 +0100 Subject: [PATCH 4/4] Avoid unnecessary DOM update if concurrent played items lack image metadata. --- src/components/nowPlayingBar/nowPlayingBar.js | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/components/nowPlayingBar/nowPlayingBar.js b/src/components/nowPlayingBar/nowPlayingBar.js index 779aaa52e7..ddf0544727 100644 --- a/src/components/nowPlayingBar/nowPlayingBar.js +++ b/src/components/nowPlayingBar/nowPlayingBar.js @@ -524,16 +524,18 @@ import { appRouter } from '../appRouter'; height: imgHeight })) : null; - if (url && url !== nowPlayingImageUrl) { - nowPlayingImageUrl = url; - imageLoader.lazyImage(nowPlayingImageElement, nowPlayingImageUrl); - nowPlayingImageElement.style.display = null; - nowPlayingTextElement.style.marginLeft = null; - } else { - nowPlayingImageUrl = null; - nowPlayingImageElement.style.backgroundImage = ''; - nowPlayingImageElement.style.display = 'none'; - nowPlayingTextElement.style.marginLeft = '1em'; + if (url !== nowPlayingImageUrl) { + if (url) { + nowPlayingImageUrl = url; + imageLoader.lazyImage(nowPlayingImageElement, nowPlayingImageUrl); + nowPlayingImageElement.style.display = null; + nowPlayingTextElement.style.marginLeft = null; + } else { + nowPlayingImageUrl = null; + nowPlayingImageElement.style.backgroundImage = ''; + nowPlayingImageElement.style.display = 'none'; + nowPlayingTextElement.style.marginLeft = '1em'; + } } if (nowPlayingItem.Id) {