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

Update pagination logic when searching to add videos to a playlist #4555

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ChunkyProgrammer
Copy link
Contributor

The old logic would filter the results and use the count of the filtered results to determine if a next page should be shown. If there were playlists or channels then the next page button would not be shown even if there was a next page.

@ChunkyProgrammer ChunkyProgrammer requested a review from a team as a code owner April 2, 2024 17:23
@ChunkyProgrammer ChunkyProgrammer requested review from SamantazFox and removed request for a team April 2, 2024 17:23
@SamantazFox
Copy link
Member

That's much simpler than what I had in mind ^^ (which involved returning the continuation token from very far in the call stack)

@SamantazFox SamantazFox added the need-testing This feature needs to be deployed and tested to see if it's working, and doesn't break something label Apr 10, 2024
@SamantazFox
Copy link
Member

Still doesn't work.

That's probably because we skip the categories (e.g "Latest (shorts) from [channel]" and "People also watched").
I'd bet it's caused by "Latest posts from [channel]", as this type of search result are not supported yet.

Here I tested searching for linus tech tips on the test instance, and there is no button:

image

@SamantazFox SamantazFox added unfinished More work is needed on this PR, or on something this PR uses. and removed need-testing This feature needs to be deployed and tested to see if it's working, and doesn't break something labels Apr 23, 2024
@ChunkyProgrammer
Copy link
Contributor Author

@SamantazFox yeah, looks like this PR isnt working because #4583 is no longer working

@ChunkyProgrammer
Copy link
Contributor Author

Also, I think you're using the search page in the screenshot (which was not modified by this code, I only modified the screen where you search videos to add to a playlist /add_playlist_items?q=SEARCH_QUERY)

@SamantazFox
Copy link
Member

Also, I think you're using the search page in the screenshot (which was not modified by this code, I only modified the screen where you search videos to add to a playlist /add_playlist_items?q=SEARCH_QUERY)

You're right .-.

@SamantazFox SamantazFox added need-testing This feature needs to be deployed and tested to see if it's working, and doesn't break something and removed unfinished More work is needed on this PR, or on something this PR uses. labels May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-testing This feature needs to be deployed and tested to see if it's working, and doesn't break something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants