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.

Sign up for GitHub

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

Jump to bottom

Improve create & edit playlist UX #5226

Merged
merged 9 commits into from
Jun 13, 2024
Merged

Improve create & edit playlist UX #5226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
380e958
* Make it impossible to create playlists with spaces wrapped name & s…
PikachuEXE Jun 4, 2024
928544d
* Update default new playlist name to include channel name when copyi…
PikachuEXE Jun 4, 2024
22cd16e
* Show duplicate playlist message sooner for blank playlist name
PikachuEXE Jun 4, 2024
cc62f88
Update per code review
PikachuEXE Jun 5, 2024
aa774c5
* Only show blank playlist name message when spaces input
PikachuEXE Jun 8, 2024
7472a95
! Fix create button not disabled when input name blank (not empty)
PikachuEXE Jun 11, 2024
81b1a37
!* Fix message shown in edit mode, update edit mode to show error mes…
PikachuEXE Jun 11, 2024
8b95010
$ Fix indentation warning from lint
PikachuEXE Jun 11, 2024
429f4e9
! Fix able to create playlist with invalid playlist names
PikachuEXE Jun 12, 2024
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
39 changes: 29 additions & 10 deletions src/renderer/components/ft-create-playlist-prompt/ft-create-playlist-prompt.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,26 +33,45 @@ export default defineComponent({
newPlaylistVideoObject: function () {
return this.$store.getters.getNewPlaylistVideoObject
},

playlistNameEmpty() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

note: once the Vue 3 migration is complete, this will be a great place for composable functions.

return this.playlistName === ''
},
playlistNameBlank() {
return !this.playlistNameEmpty && this.playlistName.trim() === ''
},
playlistWithNameExists() {
// Don't show the message with no name input
const playlistName = this.playlistName
if (this.playlistName === '') { return false }

return this.allPlaylists.some((playlist) => {
return playlist.playlistName === playlistName
})
},
playlistPersistenceDisabled() {
return this.playlistNameEmpty || this.playlistNameBlank || this.playlistWithNameExists
},
},
mounted: function () {
this.playlistName = this.newPlaylistVideoObject.title
// Faster to input required playlist name
nextTick(() => this.$refs.playlistNameInput.focus())
},
methods: {
createNewPlaylist: function () {
if (this.playlistName === '') {
showToast(this.$t('User Playlists.SinglePlaylistView.Toast["Playlist name cannot be empty. Please input a name."]'))
handlePlaylistNameInput(input) {
if (input.trim() === '') {
// Need to show message for blank input
this.playlistName = input
return
}

const nameExists = this.allPlaylists.findIndex((playlist) => {
return playlist.playlistName === this.playlistName
})
if (nameExists !== -1) {
showToast(this.$t('User Playlists.CreatePlaylistPrompt.Toast["There is already a playlist with this name. Please pick a different name."]'))
return
}
this.playlistName = input.trim()
},

createNewPlaylist: function () {
// Still possible to attempt to create via pressing enter
if (this.playlistPersistenceDisabled) { return }

const playlistObject = {
playlistName: this.playlistName,
Expand Down
13 changes: 12 additions & 1 deletion src/renderer/components/ft-create-playlist-prompt/ft-create-playlist-prompt.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,24 @@
:value="playlistName"
:maxlength="255"
class="playlistNameInput"
@input="(input) => playlistName = input"
@input="handlePlaylistNameInput"
@click="createNewPlaylist"
/>
</ft-flex-box>
<ft-flex-box v-if="playlistNameBlank">
<p>
{{ $t('User Playlists.SinglePlaylistView.Toast["Playlist name cannot be empty. Please input a name."]') }}
</p>
</ft-flex-box>
<ft-flex-box v-if="playlistWithNameExists">
<p>
{{ $t('User Playlists.CreatePlaylistPrompt.Toast["There is already a playlist with this name. Please pick a different name."]') }}
</p>
</ft-flex-box>
<ft-flex-box>
<ft-button
:label="$t('User Playlists.CreatePlaylistPrompt.Create')"
:disabled="playlistPersistenceDisabled"
Copy link
Collaborator

Choose a reason for hiding this comment

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

thought/question: I have no idea how Vue knows to make the underlying button disabled when there doesn't seem to be a prop for this in our ft-button class, but it does.

Copy link
Member

Choose a reason for hiding this comment

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

Vue calls that feature "fallthrough attributes", if the component doesn't have a prop for the attribute, it adds the attribute to the root/top-level HTML element inside that component.

Vue 3 has an entire docs page on it, but it also describes Vue 3 specific behaviour so it's not that useful for us yet, Vue 2's docs only have an offhand mention of it (https://v2.vuejs.org/v2/api/#inheritAttrs).

@click="createNewPlaylist"
/>
<ft-button
Expand Down
47 changes: 42 additions & 5 deletions src/renderer/components/playlist-info/playlist-info.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ export default defineComponent({
return this.$store.getters.getPlaylist(this.id)
},

allPlaylists: function () {
return this.$store.getters.getAllPlaylists
},

deletePlaylistPromptNames: function () {
return [
this.$t('Yes, Delete'),
Expand Down Expand Up @@ -241,6 +245,29 @@ export default defineComponent({
playlistDeletionDisabledLabel: function () {
return this.$t('User Playlists["Cannot delete the quick bookmark target playlist."]')
},

inputPlaylistNameEmpty() {
return this.newTitle === ''
},
inputPlaylistNameBlank() {
return !this.inputPlaylistNameEmpty && this.newTitle.trim() === ''
},
inputPlaylistWithNameExists() {
// Don't show the message with no name input
const playlistName = this.newTitle
const selectedUserPlaylist = this.selectedUserPlaylist
if (this.newTitle === '') { return false }

return this.allPlaylists.some((playlist) => {
// Only compare with other playlists
if (selectedUserPlaylist._id === playlist._id) { return false }

return playlist.playlistName === playlistName
})
},
playlistPersistenceDisabled() {
return this.inputPlaylistNameEmpty || this.inputPlaylistNameBlank || this.inputPlaylistWithNameExists
},
},
watch: {
showDeletePlaylistPrompt(shown) {
Expand Down Expand Up @@ -269,6 +296,16 @@ export default defineComponent({
document.removeEventListener('keydown', this.keyboardShortcutHandler)
},
methods: {
handlePlaylistNameInput(input) {
if (input.trim() === '') {
// Need to show message for blank input
this.newTitle = input
return
}

this.newTitle = input.trim()
},

toggleCopyVideosPrompt: function (force = false) {
if (this.moreVideoDataAvailable && !this.isUserPlaylist && !force) {
showToast(this.$t('User Playlists.SinglePlaylistView.Toast["Some videos in the playlist are not loaded yet. Click here to copy anyway."]'), 5000, () => {
Expand All @@ -279,15 +316,15 @@ export default defineComponent({

this.showAddToPlaylistPromptForManyVideos({
videos: this.videos,
newPlaylistDefaultProperties: { title: this.title },
newPlaylistDefaultProperties: {
title: this.channelName === '' ? this.title : `${this.title} | ${this.channelName}`,
},
})
},

savePlaylistInfo: function () {
if (this.newTitle === '') {
showToast(this.$t('User Playlists.SinglePlaylistView.Toast["Playlist name cannot be empty. Please input a name."]'))
return
}
// Still possible to attempt to create via pressing enter
if (this.playlistPersistenceDisabled) { return }

const playlist = {
playlistName: this.newTitle,
Expand Down
69 changes: 43 additions & 26 deletions src/renderer/components/playlist-info/playlist-info.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,35 +34,51 @@
</div>

<div class="playlistStats">
<ft-input
<template
v-if="editMode"
ref="playlistTitleInput"
class="inputElement"
:placeholder="$t('User Playlists.Playlist Name')"
:show-action-button="false"
:show-label="false"
:value="newTitle"
:maxlength="255"
@input="(input) => (newTitle = input)"
@keydown.enter.native="savePlaylistInfo"
/>
<h2
>
<ft-input
ref="playlistTitleInput"
class="inputElement"
:placeholder="$t('User Playlists.Playlist Name')"
:show-action-button="false"
:show-label="false"
:value="newTitle"
:maxlength="255"
@input="handlePlaylistNameInput"
@keydown.enter.native="savePlaylistInfo"
/>
<ft-flex-box v-if="inputPlaylistNameEmpty || inputPlaylistNameBlank">
<p>
{{ $t('User Playlists.SinglePlaylistView.Toast["Playlist name cannot be empty. Please input a name."]') }}
</p>
</ft-flex-box>
<ft-flex-box v-if="inputPlaylistWithNameExists">
<p>
{{ $t('User Playlists.CreatePlaylistPrompt.Toast["There is already a playlist with this name. Please pick a different name."]') }}
</p>
</ft-flex-box>
</template>
<template
v-else
class="playlistTitle"
>
{{ title }}
</h2>
<p>
{{ $tc('Global.Counts.Video Count', videoCount, {count: parsedVideoCount}) }}
<span v-if="!hideViews && !isUserPlaylist">
- {{ $tc('Global.Counts.View Count', viewCount, {count: parsedViewCount}) }}
</span>
<span>- </span>
<span v-if="infoSource !== 'local'">
{{ $t("Playlist.Last Updated On") }}
</span>
{{ lastUpdated }}
</p>
<h2
class="playlistTitle"
>
{{ title }}
</h2>
<p>
{{ $tc('Global.Counts.Video Count', videoCount, {count: parsedVideoCount}) }}
<span v-if="!hideViews && !isUserPlaylist">
- {{ $tc('Global.Counts.View Count', viewCount, {count: parsedViewCount}) }}
</span>
<span>- </span>
<span v-if="infoSource !== 'local'">
{{ $t("Playlist.Last Updated On") }}
</span>
{{ lastUpdated }}
</p>
</template>
</div>

<ft-input
Expand Down Expand Up @@ -118,6 +134,7 @@
<ft-icon-button
v-if="editMode"
:title="$t('User Playlists.Save Changes')"
:disabled="playlistPersistenceDisabled"
:icon="['fas', 'save']"
theme="secondary"
@click="savePlaylistInfo"
Expand Down