Add search playlists with matching videos function #4537

PikachuEXE · 2024-01-09T02:04:38Z

Pull Request Type

Bugfix
Feature Implementation
Documentation
Other

Related issue

Improvement to #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

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

src/renderer/components/ft-playlist-add-video-prompt/ft-playlist-add-video-prompt.css

src/renderer/views/UserPlaylists/UserPlaylists.css

PikachuEXE · 2024-01-09T05:38:32Z

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

absidue · 2024-01-09T05:48:24Z

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.

ChunkyProgrammer · 2024-01-12T17:17:17Z

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-

toby63 · 2024-01-27T21:51:46Z

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

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-01-30T04:06:36Z

static/locales/en-US.yaml

@@ -141,6 +141,7 @@ User Playlists:
  You have no playlists. Click on the create new playlist button to create a new one.: You have no playlists. Click on the create new playlist button to create a new one.
  Empty Search Message: There are no videos in this playlist that matches your search
  Search bar placeholder: Search in Playlist
+  Search in Videos: Search in Videos


This could be more specific imo

Suggested change

Search in Videos: Search in Videos

Search in Videos: Search for Videos in Playlist

This is not searching for videos, but searching for playlist(s) with matching videos
No idea how to make it short

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-01-30T04:06:51Z

static/locales/en-US.yaml

@@ -141,6 +141,7 @@ User Playlists:
  You have no playlists. Click on the create new playlist button to create a new one.: You have no playlists. Click on the create new playlist button to create a new one.
  Empty Search Message: There are no videos in this playlist that matches your search
  Search bar placeholder: Search in Playlist


Suggested change

Search bar placeholder: Search in Playlist

Search bar placeholder: Search for Playlist

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-01-30T04:14:06Z

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

PikachuEXE · 2024-01-30T07:11:34Z

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)

kommunarr · 2024-01-30T13:56:00Z

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.

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-01-30T17:30:15Z

@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

kommunarr · 2024-01-30T18:15:13Z

@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.

PikachuEXE · 2024-01-31T00:45:47Z

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)

Actions for me:

Wait for Playlist performance improvements #4597
Rework feature for searching video inside single playlist view
Rework this PR so that UX for searching videos inside playlists is improved

toby63 · 2024-01-31T20:06:55Z

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:

Users can/could search for videos in the old unified playlist, so why should this feature disappear now?
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)
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

…deos function

…matching videos function

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-03-09T15:04:46Z

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

You are correct. LGTM

Now better?

LGTM

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-03-09T17:09:15Z

@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

efb4f5ff-1298-471a-8973-3d47447115dc

LGTM

toby63 · 2024-03-09T21:45:01Z

@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.

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.
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).
#
####################

absidue · 2024-03-09T17:02:13Z

src/renderer/components/ft-playlist-add-video-prompt/ft-playlist-add-video-prompt.css

+.searchInputsRow {
+  display: grid;
+
+  /* 2 columns */
+  grid-template-columns: 1fr auto;
+  column-gap: 16px;
+}
+@media only screen and (max-width: 800px) {
+  .searchInputsRow {
+    /* Switch to 2 rows from 2 columns */
+    grid-template-columns: auto;
+    grid-template-rows: auto auto;
+  }
+}
+
+.optionsRow {
+  display: grid;
+  grid-template-columns: repeat(2, 1fr);
+  grid-template-rows: 1fr;
+}
+@media only screen and (max-width: 800px) {
+  .optionsRow {
+    /* Switch to 2 rows from 2 columns */
+    grid-template-columns: auto;
+    grid-template-rows: auto auto;
+  }
+}
+


As they both use the same media query and CSS rules, the two media query blocks can be merged into one.

Suggested change

.searchInputsRow {

display: grid;

/* 2 columns */

grid-template-columns: 1fr auto;

column-gap: 16px;

}

@media only screen and (max-width: 800px) {

.searchInputsRow {

/* Switch to 2 rows from 2 columns */

grid-template-columns: auto;

grid-template-rows: auto auto;

}

}

.optionsRow {

display: grid;

grid-template-columns: repeat(2, 1fr);

grid-template-rows: 1fr;

}

@media only screen and (max-width: 800px) {

.optionsRow {

/* Switch to 2 rows from 2 columns */

grid-template-columns: auto;

grid-template-rows: auto auto;

}

}

.searchInputsRow {

display: grid;

/* 2 columns */

grid-template-columns: 1fr auto;

column-gap: 16px;

}

.optionsRow {

display: grid;

grid-template-columns: repeat(2, 1fr);

grid-template-rows: 1fr;

}

@media only screen and (max-width: 800px) {

.searchInputsRow,

.optionsRow {

/* Switch to 2 rows from 2 columns */

grid-template-columns: auto;

grid-template-rows: auto auto;

}

}

No longer valid, they now have different properties

The code inside the media queries is the same for both.

No really
The diff shown in thread is outdated

I can't see align-items: stretch on the GitHub website or in the mobile app, are you sure that all of your local changes got pushed to GitHub?

Oh that's caused by missing change to make new toggle vertically centered
Just committed the missing change

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-03-10T10:36:59Z

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.

ArthurKun21 · 2024-03-10T11:56:16Z

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.

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-03-10T13:01:55Z

@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

toby63 · 2024-03-10T14:33:11Z

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 PikachuEXE#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?

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-03-10T17:46:17Z

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

toby63 · 2024-03-10T19:54:40Z

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 😉.
Well thats not perfect, but it's ok.

I will then wait until PikachuEXE#66 might be functional and then we might get full video search once again 🤞.

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

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-03-10T19:56:59Z

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

toby63 · 2024-03-10T20:09:35Z

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

Yes.

efb4f5ff-1298-471a-8973-3d47447115dc · 2024-03-11T06:27:49Z

Yes it does that

toby63 · 2024-03-11T13:41:36Z

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

PikachuEXE · 2024-03-13T00:22:59Z

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

…h-matching-videos * development: Fix handling of video published date in video lists (FreeTubeApp#4752) Translated using Weblate (Dutch) Translated using Weblate (Chinese (Simplified)) Bump lefthook from 1.6.4 to 1.6.5 (FreeTubeApp#4758) Bump marked from 12.0.0 to 12.0.1 (FreeTubeApp#4757) Bump electron from 29.1.0 to 29.1.1 (FreeTubeApp#4756) Translated using Weblate (Portuguese) Translated using Weblate (Portuguese (Brazil)) Improve touch controls for dash quality selector (FreeTubeApp#4750) Translated using Weblate (Serbian) Translated using Weblate (Spanish) Translated using Weblate (French) Translated using Weblate (German) Translated using Weblate (Estonian)

src/renderer/components/ft-playlist-add-video-prompt/ft-playlist-add-video-prompt.js

* development: Translated using Weblate (Czech) Translated using Weblate (Slovenian) Add search playlists with matching videos function (FreeTubeApp#4537) Translated using Weblate (Danish) Translated using Weblate (Danish) Translated using Weblate (Danish) # Conflicts: # src/renderer/views/UserPlaylists/UserPlaylists.js

PikachuEXE requested review from absidue, efb4f5ff-1298-471a-8973-3d47447115dc, ChunkyProgrammer and kommunarr January 9, 2024 02:04

FreeTubeBot enabled auto-merge (squash) January 9, 2024 02:04

github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jan 9, 2024

absidue reviewed Jan 9, 2024

View reviewed changes

src/renderer/components/ft-playlist-add-video-prompt/ft-playlist-add-video-prompt.css Outdated Show resolved Hide resolved

src/renderer/views/UserPlaylists/UserPlaylists.css Outdated Show resolved Hide resolved

PikachuEXE force-pushed the feature/playlist-search-playlists-with-matching-videos branch from 1674a34 to f370a07 Compare January 9, 2024 05:37

absidue previously approved these changes Jan 12, 2024

View reviewed changes

efb4f5ff-1298-471a-8973-3d47447115dc reviewed Jan 30, 2024

View reviewed changes

PikachuEXE marked this pull request as draft January 31, 2024 00:46

auto-merge was automatically disabled January 31, 2024 00:46
Pull request was converted to draft

github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Jan 31, 2024

PikachuEXE added the PR: WIP label Jan 31, 2024

efb4f5ff-1298-471a-8973-3d47447115dc mentioned this pull request Feb 2, 2024

[Feature Request]: Add search field to playlists PikachuEXE/FreeTube#64

Closed

3 tasks

PikachuEXE added 3 commits March 8, 2024 20:34

* Update user playlists page to add search playlists with matching vi…

ebcb584

…deos function

* Update add videos to playlists prompt to add search playlists with …

d920641

…matching videos function

* Update UI & label text

022d3f8

PikachuEXE requested a review from efb4f5ff-1298-471a-8973-3d47447115dc March 9, 2024 02:53

efb4f5ff-1298-471a-8973-3d47447115dc previously approved these changes Mar 9, 2024

View reviewed changes

absidue reviewed Mar 9, 2024

View reviewed changes

PikachuEXE requested a review from absidue March 10, 2024 11:54

* Make new toggle vertically align center

834adc8

PikachuEXE dismissed efb4f5ff-1298-471a-8973-3d47447115dc’s stale review via 834adc8 March 13, 2024 00:21

efb4f5ff-1298-471a-8973-3d47447115dc approved these changes Mar 13, 2024

View reviewed changes

absidue approved these changes Mar 13, 2024

View reviewed changes

ChunkyProgrammer reviewed Mar 13, 2024

View reviewed changes

src/renderer/components/ft-playlist-add-video-prompt/ft-playlist-add-video-prompt.js Show resolved Hide resolved

ChunkyProgrammer approved these changes Mar 14, 2024

View reviewed changes

FreeTubeBot merged commit 65a5b0c into FreeTubeApp:development Mar 14, 2024
5 checks passed

github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Mar 14, 2024

ChunkyProgrammer mentioned this pull request Mar 25, 2024

Add the ability to view Invidious playlists #4299

Closed

1 task

PikachuEXE deleted the feature/playlist-search-playlists-with-matching-videos branch April 2, 2024 03:12

	Search in Videos: Search in Videos
	Search in Videos: Search for Videos in Playlist

	Search bar placeholder: Search in Playlist
	Search bar placeholder: Search for Playlist

Add search playlists with matching videos function #4537

Add search playlists with matching videos function #4537

Conversation

PikachuEXE commented Jan 9, 2024

Pull Request Type

Related issue

Description

Screenshots

Testing

Desktop

Additional context

PikachuEXE commented Jan 9, 2024

absidue commented Jan 9, 2024 • edited Loading

ChunkyProgrammer commented Jan 12, 2024

toby63 commented Jan 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

efb4f5ff-1298-471a-8973-3d47447115dc commented Jan 30, 2024

PikachuEXE commented Jan 30, 2024

kommunarr commented Jan 30, 2024 • edited Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Jan 30, 2024

kommunarr commented Jan 30, 2024 • edited Loading

PikachuEXE commented Jan 31, 2024

toby63 commented Jan 31, 2024

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 9, 2024

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 9, 2024

efb4f5ff-1298-471a-8973-3d47447115dc left a comment

Choose a reason for hiding this comment

toby63 commented Mar 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 10, 2024 • edited Loading

ArthurKun21 commented Mar 10, 2024

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 10, 2024 • edited Loading

toby63 commented Mar 10, 2024 • edited Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 10, 2024 • edited Loading

toby63 commented Mar 10, 2024

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 10, 2024 • edited Loading

toby63 commented Mar 10, 2024

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 11, 2024

toby63 commented Mar 11, 2024

PikachuEXE commented Mar 13, 2024

absidue commented Jan 9, 2024 •

edited

Loading

kommunarr commented Jan 30, 2024 •

edited

Loading

kommunarr commented Jan 30, 2024 •

edited

Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 10, 2024 •

edited

Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 10, 2024 •

edited

Loading

toby63 commented Mar 10, 2024 •

edited

Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 10, 2024 •

edited

Loading

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 10, 2024 •

edited

Loading