-
Notifications
You must be signed in to change notification settings - Fork 902
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Play next/prev video after removing current video from the playlist #5158
Merged
FreeTubeBot
merged 7 commits into
FreeTubeApp:development
from
kommunarr:feat/play-next-or-prev-from-deleted
Jun 6, 2024
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
62af97a
Remove un-utilized allowPlayingVideoRemoval disabled option
kommunarr d63bec6
Have currently deleted video entry become the prior entry in the play…
kommunarr 7e398a8
Fix issue where first video is skipped when current video after remov…
kommunarr d00bc83
Handle first video in playlist being removed
kommunarr 0247aca
Fix shuffle prevVideo issue
kommunarr 4f37130
Fix wrong variable for reversal bug
kommunarr 257681e
Implement suggested optimization
kommunarr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,7 @@ export default defineComponent({ | |
loopEnabled: false, | ||
reversePlaylist: false, | ||
pauseOnCurrentVideo: false, | ||
prevVideoBeforeDeletion: null, | ||
channelName: '', | ||
channelId: '', | ||
playlistTitle: '', | ||
|
@@ -85,15 +86,7 @@ export default defineComponent({ | |
}, | ||
|
||
currentVideoIndexZeroBased: function () { | ||
return this.playlistItems.findIndex((item) => { | ||
if (item.playlistItemId != null && this.playlistItemId != null) { | ||
return item.playlistItemId === this.playlistItemId | ||
} else if (item.videoId != null) { | ||
return item.videoId === this.videoId | ||
} else { | ||
return item.id === this.videoId | ||
} | ||
}) | ||
return this.findIndexOfCurrentVideoInPlaylist(this.playlistItems) | ||
}, | ||
currentVideoIndexOneBased: function () { | ||
return this.currentVideoIndexZeroBased + 1 | ||
|
@@ -123,19 +116,7 @@ export default defineComponent({ | |
|
||
videoIndexInPlaylistItems: function () { | ||
const playlistItems = this.shuffleEnabled ? this.randomizedPlaylistItems : this.playlistItems | ||
|
||
return playlistItems.findIndex((item) => { | ||
if (item.playlistItemId != null && this.playlistItemId != null) { | ||
return item.playlistItemId === this.playlistItemId | ||
} else if (item.videoId != null) { | ||
return item.videoId === this.videoId | ||
} else { | ||
return item.id === this.videoId | ||
} | ||
}) | ||
}, | ||
videoIsFirstPlaylistItem: function () { | ||
return this.videoIndexInPlaylistItems === 0 | ||
return this.findIndexOfCurrentVideoInPlaylist(playlistItems) | ||
}, | ||
videoIsLastPlaylistItem: function () { | ||
return this.videoIndexInPlaylistItems === (this.playlistItems.length - 1) | ||
|
@@ -160,12 +141,12 @@ export default defineComponent({ | |
}, | ||
selectedUserPlaylistVideoCount () { | ||
// Re-fetch from local store when current user playlist updated | ||
this.parseUserPlaylist(this.selectedUserPlaylist, { allowPlayingVideoRemoval: true }) | ||
this.parseUserPlaylist(this.selectedUserPlaylist) | ||
this.shufflePlaylistItems() | ||
}, | ||
selectedUserPlaylistLastUpdatedAt () { | ||
// Re-fetch from local store when current user playlist updated | ||
this.parseUserPlaylist(this.selectedUserPlaylist, { allowPlayingVideoRemoval: true }) | ||
this.parseUserPlaylist(this.selectedUserPlaylist) | ||
}, | ||
videoId: function (newId, oldId) { | ||
// Check if next video is from the shuffled list or if the user clicked a different video | ||
|
@@ -184,6 +165,9 @@ export default defineComponent({ | |
} | ||
} | ||
}, | ||
playlistItemId: function () { | ||
this.prevVideoBeforeDeletion = null | ||
}, | ||
watchViewLoading: function (newVal, oldVal) { | ||
// This component is loaded/rendered before watch view loaded | ||
if (oldVal && !newVal) { | ||
|
@@ -232,6 +216,23 @@ export default defineComponent({ | |
} | ||
}, | ||
methods: { | ||
findIndexOfCurrentVideoInPlaylist: function (playlist) { | ||
const playlistItemId = this.playlistItemId | ||
const prevVideoBeforeDeletion = this.prevVideoBeforeDeletion | ||
const videoId = this.videoId | ||
return playlist.findIndex((item) => { | ||
if (item.playlistItemId && (playlistItemId || prevVideoBeforeDeletion?.playlistItemId)) { | ||
return item.playlistItemId === playlistItemId || item.playlistItemId === prevVideoBeforeDeletion?.playlistItemId | ||
} else if (item.videoId) { | ||
return item.videoId === videoId || item.videoId === prevVideoBeforeDeletion?.videoId | ||
} else if (item.id) { | ||
return item.id === videoId || item.id === prevVideoBeforeDeletion?.videoId | ||
} | ||
|
||
return false | ||
}) | ||
}, | ||
|
||
getPlaylistInfoWithDelay: function () { | ||
if (this.getPlaylistInfoRun) { return } | ||
|
||
|
@@ -348,8 +349,23 @@ export default defineComponent({ | |
playlistType: this.playlistType, | ||
} | ||
|
||
const videoIndex = this.videoIndexInPlaylistItems | ||
const targetVideoIndex = (this.videoIsFirstPlaylistItem || this.videoIsNotPlaylistItem) ? this.playlistItems.length - 1 : videoIndex - 1 | ||
let videoIndex = this.videoIndexInPlaylistItems | ||
|
||
/* | ||
* When the current video being watched in the playlist is deleted, | ||
* the previous video is shown as the "current" one. | ||
* So if we want to play the previous video, in this case, | ||
* we actually want to actually play the "current" video. | ||
* The only exception is when shuffle is enabled, as we don't actually | ||
* want to play the last sequential video with shuffle. | ||
*/ | ||
if (this.prevVideoBeforeDeletion && !this.shuffleEnabled) { | ||
videoIndex++ | ||
} | ||
|
||
// Wrap around to the end of the playlist only if there are no remaining earlier videos | ||
const targetVideoIndex = (videoIndex === 0 || this.videoIsNotPlaylistItem) ? this.playlistItems.length - 1 : videoIndex - 1 | ||
|
||
if (this.shuffleEnabled) { | ||
const targetPlaylistItem = this.randomizedPlaylistItems[targetVideoIndex] | ||
|
||
|
@@ -466,24 +482,29 @@ export default defineComponent({ | |
}) | ||
}, | ||
|
||
parseUserPlaylist: function (playlist, { allowPlayingVideoRemoval = true } = {}) { | ||
parseUserPlaylist: function (playlist) { | ||
this.playlistTitle = playlist.playlistName | ||
this.channelName = '' | ||
this.channelId = '' | ||
|
||
if (this.playlistItems.length === 0 || allowPlayingVideoRemoval) { | ||
this.playlistItems = playlist.videos | ||
} else { | ||
// `this.currentVideo` relies on `playlistItems` | ||
const latestPlaylistContainsCurrentVideo = playlist.videos.some(v => v.playlistItemId === this.playlistItemId) | ||
// Only update list of videos if latest video list still contains currently playing video | ||
if (latestPlaylistContainsCurrentVideo) { | ||
this.playlistItems = playlist.videos | ||
} | ||
const isCurrentVideoInParsedPlaylist = this.findIndexOfCurrentVideoInPlaylist(playlist.videos) !== -1 | ||
if (!isCurrentVideoInParsedPlaylist) { | ||
Comment on lines
+490
to
+491
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why use double negative here... (and below) |
||
// grab 2nd video if the 1st one is current & deleted | ||
// or the prior video in the list before the current video's deletion | ||
const targetVideoIndex = (this.currentVideoIndexZeroBased === 0 ? 1 : this.currentVideoIndexZeroBased - 1) | ||
this.prevVideoBeforeDeletion = this.playlistItems[targetVideoIndex] | ||
} | ||
|
||
let playlistItems = playlist.videos | ||
if (this.reversePlaylist) { | ||
this.playlistItems = this.playlistItems.toReversed() | ||
playlistItems = playlistItems.toReversed() | ||
} | ||
this.playlistItems = playlistItems | ||
|
||
// grab the first video of the parsed playlit if the current video is not in either the current or parsed data | ||
// (e.g., reloading the page after the current video has already been removed from the playlist) | ||
if (!isCurrentVideoInParsedPlaylist && this.prevVideoBeforeDeletion == null) { | ||
this.prevVideoBeforeDeletion = this.playlistItems[0] | ||
} | ||
|
||
this.isLoading = false | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: this should be
videos
/playlistItems
findIndexOfCurrentVideoInPlaylist
is called with various source of video lists (with different sorting orders applied)