From 4fe696bf0b1872e62df6312037f72d25856e4eb6 Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Thu, 26 Sep 2024 23:21:17 +1200 Subject: [PATCH 01/12] feat: bring up to date to fix merge issues --- src/renderer/helpers/playlists.js | 16 +++++++++++++++- src/renderer/views/Playlist/Playlist.js | 19 +++++++++++++++++++ static/locales/en-GB.yaml | 4 ++++ static/locales/en-US.yaml | 4 ++++ 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/renderer/helpers/playlists.js b/src/renderer/helpers/playlists.js index 5f53972f4b43..e21b2e53084e 100644 --- a/src/renderer/helpers/playlists.js +++ b/src/renderer/helpers/playlists.js @@ -5,6 +5,8 @@ export const SORT_BY_VALUES = { AuthorDescending: 'author_descending', VideoTitleAscending: 'video_title_ascending', VideoTitleDescending: 'video_title_descending', + VideoLengthAscending: 'video_length_ascending', + VideoLengthDescending: 'video_length_descending', Custom: 'custom' } @@ -19,7 +21,9 @@ export function getSortedPlaylistItems(playlistItems, sortOrder, locale, reverse sortOrder === SORT_BY_VALUES.VideoTitleAscending || sortOrder === SORT_BY_VALUES.VideoTitleDescending || sortOrder === SORT_BY_VALUES.AuthorAscending || - sortOrder === SORT_BY_VALUES.AuthorDescending + sortOrder === SORT_BY_VALUES.AuthorDescending || + sortOrder === SORT_BY_VALUES.VideoLengthAscending || + sortOrder === SORT_BY_VALUES.VideoLengthDescending ) { collator = new Intl.Collator([locale, 'en']) } @@ -45,6 +49,16 @@ function compareTwoPlaylistItems(a, b, sortOrder, collator) { return collator.compare(a.author, b.author) case SORT_BY_VALUES.AuthorDescending: return collator.compare(b.author, a.author) + case SORT_BY_VALUES.VideoLengthAscending: { + const aLengthForSort = (isNaN(a.lengthSeconds) || a.lengthSeconds === 0) ? 0 : a.lengthSeconds + const bLengthForSort = (isNaN(b.lengthSeconds) || b.lengthSeconds === 0) ? 0 : b.lengthSeconds + return aLengthForSort - bLengthForSort + } + case SORT_BY_VALUES.VideoLengthDescending: { + const aLengthForSort = (isNaN(a.lengthSeconds) || a.lengthSeconds === 0) ? 0 : a.lengthSeconds + const bLengthForSort = (isNaN(b.lengthSeconds) || b.lengthSeconds === 0) ? 0 : b.lengthSeconds + return bLengthForSort - aLengthForSort + } default: console.error(`Unknown sortOrder: ${sortOrder}`) return 0 diff --git a/src/renderer/views/Playlist/Playlist.js b/src/renderer/views/Playlist/Playlist.js index 8121a626f09b..efcd67e45996 100644 --- a/src/renderer/views/Playlist/Playlist.js +++ b/src/renderer/views/Playlist/Playlist.js @@ -180,6 +180,7 @@ export default defineComponent({ return this.sortOrder === SORT_BY_VALUES.Custom }, sortedPlaylistItems: function () { + this.showNoticesSometimes() return getSortedPlaylistItems(this.playlistItems, this.sortOrder, this.currentLocale) }, visiblePlaylistItems: function () { @@ -214,6 +215,10 @@ export default defineComponent({ return this.$t('Playlist.Sort By.AuthorAscending') case SORT_BY_VALUES.AuthorDescending: return this.$t('Playlist.Sort By.AuthorDescending') + case SORT_BY_VALUES.VideoLengthAscending: + return this.$t('Playlist.Sort By.VideoLengthAscending') + case SORT_BY_VALUES.VideoLengthDescending: + return this.$t('Playlist.Sort By.VideoLengthDescending') default: console.error(`Unknown sort: ${k}`) return k @@ -420,6 +425,20 @@ export default defineComponent({ showToast(this.$t('User Playlists.SinglePlaylistView.Toast.This playlist does not exist')) }, + showNoticesSometimes: function () { + if (this.alreadyShownNotice) return + if ( + this.sortOrder === SORT_BY_VALUES.VideoLengthAscending || + this.sortOrder === SORT_BY_VALUES.VideoLengthDescending + ) { + const anyVideoMissingLength = this.playlistItems.some(v => isNaN(v.lengthSeconds) || v.lengthSeconds === 0) + if (anyVideoMissingLength) { + showToast(this.$t('User Playlists.SinglePlaylistView.Toast.This playlist has a video with a length error'), 5000) + this.alreadyShownNotice = true + } + } + }, + getNextPage: function () { switch (this.infoSource) { case 'local': diff --git a/static/locales/en-GB.yaml b/static/locales/en-GB.yaml index 995766999782..51554b586015 100644 --- a/static/locales/en-GB.yaml +++ b/static/locales/en-GB.yaml @@ -174,6 +174,8 @@ User Playlists: This playlist is protected and cannot be removed.: This playlist is protected and cannot be removed. This playlist does not exist: This playlist does not exist + + This playlist has a video with a length error: This playlist contains at least one video that doesn't have a length, it will be sorted as if their length is zero. Playlist {playlistName} has been deleted.: Playlist {playlistName} has been deleted. Video has been removed: Video has been removed @@ -1006,6 +1008,8 @@ Playlist: AuthorAscending: Author (A-Z) AuthorDescending: Author (Z-A) VideoTitleAscending: Title (A-Z) + VideoLengthAscending: Length (Shortest first) + VideoLengthDescending: Length (Longest first) Toggle Theatre Mode: 'Toggle Theatre Mode' Change Format: Change Media Formats: 'Change Media Formats' diff --git a/static/locales/en-US.yaml b/static/locales/en-US.yaml index f4366ef53ebd..9413d002a835 100644 --- a/static/locales/en-US.yaml +++ b/static/locales/en-US.yaml @@ -236,6 +236,8 @@ User Playlists: Playlist {playlistName} is the new quick bookmark playlist.: Playlist {playlistName} is the new quick bookmark playlist. This playlist does not exist: This playlist does not exist + + This playlist has a video with a length error: This playlist contains at least one video that doesn't have a length, it will be sorted as if their length is zero. AddVideoPrompt: Select a playlist to add your N videos to: 'Select a playlist to add your video to | Select a playlist to add your {videoCount} videos to' N playlists selected: '{playlistCount} Selected' @@ -928,6 +930,8 @@ Playlist: AuthorDescending: Author (Z-A) VideoTitleAscending: Title (A-Z) VideoTitleDescending: Title (Z-A) + VideoLengthAscending: Length (Shortest first) + VideoLengthDescending: Length (Longest first) Custom: Custom # On Video Watch Page From 959c66f9ca6e28d6d951a48e02db9298c6e36271 Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Fri, 27 Sep 2024 14:04:06 +1200 Subject: [PATCH 02/12] feat: declare alreadyShownNotice property --- src/renderer/views/Playlist/Playlist.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/renderer/views/Playlist/Playlist.js b/src/renderer/views/Playlist/Playlist.js index efcd67e45996..ff8cc6610b41 100644 --- a/src/renderer/views/Playlist/Playlist.js +++ b/src/renderer/views/Playlist/Playlist.js @@ -74,6 +74,7 @@ export default defineComponent({ getPlaylistInfoDebounce: function() {}, playlistInEditMode: false, forceListView: false, + alreadyShownNotice: false, videoSearchQuery: '', From 1508cc9fd3f1c4407cc24d8e84dead7771b87689 Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Sat, 28 Sep 2024 23:11:43 +1200 Subject: [PATCH 03/12] refactor: change VideoLength to VideoDuration --- src/renderer/helpers/playlists.js | 12 ++++++------ src/renderer/views/Playlist/Playlist.js | 12 ++++++------ static/locales/en-GB.yaml | 4 ++-- static/locales/en-US.yaml | 4 ++-- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/renderer/helpers/playlists.js b/src/renderer/helpers/playlists.js index e21b2e53084e..99709634cb14 100644 --- a/src/renderer/helpers/playlists.js +++ b/src/renderer/helpers/playlists.js @@ -5,8 +5,8 @@ export const SORT_BY_VALUES = { AuthorDescending: 'author_descending', VideoTitleAscending: 'video_title_ascending', VideoTitleDescending: 'video_title_descending', - VideoLengthAscending: 'video_length_ascending', - VideoLengthDescending: 'video_length_descending', + VideoDurationAscending: 'video_length_ascending', + VideoDurationDescending: 'video_length_descending', Custom: 'custom' } @@ -22,8 +22,8 @@ export function getSortedPlaylistItems(playlistItems, sortOrder, locale, reverse sortOrder === SORT_BY_VALUES.VideoTitleDescending || sortOrder === SORT_BY_VALUES.AuthorAscending || sortOrder === SORT_BY_VALUES.AuthorDescending || - sortOrder === SORT_BY_VALUES.VideoLengthAscending || - sortOrder === SORT_BY_VALUES.VideoLengthDescending + sortOrder === SORT_BY_VALUES.VideoDurationAscending || + sortOrder === SORT_BY_VALUES.VideoDurationDescending ) { collator = new Intl.Collator([locale, 'en']) } @@ -49,12 +49,12 @@ function compareTwoPlaylistItems(a, b, sortOrder, collator) { return collator.compare(a.author, b.author) case SORT_BY_VALUES.AuthorDescending: return collator.compare(b.author, a.author) - case SORT_BY_VALUES.VideoLengthAscending: { + case SORT_BY_VALUES.VideoDurationAscending: { const aLengthForSort = (isNaN(a.lengthSeconds) || a.lengthSeconds === 0) ? 0 : a.lengthSeconds const bLengthForSort = (isNaN(b.lengthSeconds) || b.lengthSeconds === 0) ? 0 : b.lengthSeconds return aLengthForSort - bLengthForSort } - case SORT_BY_VALUES.VideoLengthDescending: { + case SORT_BY_VALUES.VideoDurationDescending: { const aLengthForSort = (isNaN(a.lengthSeconds) || a.lengthSeconds === 0) ? 0 : a.lengthSeconds const bLengthForSort = (isNaN(b.lengthSeconds) || b.lengthSeconds === 0) ? 0 : b.lengthSeconds return bLengthForSort - aLengthForSort diff --git a/src/renderer/views/Playlist/Playlist.js b/src/renderer/views/Playlist/Playlist.js index ff8cc6610b41..bec07552363d 100644 --- a/src/renderer/views/Playlist/Playlist.js +++ b/src/renderer/views/Playlist/Playlist.js @@ -216,10 +216,10 @@ export default defineComponent({ return this.$t('Playlist.Sort By.AuthorAscending') case SORT_BY_VALUES.AuthorDescending: return this.$t('Playlist.Sort By.AuthorDescending') - case SORT_BY_VALUES.VideoLengthAscending: - return this.$t('Playlist.Sort By.VideoLengthAscending') - case SORT_BY_VALUES.VideoLengthDescending: - return this.$t('Playlist.Sort By.VideoLengthDescending') + case SORT_BY_VALUES.VideoDurationAscending: + return this.$t('Playlist.Sort By.VideoDurationAscending') + case SORT_BY_VALUES.VideoDurationDescending: + return this.$t('Playlist.Sort By.VideoDurationDescending') default: console.error(`Unknown sort: ${k}`) return k @@ -429,8 +429,8 @@ export default defineComponent({ showNoticesSometimes: function () { if (this.alreadyShownNotice) return if ( - this.sortOrder === SORT_BY_VALUES.VideoLengthAscending || - this.sortOrder === SORT_BY_VALUES.VideoLengthDescending + this.sortOrder === SORT_BY_VALUES.VideoDurationAscending || + this.sortOrder === SORT_BY_VALUES.VideoDurationDescending ) { const anyVideoMissingLength = this.playlistItems.some(v => isNaN(v.lengthSeconds) || v.lengthSeconds === 0) if (anyVideoMissingLength) { diff --git a/static/locales/en-GB.yaml b/static/locales/en-GB.yaml index 51554b586015..c726d5a477b7 100644 --- a/static/locales/en-GB.yaml +++ b/static/locales/en-GB.yaml @@ -1008,8 +1008,8 @@ Playlist: AuthorAscending: Author (A-Z) AuthorDescending: Author (Z-A) VideoTitleAscending: Title (A-Z) - VideoLengthAscending: Length (Shortest first) - VideoLengthDescending: Length (Longest first) + VideoDurationAscending: Length (Shortest first) + VideoDurationDescending: Length (Longest first) Toggle Theatre Mode: 'Toggle Theatre Mode' Change Format: Change Media Formats: 'Change Media Formats' diff --git a/static/locales/en-US.yaml b/static/locales/en-US.yaml index 9413d002a835..d602cf977e67 100644 --- a/static/locales/en-US.yaml +++ b/static/locales/en-US.yaml @@ -930,8 +930,8 @@ Playlist: AuthorDescending: Author (Z-A) VideoTitleAscending: Title (A-Z) VideoTitleDescending: Title (Z-A) - VideoLengthAscending: Length (Shortest first) - VideoLengthDescending: Length (Longest first) + VideoDurationAscending: Length (Shortest first) + VideoDurationDescending: Length (Longest first) Custom: Custom # On Video Watch Page From 403d81098a9de460b649a09820ad8e15cf64eada Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Sat, 28 Sep 2024 23:49:15 +1200 Subject: [PATCH 04/12] refactor: update translations to also use duration --- src/renderer/views/Playlist/Playlist.js | 6 +++--- static/locales/en-GB.yaml | 6 +++--- static/locales/en-US.yaml | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/renderer/views/Playlist/Playlist.js b/src/renderer/views/Playlist/Playlist.js index bec07552363d..7e1abe3a4820 100644 --- a/src/renderer/views/Playlist/Playlist.js +++ b/src/renderer/views/Playlist/Playlist.js @@ -432,9 +432,9 @@ export default defineComponent({ this.sortOrder === SORT_BY_VALUES.VideoDurationAscending || this.sortOrder === SORT_BY_VALUES.VideoDurationDescending ) { - const anyVideoMissingLength = this.playlistItems.some(v => isNaN(v.lengthSeconds) || v.lengthSeconds === 0) - if (anyVideoMissingLength) { - showToast(this.$t('User Playlists.SinglePlaylistView.Toast.This playlist has a video with a length error'), 5000) + const anyVideoMissingDuration = this.playlistItems.some(v => isNaN(v.lengthSeconds) || v.lengthSeconds === 0) + if (anyVideoMissingDuration) { + showToast(this.$t('User Playlists.SinglePlaylistView.Toast.This playlist has a video with a duration error'), 5000) this.alreadyShownNotice = true } } diff --git a/static/locales/en-GB.yaml b/static/locales/en-GB.yaml index c726d5a477b7..b9269c166f5e 100644 --- a/static/locales/en-GB.yaml +++ b/static/locales/en-GB.yaml @@ -175,7 +175,7 @@ User Playlists: and cannot be removed. This playlist does not exist: This playlist does not exist - This playlist has a video with a length error: This playlist contains at least one video that doesn't have a length, it will be sorted as if their length is zero. + This playlist has a video with a duration error: This playlist contains at least one video that doesn't have a duration, it will be sorted as if their duration is zero. Playlist {playlistName} has been deleted.: Playlist {playlistName} has been deleted. Video has been removed: Video has been removed @@ -1008,8 +1008,8 @@ Playlist: AuthorAscending: Author (A-Z) AuthorDescending: Author (Z-A) VideoTitleAscending: Title (A-Z) - VideoDurationAscending: Length (Shortest first) - VideoDurationDescending: Length (Longest first) + VideoDurationAscending: Duration (Shortest first) + VideoDurationDescending: Duration (Longest first) Toggle Theatre Mode: 'Toggle Theatre Mode' Change Format: Change Media Formats: 'Change Media Formats' diff --git a/static/locales/en-US.yaml b/static/locales/en-US.yaml index d602cf977e67..430a73dece2b 100644 --- a/static/locales/en-US.yaml +++ b/static/locales/en-US.yaml @@ -237,7 +237,7 @@ User Playlists: This playlist does not exist: This playlist does not exist - This playlist has a video with a length error: This playlist contains at least one video that doesn't have a length, it will be sorted as if their length is zero. + This playlist has a video with a duration error: This playlist contains at least one video that doesn't have a duration, it will be sorted as if their duration is zero. AddVideoPrompt: Select a playlist to add your N videos to: 'Select a playlist to add your video to | Select a playlist to add your {videoCount} videos to' N playlists selected: '{playlistCount} Selected' @@ -930,8 +930,8 @@ Playlist: AuthorDescending: Author (Z-A) VideoTitleAscending: Title (A-Z) VideoTitleDescending: Title (Z-A) - VideoDurationAscending: Length (Shortest first) - VideoDurationDescending: Length (Longest first) + VideoDurationAscending: Duration (Shortest first) + VideoDurationDescending: Duration (Longest first) Custom: Custom # On Video Watch Page From e0a72df6039da1af7e30b733b40d67ce1545847a Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Sat, 28 Sep 2024 23:52:06 +1200 Subject: [PATCH 05/12] refactor: update SORT_BY_VALUES to also use duration --- src/renderer/helpers/playlists.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderer/helpers/playlists.js b/src/renderer/helpers/playlists.js index 99709634cb14..4fda70a49983 100644 --- a/src/renderer/helpers/playlists.js +++ b/src/renderer/helpers/playlists.js @@ -5,8 +5,8 @@ export const SORT_BY_VALUES = { AuthorDescending: 'author_descending', VideoTitleAscending: 'video_title_ascending', VideoTitleDescending: 'video_title_descending', - VideoDurationAscending: 'video_length_ascending', - VideoDurationDescending: 'video_length_descending', + VideoDurationAscending: 'video_duration_ascending', + VideoDurationDescending: 'video_duration_descending', Custom: 'custom' } From 521c5a39615797ec58014ebee04b09945b9f576d Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Sun, 29 Sep 2024 23:56:20 +1300 Subject: [PATCH 06/12] refactor: deduplicate repeated code into seperate function --- src/renderer/helpers/playlists.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/renderer/helpers/playlists.js b/src/renderer/helpers/playlists.js index 4fda70a49983..19ace71a4b3c 100644 --- a/src/renderer/helpers/playlists.js +++ b/src/renderer/helpers/playlists.js @@ -35,6 +35,10 @@ export function getSortedPlaylistItems(playlistItems, sortOrder, locale, reverse }) } +function getDurationForSort(a) { + return (isNaN(a.lengthSeconds) || a.lengthSeconds === 0) ? 0 : a.lengthSeconds +} + function compareTwoPlaylistItems(a, b, sortOrder, collator) { switch (sortOrder) { case SORT_BY_VALUES.DateAddedNewest: @@ -50,14 +54,10 @@ function compareTwoPlaylistItems(a, b, sortOrder, collator) { case SORT_BY_VALUES.AuthorDescending: return collator.compare(b.author, a.author) case SORT_BY_VALUES.VideoDurationAscending: { - const aLengthForSort = (isNaN(a.lengthSeconds) || a.lengthSeconds === 0) ? 0 : a.lengthSeconds - const bLengthForSort = (isNaN(b.lengthSeconds) || b.lengthSeconds === 0) ? 0 : b.lengthSeconds - return aLengthForSort - bLengthForSort + return getDurationForSort(a) - getDurationForSort(b) } case SORT_BY_VALUES.VideoDurationDescending: { - const aLengthForSort = (isNaN(a.lengthSeconds) || a.lengthSeconds === 0) ? 0 : a.lengthSeconds - const bLengthForSort = (isNaN(b.lengthSeconds) || b.lengthSeconds === 0) ? 0 : b.lengthSeconds - return bLengthForSort - aLengthForSort + return getDurationForSort(b) - getDurationForSort(a) } default: console.error(`Unknown sortOrder: ${sortOrder}`) From 5714ebbfea715ff7630349de8d740a2e3cac388b Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Mon, 30 Sep 2024 18:19:00 +1300 Subject: [PATCH 07/12] feat: if the duration is missing, try and grab it from the history cache --- src/renderer/views/Playlist/Playlist.js | 37 +++++++++++++++++-------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/renderer/views/Playlist/Playlist.js b/src/renderer/views/Playlist/Playlist.js index 7e1abe3a4820..25b3fe19ad7c 100644 --- a/src/renderer/views/Playlist/Playlist.js +++ b/src/renderer/views/Playlist/Playlist.js @@ -20,6 +20,7 @@ import { getIconForSortPreference, setPublishedTimestampsInvidious, showToast, + deepCopy, } from '../../helpers/utils' import { invidiousGetPlaylistInfo, youtubeImageUrlToInvidious } from '../../helpers/api/invidious' import { getSortedPlaylistItems, SORT_BY_VALUES } from '../../helpers/playlists' @@ -181,7 +182,14 @@ export default defineComponent({ return this.sortOrder === SORT_BY_VALUES.Custom }, sortedPlaylistItems: function () { - this.showNoticesSometimes() + if ( + this.sortOrder === SORT_BY_VALUES.VideoDurationAscending || + this.sortOrder === SORT_BY_VALUES.VideoDurationDescending + ) { + const playlistItems = this.getDurationPlaylistItems() + this.showNoticesSometimes(playlistItems) + return getSortedPlaylistItems(playlistItems, this.sortOrder, this.currentLocale) + } return getSortedPlaylistItems(this.playlistItems, this.sortOrder, this.currentLocale) }, visiblePlaylistItems: function () { @@ -426,18 +434,25 @@ export default defineComponent({ showToast(this.$t('User Playlists.SinglePlaylistView.Toast.This playlist does not exist')) }, - showNoticesSometimes: function () { - if (this.alreadyShownNotice) return - if ( - this.sortOrder === SORT_BY_VALUES.VideoDurationAscending || - this.sortOrder === SORT_BY_VALUES.VideoDurationDescending - ) { - const anyVideoMissingDuration = this.playlistItems.some(v => isNaN(v.lengthSeconds) || v.lengthSeconds === 0) - if (anyVideoMissingDuration) { - showToast(this.$t('User Playlists.SinglePlaylistView.Toast.This playlist has a video with a duration error'), 5000) - this.alreadyShownNotice = true + getDurationPlaylistItems: function () { + const modifiedPlaylistItems = deepCopy(this.playlistItems) + for (const v in modifiedPlaylistItems) { + const lengthSeconds = modifiedPlaylistItems[v].lengthSeconds + if (isNaN(lengthSeconds) || lengthSeconds === 0 || lengthSeconds === '') { + const videoHistory = this.$store.getters.getHistoryCacheById[modifiedPlaylistItems[v].videoId] + if (typeof videoHistory !== 'undefined') modifiedPlaylistItems[v].lengthSeconds = videoHistory.lengthSeconds } } + return modifiedPlaylistItems + }, + + showNoticesSometimes: function (playlistItems) { + if (this.alreadyShownNotice) return + const anyVideoMissingDuration = playlistItems.some(v => isNaN(v.lengthSeconds) || v.lengthSeconds === 0 || v.lengthSeconds === '') + if (anyVideoMissingDuration) { + showToast(this.$t('User Playlists.SinglePlaylistView.Toast.This playlist has a video with a duration error'), 5000) + this.alreadyShownNotice = true + } }, getNextPage: function () { From 3c73483c45fedac540c5d1a827fb77b42bfde998 Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Mon, 30 Sep 2024 18:30:14 +1300 Subject: [PATCH 08/12] fix: if the lengthSeconds is a string, treat it as zero (fix for RSS) --- src/renderer/helpers/playlists.js | 8 ++++---- src/renderer/views/Playlist/Playlist.js | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/renderer/helpers/playlists.js b/src/renderer/helpers/playlists.js index 19ace71a4b3c..6f4723f5da90 100644 --- a/src/renderer/helpers/playlists.js +++ b/src/renderer/helpers/playlists.js @@ -35,8 +35,8 @@ export function getSortedPlaylistItems(playlistItems, sortOrder, locale, reverse }) } -function getDurationForSort(a) { - return (isNaN(a.lengthSeconds) || a.lengthSeconds === 0) ? 0 : a.lengthSeconds +export function checkDurationFromVideo(a) { + return (isNaN(a.lengthSeconds) || a.lengthSeconds === 0 || typeof a.lengthSeconds === 'string') ? 0 : a.lengthSeconds } function compareTwoPlaylistItems(a, b, sortOrder, collator) { @@ -54,10 +54,10 @@ function compareTwoPlaylistItems(a, b, sortOrder, collator) { case SORT_BY_VALUES.AuthorDescending: return collator.compare(b.author, a.author) case SORT_BY_VALUES.VideoDurationAscending: { - return getDurationForSort(a) - getDurationForSort(b) + return checkDurationFromVideo(a) - checkDurationFromVideo(b) } case SORT_BY_VALUES.VideoDurationDescending: { - return getDurationForSort(b) - getDurationForSort(a) + return checkDurationFromVideo(b) - checkDurationFromVideo(a) } default: console.error(`Unknown sortOrder: ${sortOrder}`) diff --git a/src/renderer/views/Playlist/Playlist.js b/src/renderer/views/Playlist/Playlist.js index 25b3fe19ad7c..e70411e49c1e 100644 --- a/src/renderer/views/Playlist/Playlist.js +++ b/src/renderer/views/Playlist/Playlist.js @@ -438,7 +438,7 @@ export default defineComponent({ const modifiedPlaylistItems = deepCopy(this.playlistItems) for (const v in modifiedPlaylistItems) { const lengthSeconds = modifiedPlaylistItems[v].lengthSeconds - if (isNaN(lengthSeconds) || lengthSeconds === 0 || lengthSeconds === '') { + if (isNaN(lengthSeconds) || lengthSeconds === 0 || typeof lengthSeconds === 'string') { const videoHistory = this.$store.getters.getHistoryCacheById[modifiedPlaylistItems[v].videoId] if (typeof videoHistory !== 'undefined') modifiedPlaylistItems[v].lengthSeconds = videoHistory.lengthSeconds } @@ -448,7 +448,7 @@ export default defineComponent({ showNoticesSometimes: function (playlistItems) { if (this.alreadyShownNotice) return - const anyVideoMissingDuration = playlistItems.some(v => isNaN(v.lengthSeconds) || v.lengthSeconds === 0 || v.lengthSeconds === '') + const anyVideoMissingDuration = playlistItems.some(v => isNaN(v.lengthSeconds) || v.lengthSeconds === 0 || typeof v.lengthSeconds === 'string') if (anyVideoMissingDuration) { showToast(this.$t('User Playlists.SinglePlaylistView.Toast.This playlist has a video with a duration error'), 5000) this.alreadyShownNotice = true From c9030442f888ede176ed11213e0338c691fd1e7a Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Mon, 30 Sep 2024 21:18:58 +1300 Subject: [PATCH 09/12] refactor: make checking the duration of a video use the same helper function everywhere --- src/renderer/helpers/playlists.js | 2 +- src/renderer/views/Playlist/Playlist.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/renderer/helpers/playlists.js b/src/renderer/helpers/playlists.js index 6f4723f5da90..767d18b61460 100644 --- a/src/renderer/helpers/playlists.js +++ b/src/renderer/helpers/playlists.js @@ -36,7 +36,7 @@ export function getSortedPlaylistItems(playlistItems, sortOrder, locale, reverse } export function checkDurationFromVideo(a) { - return (isNaN(a.lengthSeconds) || a.lengthSeconds === 0 || typeof a.lengthSeconds === 'string') ? 0 : a.lengthSeconds + return (isNaN(a.lengthSeconds) || a.lengthSeconds === 0 || typeof a.lengthSeconds !== 'number') ? 0 : a.lengthSeconds } function compareTwoPlaylistItems(a, b, sortOrder, collator) { diff --git a/src/renderer/views/Playlist/Playlist.js b/src/renderer/views/Playlist/Playlist.js index e70411e49c1e..09d4d47d5835 100644 --- a/src/renderer/views/Playlist/Playlist.js +++ b/src/renderer/views/Playlist/Playlist.js @@ -23,7 +23,7 @@ import { deepCopy, } from '../../helpers/utils' import { invidiousGetPlaylistInfo, youtubeImageUrlToInvidious } from '../../helpers/api/invidious' -import { getSortedPlaylistItems, SORT_BY_VALUES } from '../../helpers/playlists' +import { getSortedPlaylistItems, checkDurationFromVideo, SORT_BY_VALUES } from '../../helpers/playlists' import packageDetails from '../../../../package.json' import { MOBILE_WIDTH_THRESHOLD, PLAYLIST_HEIGHT_FORCE_LIST_THRESHOLD } from '../../../constants' @@ -437,8 +437,8 @@ export default defineComponent({ getDurationPlaylistItems: function () { const modifiedPlaylistItems = deepCopy(this.playlistItems) for (const v in modifiedPlaylistItems) { - const lengthSeconds = modifiedPlaylistItems[v].lengthSeconds - if (isNaN(lengthSeconds) || lengthSeconds === 0 || typeof lengthSeconds === 'string') { + const video = modifiedPlaylistItems[v] + if (!checkDurationFromVideo(video)) { const videoHistory = this.$store.getters.getHistoryCacheById[modifiedPlaylistItems[v].videoId] if (typeof videoHistory !== 'undefined') modifiedPlaylistItems[v].lengthSeconds = videoHistory.lengthSeconds } @@ -448,7 +448,7 @@ export default defineComponent({ showNoticesSometimes: function (playlistItems) { if (this.alreadyShownNotice) return - const anyVideoMissingDuration = playlistItems.some(v => isNaN(v.lengthSeconds) || v.lengthSeconds === 0 || typeof v.lengthSeconds === 'string') + const anyVideoMissingDuration = playlistItems.some(v => checkDurationFromVideo(v)) if (anyVideoMissingDuration) { showToast(this.$t('User Playlists.SinglePlaylistView.Toast.This playlist has a video with a duration error'), 5000) this.alreadyShownNotice = true From aebeb447bb801d510358be642528c02cb05241f7 Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Mon, 30 Sep 2024 21:23:42 +1300 Subject: [PATCH 10/12] fix/refactor: make sure toast only shows when a item is missing a duration/use a foreach loop to check the videos in a playlist --- src/renderer/views/Playlist/Playlist.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/renderer/views/Playlist/Playlist.js b/src/renderer/views/Playlist/Playlist.js index 09d4d47d5835..7b3db5760f7e 100644 --- a/src/renderer/views/Playlist/Playlist.js +++ b/src/renderer/views/Playlist/Playlist.js @@ -436,19 +436,18 @@ export default defineComponent({ getDurationPlaylistItems: function () { const modifiedPlaylistItems = deepCopy(this.playlistItems) - for (const v in modifiedPlaylistItems) { - const video = modifiedPlaylistItems[v] + modifiedPlaylistItems.forEach(video => { if (!checkDurationFromVideo(video)) { - const videoHistory = this.$store.getters.getHistoryCacheById[modifiedPlaylistItems[v].videoId] - if (typeof videoHistory !== 'undefined') modifiedPlaylistItems[v].lengthSeconds = videoHistory.lengthSeconds + const videoHistory = this.$store.getters.getHistoryCacheById[video.videoId] + if (typeof videoHistory !== 'undefined') video.lengthSeconds = videoHistory.lengthSeconds } - } + }) return modifiedPlaylistItems }, showNoticesSometimes: function (playlistItems) { if (this.alreadyShownNotice) return - const anyVideoMissingDuration = playlistItems.some(v => checkDurationFromVideo(v)) + const anyVideoMissingDuration = playlistItems.some(v => !checkDurationFromVideo(v)) if (anyVideoMissingDuration) { showToast(this.$t('User Playlists.SinglePlaylistView.Toast.This playlist has a video with a duration error'), 5000) this.alreadyShownNotice = true From 0613bbfaf095a8532c68e8232c96bde375a881c2 Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Mon, 30 Sep 2024 22:50:50 +0000 Subject: [PATCH 11/12] feat: apply suggestions from code review Co-authored-by: PikachuEXE --- src/renderer/helpers/playlists.js | 17 +++++++++++++---- src/renderer/views/Playlist/Playlist.js | 6 +++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/renderer/helpers/playlists.js b/src/renderer/helpers/playlists.js index 767d18b61460..93a844c1b017 100644 --- a/src/renderer/helpers/playlists.js +++ b/src/renderer/helpers/playlists.js @@ -35,8 +35,17 @@ export function getSortedPlaylistItems(playlistItems, sortOrder, locale, reverse }) } -export function checkDurationFromVideo(a) { - return (isNaN(a.lengthSeconds) || a.lengthSeconds === 0 || typeof a.lengthSeconds !== 'number') ? 0 : a.lengthSeconds +export function videoDurationPresent(v) { + if (typeof v.lengthSeconds !== 'number') { return false } + + return !(isNaN(v.lengthSeconds) || v.lengthSeconds === 0) +} + +function videoDurationWithFallback(v) { + if (videoDurationPresent(v)) { return v.lengthSeconds } + + // Fallback + return 0 } function compareTwoPlaylistItems(a, b, sortOrder, collator) { @@ -54,10 +63,10 @@ function compareTwoPlaylistItems(a, b, sortOrder, collator) { case SORT_BY_VALUES.AuthorDescending: return collator.compare(b.author, a.author) case SORT_BY_VALUES.VideoDurationAscending: { - return checkDurationFromVideo(a) - checkDurationFromVideo(b) + return videoDurationWithFallback(a) - videoDurationWithFallback(b) } case SORT_BY_VALUES.VideoDurationDescending: { - return checkDurationFromVideo(b) - checkDurationFromVideo(a) + return videoDurationWithFallback(b) - videoDurationWithFallback(a) } default: console.error(`Unknown sortOrder: ${sortOrder}`) diff --git a/src/renderer/views/Playlist/Playlist.js b/src/renderer/views/Playlist/Playlist.js index 7b3db5760f7e..fc6ef516984a 100644 --- a/src/renderer/views/Playlist/Playlist.js +++ b/src/renderer/views/Playlist/Playlist.js @@ -23,7 +23,7 @@ import { deepCopy, } from '../../helpers/utils' import { invidiousGetPlaylistInfo, youtubeImageUrlToInvidious } from '../../helpers/api/invidious' -import { getSortedPlaylistItems, checkDurationFromVideo, SORT_BY_VALUES } from '../../helpers/playlists' +import { getSortedPlaylistItems, videoDurationPresent, SORT_BY_VALUES } from '../../helpers/playlists' import packageDetails from '../../../../package.json' import { MOBILE_WIDTH_THRESHOLD, PLAYLIST_HEIGHT_FORCE_LIST_THRESHOLD } from '../../../constants' @@ -437,7 +437,7 @@ export default defineComponent({ getDurationPlaylistItems: function () { const modifiedPlaylistItems = deepCopy(this.playlistItems) modifiedPlaylistItems.forEach(video => { - if (!checkDurationFromVideo(video)) { + if (!videoDurationPresent(video)) { const videoHistory = this.$store.getters.getHistoryCacheById[video.videoId] if (typeof videoHistory !== 'undefined') video.lengthSeconds = videoHistory.lengthSeconds } @@ -447,7 +447,7 @@ export default defineComponent({ showNoticesSometimes: function (playlistItems) { if (this.alreadyShownNotice) return - const anyVideoMissingDuration = playlistItems.some(v => !checkDurationFromVideo(v)) + const anyVideoMissingDuration = playlistItems.some(v => !videoDurationPresent(v)) if (anyVideoMissingDuration) { showToast(this.$t('User Playlists.SinglePlaylistView.Toast.This playlist has a video with a duration error'), 5000) this.alreadyShownNotice = true From 8930710e63b33df33634b5e5bcaef268c6567622 Mon Sep 17 00:00:00 2001 From: Thomas Dickson Date: Tue, 1 Oct 2024 11:53:11 +1300 Subject: [PATCH 12/12] fix: make lint test pass --- src/renderer/helpers/playlists.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderer/helpers/playlists.js b/src/renderer/helpers/playlists.js index 93a844c1b017..0a458c1b7fe5 100644 --- a/src/renderer/helpers/playlists.js +++ b/src/renderer/helpers/playlists.js @@ -37,7 +37,7 @@ export function getSortedPlaylistItems(playlistItems, sortOrder, locale, reverse export function videoDurationPresent(v) { if (typeof v.lengthSeconds !== 'number') { return false } - + return !(isNaN(v.lengthSeconds) || v.lengthSeconds === 0) }