FreeTube icon indicating copy to clipboard operation
FreeTube copied to clipboard

Add search playlists with matching videos function

Open PikachuEXE opened this issue 1 year ago • 3 comments

Pull Request Type

  • [ ] Bugfix
  • [x] Feature Implementation
  • [ ] Documentation
  • [ ] Other

Related issue

Improvement to https://github.com/FreeTubeApp/FreeTube/pull/4234

Description

  • Update user playlists page to add search playlists with matching videos function
  • Update add videos to playlists prompt to add search playlists with matching videos function

Screenshots

image image

Testing

  • Add video(s) not matching playlist name
  • Search with partial video title, turn on/off toggle to see search results

Desktop

  • OS:
  • OS Version:
  • FreeTube version:

Additional context

PikachuEXE avatar Jan 09 '24 02:01 PikachuEXE

If there is no web build nesting can be used now (I tested locally before submitting PR, just not the web build) https://caniuse.com/css-nesting

Anyway updated

PikachuEXE avatar Jan 09 '24 05:01 PikachuEXE

Also won't work if we ever switch to Tauri, but we'll probably have to get rid of loads of other modern CSS and JavaScript stuff then anyway. Tauri 1 supports macOS 10.13+ (High Sierra), which was released in 2017.

However we could always set the minimum version higher ourselves.

absidue avatar Jan 09 '24 05:01 absidue

Also won't work if we ever switch to Tauri, but we'll probably have to get rid of loads of other modern CSS and JavaScript stuff then anyway. Tauri 1 supports macOS 10.13+ (High Sierra), which was released in 2017.

If we need to support older versions then we could always use browserslist with postcss and babel configurations https://github.com/browserslist/browserslist?tab=readme-ov-file#browserslist-

ChunkyProgrammer avatar Jan 12 '24 17:01 ChunkyProgrammer

Well this only shows the playlist that the video is included in, not the video(s) itself.

toby63 avatar Jan 27 '24 21:01 toby63

Not a huge fan of the placement. I understand why you picked that placement for it but i looks weird to me.

What about aligning it with the dropdown, something like

Capture

Updated UI & label text, take a look I think aligning the toggle with the sort is weird (feel detached from the search input text box for no good reason) image

PikachuEXE avatar Jan 30 '24 07:01 PikachuEXE

Trying to understand the use case. Someone wants to find videos they saved with a specific keyword (which is what this search bar did in previous versions of FreeTube, and what most people will assume this does when loading up the updated app). They put the keyword into the search bar and are presented with the playlists that contain a specific video. They then have to scroll a lot to find that section of their playlist. It doesn't sound like a pleasant UX. I would not be personally familiar with any other use cases.

kommunarr avatar Jan 30 '24 13:01 kommunarr

@PikachuEXE hmm i understand what you're saying. Not sure whats the best approach here

@jasonhenriquez suggestion: Maybe make the searchbar function as before without the toggle? So remove toggle and users can use the searchbar to 1) playlist name, 2) video title. If we go that way maybe we should also add a search in the playlist it self so the user can look up the vid they're trying to find

@efb4f5ff-1298-471a-8973-3d47447115dc I'm not sure if that addresses my concern. Or rather, not a concern, but just a question of what problem is being solved by this feature being present on the User Playlists page. At least in my usage, I haven't encountered the use case of needing to find which playlist has which video based on a keyword. If I did, what would I do from there? Click to open the playlist and then do another search to find the video I just searched?

Instead, I often encounter the problem of needing to find the video link(s) / entry(s) that exists in one of my playlists based on the keyword. The location of this search bar is where we've trained current users of FreeTube to find the feature for that second use case. I think this space would be better reserved for a playlist-agnostic video search bar. This would work in that the video search results would load in lieu of or below the playlists, very similar to what we had there prior. There's then the question of what to do when one video appears in multiple playlists or has duplicate entries (my thought would be to just show it multiple times, optionally with information on its owning playlist and/or playlist index), but that's thankfully a rarer and far less irksome problem to encounter than the ones this solution resolves. This minimize the jarringness of the existing feature being replaced with something that looks similar but does something very different and is more comparatively niche in utility.

kommunarr avatar Jan 30 '24 18:01 kommunarr

  • Make it search for video info always would return to many results, not good
  • I understand the search for video in playlist UX requires improvement which depends on feature for searching video inside single playlist view which cannot be pushed yet (implemented but would conflict w/ https://github.com/FreeTubeApp/FreeTube/pull/4597)

Actions for me:

  • Wait for https://github.com/FreeTubeApp/FreeTube/pull/4597
  • Rework feature for searching video inside single playlist view
  • Rework this PR so that UX for searching videos inside playlists is improved

PikachuEXE avatar Jan 31 '24 00:01 PikachuEXE

  • Make it search for video info always would return to many results, not good

    • I understand the search for video in playlist UX requires improvement which depends on feature for searching video inside single playlist view which cannot be pushed yet (implemented but would conflict w/ Playlist performance improvements #4597)

Sry if it is seen as inappropriate to comment, but I think:

  1. Users can/could search for videos in the old unified playlist, so why should this feature disappear now?
  2. The main reason people would like to search is probably for videos, but still it can be useful to search for playlists too, so I would of course keep the toogle (playlist/videos)
  3. One additional idea would be, to add some note to the search results from or in which playlist(s) the videos are. e.g. Video Thumbnail Video title included in Playlist X & Y

toby63 avatar Jan 31 '24 20:01 toby63

Now https://github.com/FreeTubeApp/FreeTube/pull/4622 is merged And also updated this PR so that clicking on playlist link with Playlists with Matching Videos enabled would search for same query text when view switched

e.g. image image

PikachuEXE avatar Mar 08 '24 13:03 PikachuEXE

Thats a really cool implementation!

Found a bug though :)

Search in playlist shouldn't be active with this feature enabled

https://github.com/FreeTubeApp/FreeTube/assets/73130443/15a9b414-256b-4bcb-a28a-232b5a7dd1d1

Edit: idk why but the height of the toggle makes me a bit uncomfortable. What about aligning it with the height of the dropdown ?

Now #4622 is merged And also updated this PR so that clicking on playlist link with Playlists with Matching Videos enabled would search for same query text when view switched

Sry for bothering again, but I think a lot of users will agree with me on this one, so I once again would like to voice my oppinion.

I still disagree with this type of implementation, it is a step backwards from how things were done before. With the unified playlist you could simply search for videos directly.

With this implementation IIUC a user searching for a video would need two steps instead of one step to get to the video/s he wants.

May I ask, is this only an intermediate solution? Are there technical reasons for this or is this simply a design decision?

If it's solely a design decision then I don't understand it.

I think it would be a much better solution to be able to search for videos directly, just like before. Additionally you could add some kind of indicator in which Playlist the Video is (for sake of easiness: If the video is in multiple playlists, only the first playlist the video was added to could be shown).

toby63 avatar Mar 08 '24 19:03 toby63

Search in playlist shouldn't be active with this feature enabled

I guess you mean it shouldn't be active inside empty playlists Let me know if wrong

idk why but the height of the toggle makes me a bit uncomfortable. What about aligning it with the height of the dropdown

Now better? image

PikachuEXE avatar Mar 09 '24 02:03 PikachuEXE

I guess you mean it shouldn't be active inside empty playlists Let me know if wrong

You are correct. LGTM

Now better? image

LGTM

@toby63 We cant just show playlists and videos along side each other that will create a big mess. The playlist default search is searching only for playlist titles BUT if the user wants to find a video the just hit that toggle and search for the video title and it will show you results of the playlists that matches the search query for the video you are looking for.

How this is currently implemented is a step into the right direction IMO

@toby63 We cant just show playlists and videos along side each other that will create a big mess. The playlist default search is searching only for playlist titles BUT if the user wants to find a video the just hit that toggle and search for the video title and it will show you results of the playlists that matches the search query for the video you are looking for.

How this is currently implemented is a step into the right direction IMO

I disagree.

  1. It doesn't have to be a mess. One way would be: You would have a toogle "Search for Videos" and then you only show video results. I bet that this is the thing 99% of the users want. It will not matter to most users in what playlist the video is that they are searching for.
  2. Even if you implement my proposal to also show playlists, it won't be a mess IMHO. It will only be a UI thing:

Example for how one specific result would like:

###############################
# Upper part:
#  Shows Video title and maybe thumnail etc. If you click on it, it will open the video
#
######################
# Lower Part (clearly seperated):
# Shows Playlist title. If you click on it, it will load the playlist (at best at the position that the video is on).
#
####################

toby63 avatar Mar 09 '24 21:03 toby63

I bet that this is the thing 99% of the users want. It will not matter to most users in what playlist the video is that they are searching for.

This isnt a strong argument and one you cant make. You cant voice your opinion and make that a reflection of how others will or will not use FT.

############################### Upper part: Shows Video title and maybe thumnail etc. If you click on it, it will open the video

That would be nice yes but im pretty sure that isnt something we can do. First of all Videos are now properly part of a Playlist. That means that a Playlist contains X amount of Videos. Doing something like you suggested would mean we have to somehow decouple the Video that is stored inside the Playlist when you want to watch the video.

I dont see anything like this happening and i stand by my point that this implementation is in the right direction.

Would it be possible to also include the channel name in the search

like if the search text is in the video title and/or youtube channel.

ArthurKun21 avatar Mar 10 '24 11:03 ArthurKun21

@ArthurKun21 I think that suits the search within a specific Playlist better than the global Playlist search but i like to hear from the other devs in the team about this. If we decide that it suits the search within a Playlist better than that is out of scope of this PR.

Edit: If we do this then we should do it for both searches otherwise it would cause confusion

I bet that this is the thing 99% of the users want. It will not matter to most users in what playlist the video is that they are searching for.

This isnt a strong argument and one you cant make. You cant voice your opinion and make that a reflection of how others will or will not use FT.

It will still turn out to be true, believe me. If I were you, I would at least warn users about such a change and maybe even offer the old "way" with a unified playlist for those who like that a lot (in newer versions).

############################### Upper part: Shows Video title and maybe thumnail etc. If you click on it, it will open the video

That would be nice yes but im pretty sure that isnt something we can do. First of all Videos are now properly part of a Playlist. That means that a Playlist contains X amount of Videos. Doing something like you suggested would mean we have to somehow decouple the Video that is stored inside the Playlist when you want to watch the video.

I dont see anything like this happening and i stand by my point that this implementation is in the right direction.

Well it would probably be possible with https://github.com/PikachuEXE/FreeTube/issues/66, but I acknowledge that it's difficult (as outlined in that FR). Still I don't quite understand, here you also search for the videos (because otherwise you would not know where they are), so how is it done and why can't the videos then be shown directly? I mean, a title and a link to the video would be enough, no thumbnails would need to be loaded IMHO etc.

And what about my "side-line" proposal, that if you click on the playlist FreeTube could at least "jump" directly to the video position in the playlist? It would still be two clicks, but not a double-search. Or is this already the plan?

toby63 avatar Mar 10 '24 14:03 toby63

I would at least warn users about such a change

Ofcourse we will inform current users about the change

And what about my "side-line" proposal, that if you click on the playlist FreeTube could at least "jump" directly to the video position in the playlist? It would still be two clicks, but not a double-search. Or is this already the plan?

@toby63 This is how it behaves now

https://github.com/FreeTubeApp/FreeTube/assets/73130443/f67a0245-1a22-409a-813e-ad43779bc3ba

I would at least warn users about such a change

Ofcourse we will inform current users about the change

And what about my "side-line" proposal, that if you click on the playlist FreeTube could at least "jump" directly to the video position in the playlist? It would still be two clicks, but not a double-search. Or is this already the plan?

@toby63 This is how it behaves now VirtualBoxVM_ZeWk10pGUd.mp4

Thx for the illustration, I see you already had this idea :wink:. Well thats not perfect, but it's ok.

I will then wait until https://github.com/PikachuEXE/FreeTube/issues/66 might be functional and then we might get full video search once again :crossed_fingers:.

Just out of curiosity, does this work with mutiple search results in one playlist too?

toby63 avatar Mar 10 '24 19:03 toby63

What do you mean by multiple searches? Multiple video results showing inside the playlist based on the search?

What do you mean by multiple searches? Multiple video results showing inside the playlist based on the search?

Yes.

toby63 avatar Mar 10 '24 20:03 toby63

Yes it does that

@efb4f5ff-1298-471a-8973-3d47447115dc Thx and also thx for the patience :slightly_smiling_face:.

toby63 avatar Mar 11 '24 13:03 toby63

@efb4f5ff-1298-471a-8973-3d47447115dc I forgot to make the toggle in add to playlist prompt to be vertically centered (now done) Please recheck image

PikachuEXE avatar Mar 13 '24 00:03 PikachuEXE