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 single playlst view for user playlist to have pagination #4593

Conversation

PikachuEXE
Copy link
Collaborator

@PikachuEXE PikachuEXE commented Jan 23, 2024

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

Maybe this makes #4578 better

Description

Add pagination to single playlist view for user playlists

Screenshots

image

Testing

  • Add some playlists with 100+ videos (e.g. copy some playlists from https://www.youtube.com/channel/UCXuqSBlHAE6Xw-yeJA0Tunw, sort by Last Video Added)
  • Ensure single playlist view got videos paginated
  • Ensure pagination limit NOT remembered
  • Ensure user playlist can be copied without warning even not all videos loaded
  • Ensure this issue is fixed: When moving a video in a playlist up or down, the animation for the videos swapping plays twice
  • What else

Desktop

  • OS:
  • OS Version:
  • FreeTube version:

Additional context

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) January 23, 2024 01:08
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jan 23, 2024
@kommunarr
Copy link
Collaborator

Bug that I discovered while testing this (not sure if novel): When moving a video in a playlist up or down, the animation for the videos swapping plays twice.

@kommunarr
Copy link
Collaborator

kommunarr commented Jan 23, 2024

bug (blocking): My playlist with 4000 videos became 200 videos during testing of this. Edit: It just happened again after hard restarting the app, truncating the playlist to 100 videos. Those saved videos are gone for good. Seems to be unique to this PR.

issue (non-blocking): This makes copying user playlists as irksome as copying remote playlists now.

@kommunarr
Copy link
Collaborator

kommunarr commented Jan 23, 2024

bug: it stopped paginating for me after 300 videos.
Screenshot_20240122_194752

bug: it also stopped paginating for other playlists too. Even when switching to a different tab (like Subscriptions) and back. Even after Ctrl + R... I had to restart the app.

@PikachuEXE
Copy link
Collaborator Author

My playlist with 4000 videos became 200 videos during testing of this. Edit: It just happened again after hard restarting the app, truncating the playlist to 100 videos. Those saved videos are gone for good. Seems to be unique to this PR.

Fixed

it also stopped paginating for other playlists too. Even when switching to a different tab (like Subscriptions) and back. Even after Ctrl + R... I had to restart the app.

Coz I didn't push 2nd commit before I submit PR, please re-test

@PikachuEXE
Copy link
Collaborator Author

This makes copying user playlists as irksome as copying remote playlists now.

Fixed

@PikachuEXE
Copy link
Collaborator Author

When moving a video in a playlist up or down, the animation for the videos swapping plays twice.

Caused by #4518 but I don't know how yet...

@PikachuEXE
Copy link
Collaborator Author

When moving a video in a playlist up or down, the animation for the videos swapping plays twice.

Fixed

Copy link
Contributor

github-actions bot commented Feb 1, 2024

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

@github-actions github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Feb 1, 2024
@PikachuEXE
Copy link
Collaborator Author

Not needed anymore

@PikachuEXE PikachuEXE closed this Feb 1, 2024
auto-merge was automatically disabled February 1, 2024 12:40

Pull request was closed

@PikachuEXE PikachuEXE deleted the feature/playlist/pagination branch April 2, 2024 03:13
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.

2 participants