Skip to content
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

Make adding duplicate disabled by default #5044

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,29 @@

.optionsRow {
display: grid;
grid-template-columns: repeat(2, 1fr);
grid-template-rows: 1fr;
grid-template-columns: repeat(auto-fit, minmax(100px, 1fr));
align-items: center;
}

.tightOptions {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(100px, max-content));
column-gap: 16px;
align-items: center;
}

@media only screen and (width <= 800px) {
.optionsRow {
/* Switch to 2 rows from 2 columns */
/* Switch to rows from columns */
grid-template-columns: auto;
grid-template-rows: auto auto;
grid-template-rows: repeat(auto-fit, auto);
align-items: stretch;
}

.tightOptions {
/* Switch to rows from columns */
grid-template-columns: auto;
grid-template-rows: repeat(auto-fit, auto);
align-items: stretch;
}
}
Expand All @@ -51,7 +64,11 @@
overflow-y: scroll;
}

.playlist-selector-container {
.playlist-selector-container:not(.disabled) {
/* Make them look selectable */
cursor: pointer;
}

.playlist-selector-container.disabled {
opacity: 0.5;
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export default defineComponent({
updateQueryDebounce: function() {},
lastShownAt: Date.now(),
sortBy: SORT_BY_VALUES.LatestUpdatedFirst,
addingDuplicateVideosEnabled: false,
}
},
computed: {
Expand Down Expand Up @@ -111,6 +112,9 @@ export default defineComponent({
toBeAddedToPlaylistVideoList: function () {
return this.$store.getters.getToBeAddedToPlaylistVideoList
},
toBeAddedToPlaylistVideoIdList: function () {
return this.toBeAddedToPlaylistVideoList.map((v) => v.videoId)
},
newPlaylistDefaultProperties: function () {
return this.$store.getters.getNewPlaylistDefaultProperties
},
Expand Down Expand Up @@ -161,6 +165,23 @@ export default defineComponent({
sortBySelectValues() {
return Object.values(SORT_BY_VALUES)
},

playlistIdsContainingVideosToBeAdded() {
const ids = []

this.allPlaylists.forEach((playlist) => {
const playlistVideoIdSet = playlist.videos.reduce((s, v) => s.add(v.videoId), new Set())

if (this.toBeAddedToPlaylistVideoIdList.every((vid) => playlistVideoIdSet.has(vid))) {
ids.push(playlist._id)
}
})

return ids
},
anyPlaylistContainsVideosToBeAdded() {
return this.playlistIdsContainingVideosToBeAdded.length > 0
},
},
watch: {
allPlaylistsLength(val, oldVal) {
Expand Down Expand Up @@ -202,6 +223,16 @@ export default defineComponent({
// due to enter key press in CreatePlaylistPrompt
nextTick(() => this.$refs.searchBar.focus())
},

addingDuplicateVideosEnabled(val) {
if (val) { return }

// Only care when addingDuplicateVideosEnabled disabled
// Remove disabled playlists
this.selectedPlaylistIdList = this.selectedPlaylistIdList.filter(playlistId => {
return !this.playlistIdsContainingVideosToBeAdded.includes(playlistId)
})
},
},
mounted: function () {
this.updateQueryDebounce = debounce(this.updateQuery, 500)
Expand Down Expand Up @@ -238,10 +269,16 @@ export default defineComponent({
const playlist = this.allPlaylists.find((list) => list._id === selectedPlaylistId)
if (playlist == null) { return }

// Use [].concat to avoid `do not mutate vuex store state outside mutation handlers`
let videosToBeAdded = [].concat(this.toBeAddedToPlaylistVideoList)
if (!this.addingDuplicateVideosEnabled) {
const playlistVideoIds = playlist.videos.map((v) => v.videoId)
videosToBeAdded = videosToBeAdded.filter((v) => !playlistVideoIds.includes(v.videoId))
PikachuEXE marked this conversation as resolved.
Show resolved Hide resolved
}

this.addVideos({
_id: playlist._id,
// Use [].concat to avoid `do not mutate vuex store state outside mutation handlers`
videos: [].concat(this.toBeAddedToPlaylistVideoList),
videos: videosToBeAdded,
})
addedPlaylistIds.add(playlist._id)
// Update playlist's `lastUpdatedAt`
Expand Down Expand Up @@ -281,6 +318,12 @@ export default defineComponent({

getIconForSortPreference: (s) => getIconForSortPreference(s),

playlistDisabled(playlistId) {
if (this.addingDuplicateVideosEnabled) { return false }

return this.playlistIdsContainingVideosToBeAdded.includes(playlistId)
},

...mapActions([
'addVideos',
'updatePlaylist',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,25 @@
<div
class="optionsRow"
>
<ft-toggle-switch
:label="$t('User Playlists.Playlists with Matching Videos')"
:compact="true"
:default-value="doSearchPlaylistsWithMatchingVideos"
@change="doSearchPlaylistsWithMatchingVideos = !doSearchPlaylistsWithMatchingVideos"
/>
<div
class="tightOptions"
>
<ft-toggle-switch
class="matchingVideoToggle"
:label="$t('User Playlists.Playlists with Matching Videos')"
:compact="true"
:default-value="doSearchPlaylistsWithMatchingVideos"
@change="doSearchPlaylistsWithMatchingVideos = !doSearchPlaylistsWithMatchingVideos"
/>
<ft-toggle-switch
v-if="anyPlaylistContainsVideosToBeAdded"
class="allowDuplicateToggle"
:label="$t('User Playlists.AddVideoPrompt.Allow Adding Duplicate Video(s)')"
:compact="true"
:default-value="addingDuplicateVideosEnabled"
@change="addingDuplicateVideosEnabled = !addingDuplicateVideosEnabled"
/>
</div>
<ft-select
v-if="allPlaylists.length > 1"
class="sortSelect"
Expand All @@ -48,14 +61,20 @@
<ft-flex-box>
<div
v-for="(playlist, index) in activePlaylists"
:key="playlist._id"
:key="`${playlist._id}-${playlistDisabled(playlist._id)}`"
class="playlist-selector-container"
:class="{
disabled: playlistDisabled(playlist._id),
}"
PikachuEXE marked this conversation as resolved.
Show resolved Hide resolved
:aria-disabled="playlistDisabled(playlist._id)"
>
<ft-playlist-selector
tabindex="0"
:tabindex="playlistDisabled(playlist._id) ? -1 : 0"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought (future): I would also like to refactor this to use input type="checkbox" someday, and similar for other form elements, because us imitating accessible functionality with custom controls ends up being error-prone and more work (this is pre-existing to this PR, just seems especially apparent with what we have to do to imitate the powerful disabled attribute)

:playlist="playlist"
:index="index"
:selected="selectedPlaylistIdList.includes(playlist._id)"
:disabled="playlistDisabled(playlist._id)"
:adding-duplicate-videos-enabled="addingDuplicateVideosEnabled"
@selected="countSelected(playlist._id)"
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ export default defineComponent({
type: Boolean,
required: true,
},
disabled: {
type: Boolean,
required: true,
},
addingDuplicateVideosEnabled: {
type: Boolean,
required: true,
},
},
emits: ['selected'],
data: function () {
Expand Down Expand Up @@ -76,10 +84,40 @@ export default defineComponent({
count: this.loneVideoPresenceCountInPlaylist,
})
},
multiVideoPresenceCountInPlaylist() {
if (this.toBeAddedToPlaylistVideoList.length < 2) { return null }

// Count of to be added videos already present in this playlist
const v = this.toBeAddedToPlaylistVideoList.reduce((accumulator, toBeAddedToVideo) => {
return this.playlist.videos.some((pv) => pv.videoId === toBeAddedToVideo.videoId)
? accumulator + 1
: accumulator
}, 0)
// Don't display zero value
return v === 0 ? null : v
},
multiVideoPresenceCountInPlaylistText() {
if (this.multiVideoPresenceCountInPlaylist == null) { return null }

if (this.addingDuplicateVideosEnabled || this.toBeAddedToPlaylistVideoList.length === this.multiVideoPresenceCountInPlaylist) {
return this.$t('User Playlists.AddVideoPrompt.{videoCount}/{totalVideoCount} Videos Already Added', {
videoCount: this.multiVideoPresenceCountInPlaylist,
totalVideoCount: this.toBeAddedToPlaylistVideoList.length,
})
}

return this.$t('User Playlists.AddVideoPrompt.{videoCount}/{totalVideoCount} Videos Will Be Added', {
videoCount: this.toBeAddedToPlaylistVideoList.length - this.multiVideoPresenceCountInPlaylist,
totalVideoCount: this.toBeAddedToPlaylistVideoList.length,
})
},
videoPresenceCountInPlaylistText() {
return this.loneVideoPresenceCountInPlaylistText ?? this.multiVideoPresenceCountInPlaylistText
},
videoPresenceCountInPlaylistTextVisible() {
if (!this.videoPresenceCountInPlaylistTextShouldBeVisible) { return false }

return this.loneVideoPresenceCountInPlaylistText != null
return this.videoPresenceCountInPlaylistText != null
},
},
created: function () {
Expand All @@ -100,6 +138,8 @@ export default defineComponent({
},

toggleSelection: function () {
if (this.disabled) { return }

this.$emit('selected', this.index)
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
v-if="videoPresenceCountInPlaylistTextVisible"
class="videoPresenceCount"
>
{{ loneVideoPresenceCountInPlaylistText }}
{{ videoPresenceCountInPlaylistText }}
</div>
</div>
</div>
Expand Down
5 changes: 4 additions & 1 deletion static/locales/en-US.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,12 @@ User Playlists:
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'
Search in Playlists: Search in Playlists
Allow Adding Duplicate Video(s): Allow Adding Duplicate Video(s)
Save: Save

Added {count} Times: 'Added {count} Time | Added {count} Times'
Added {count} Times: 'Already Added | Added {count} Times'
"{videoCount}/{totalVideoCount} Videos Will Be Added": '{videoCount}/{totalVideoCount} Videos Will Be Added'
"{videoCount}/{totalVideoCount} Videos Already Added": '{videoCount}/{totalVideoCount} Videos Already Added'

Toast:
You haven't selected any playlist yet.: You haven't selected any playlist yet.
Expand Down
2 changes: 1 addition & 1 deletion static/locales/en_GB.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ User Playlists:
playlists
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
Added {count} Times: Added {count} time | Added {count} times
Added {count} Times: Already Added | Added {count} times
CreatePlaylistPrompt:
New Playlist Name: New Playlist name
Create: Create
Expand Down