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

Migrate the Playlist page to youtubei.js #2903

Merged
merged 2 commits into from Dec 13, 2022
Merged

Migrate the Playlist page to youtubei.js #2903

merged 2 commits into from Dec 13, 2022

Conversation

absidue
Copy link
Member

@absidue absidue commented Nov 30, 2022

Migrate the Playlist page to youtubei.js

Pull Request Type

  • Feature Implementation

Description

This replaces the use of ytpl on the playlist page with youtubei.js, it doesn't implement pagination yet, as I feel like that should be implemented for the Invidious API at the same time.

I also haven't touched the playlist view on the watch page yet, currently we force ytpl to paginate the playlist and return all the videos in one go (takes ages), YouTube on the other hand only shows 200 videos on the watch page, so maybe we want to switch to only showing a few pages of the playlist on the watch page?

Testing

Try viewing various playlists, it should behave the same as before.

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.18.0

Additional context

Love having autocomplete.

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 30, 2022 22:56
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 30, 2022
@ChunkyProgrammer
Copy link
Member

Different types of playlist urls:

Explore page playlist:
https://m.youtube.com/playlist?list=RDCLAK5uy_nxNaxtS5HwGabWeGHsAADVA4KKl5l12eQ

Regular playlist:
https://www.youtube.com/playlist?list=PL8mG-RkN2uTx9Er8AqFDnD6mFvje5fybs

Channel videos (autogenerated):
https://www.youtube.com/playlist?list=UULPXuqSBlHAE6Xw-yeJA0Tunw

@PikachuEXE
Copy link
Collaborator

I got issue loading https://m.youtube.com/playlist?list=RDCLAK5uy_nxNaxtS5HwGabWeGHsAADVA4KKl5l12eQ
image
(No issue on development)

The other 2 playlists got no issue

@absidue
Copy link
Member Author

absidue commented Dec 1, 2022 •
edited

It's possible that those randomly generated playlists, that don't actually exist, aren't supported by youtubei.js either, as they don't work on the YouTube website or with ytpl.
playlist-error-banner

I'll look into it though.

@absidue
Copy link
Member Author

absidue commented Dec 1, 2022

They are mostly supported just need a few changes upstream to make them work properly. The thumbnails array for the playlist author is empty but as all the other information is there, it seems worth it.

@absidue absidue mentioned this pull request Dec 1, 2022
Merged
9 tasks
@absidue
Copy link
Member Author

absidue commented Dec 1, 2022

Marking this as WIP until LuanRT/YouTube.js#251 is merged or discussed.

@absidue absidue marked this pull request as draft December 1, 2022 21:50
auto-merge was automatically disabled December 1, 2022 21:50

Pull request was converted to draft

@absidue absidue added PR: WIP and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Dec 1, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Dec 2, 2022

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

Conflicts have been resolved. A maintainer will review the pull request shortly.

@FreeTubeApp FreeTubeApp deleted a comment from github-actions bot Dec 10, 2022
@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@absidue
Copy link
Member Author

absidue commented Dec 10, 2022

The upstream PR has been merged but I'll leave this as a draft until it's included in a release.

@ChunkyProgrammer
Copy link
Member

The upstream PR has been merged but I'll leave this as a draft until it's included in a release.

A new release was published fyi

PikachuEXE
PikachuEXE reviewed Dec 12, 2022
View reviewed changes
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

This?

src/renderer/components/playlist-info/playlist-info.js Show resolved Hide resolved
src/renderer/components/playlist-info/playlist-info.js Show resolved Hide resolved
@absidue absidue marked this pull request as ready for review December 12, 2022 15:30
@absidue absidue removed the PR: WIP label Dec 12, 2022
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) December 12, 2022 15:31
@absidue absidue added the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 12, 2022
@absidue
Copy link
Member Author

absidue commented Dec 12, 2022

This is ready for review now, rebased it on the development branch so it can be tested with the latest youtubei.js version.

PikachuEXE
PikachuEXE approved these changes Dec 13, 2022
View reviewed changes
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

All 3 loaded fine

@FreeTubeBot FreeTubeBot merged commit 36fc946 into FreeTubeApp:development Dec 13, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 13, 2022
@absidue absidue deleted the playlists-youtubei branch December 13, 2022 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PikachuEXE PikachuEXE PikachuEXE approved these changes

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc approved these changes

@ChunkyProgrammer ChunkyProgrammer ChunkyProgrammer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@absidue @ChunkyProgrammer @PikachuEXE @efb4f5ff-1298-471a-8973-3d47447115dc @FreeTubeBot