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

Add back author search in video playlist searching #4919

Conversation

kommunarr
Copy link
Collaborator

@kommunarr kommunarr commented Apr 10, 2024

Add back author search in video playlist searching

Pull Request Type

  • Feature Implementation

Related issue

N/A (marked as bug in #4857)

Description

Reuses previous paradigm (the paradigm currently in place in History.js). This was the pre-existing behavior before the Playlist PR. See

const filteredQuery = this.favoritesPlaylist.videos.filter((video) => {
if (typeof (video.title) !== 'string' || typeof (video.author) !== 'string') {
return false
} else {
return video.title.toLowerCase().includes(lowerCaseQuery) || video.author.toLowerCase().includes(lowerCaseQuery)
}
.

Testing

  • Search for video title (variegate casing, should be insensitive)
  • Search for author (variegate casing, should be insensitive)

Desktop

  • OS: OpenSUSE
  • OS Version: TW
  • FreeTube version: Why do we even ask this one for PRs?

Reuses previous paradigm (the paradigm currently in place in History.js). This was the pre-existing behavior before the Playlist PR. See https://github.com/FreeTubeApp/FreeTube/blob/5c8d49bf51dde8ffd8c8d291dce3e5ff27b0d9fb/src/renderer/views/UserPlaylists/UserPlaylists.js#L98-L103.
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 10, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) April 10, 2024 11:38
Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

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

Instead of bailing out when either of them are missing it would be better to check them individually e.g. with this pseudo-code:

if has title and match in title
    return true

if has author and match in author
    return true

return false

Copy link
Member

Choose a reason for hiding this comment

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

Probably going to suggest something controversial here.

I think i like to see a Filter on the main playlist page that enables Search by Author so i instantly know in what playlists the author is in in and if i click on that playlist that search shows me the same query as the one i just on the main playlist page. Just like how it works with the other filter

VirtualBoxVM_wBPwlbaZFQ.mp4

@kommunarr
Copy link
Collaborator Author

It sounds interesting, but I don't want to add it as a feature unless a user asks for it. I don't have a use case for that personally.

@PikachuEXE
Copy link
Collaborator

Users should be able to search for author in user playlists view too when Playlists with matching videos is on
Otherwise it's inconsistent since they can search for video title there now

Places to change: search if (this.doSearchPlaylistsWithMatchingVideos) {

@kommunarr
Copy link
Collaborator Author

My bad, I misread the suggestion as adding a new filter toggle for video author search specifically. Added now.

@efb4f5ff-1298-471a-8973-3d47447115dc

My bad, I misread the suggestion as adding a new filter toggle for video author search specifically. Added now.

Well i actually did mean that but Pika suggestions is way better

@FreeTubeBot FreeTubeBot merged commit d6e5439 into FreeTubeApp:development Apr 11, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants