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

Add playlist duration to playlist view #7007

Open
wants to merge 4 commits into
base: development
Choose a base branch
from

Conversation

MarmadileManteater
Copy link
Contributor

@MarmadileManteater MarmadileManteater commented Mar 14, 2025

Add playlist duration to playlist view

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

This is based on this PR: #6657
closes #6152

Description

This PR adds the total playlist duration to the playlist-info component which is displayed within the Playlist view.

Screenshots

before after
2025-03-14-090636_hyprshot 2025-03-14-090720_hyprshot

Testing

  1. Check a creator playlist (ex: https://youtube.com/playlist?list=PL2EEDBEDC9C2BDF07 )
  2. Check a user playlist
  3. Check a playlist where none of the videos have a duration
    (This can be accomplished by adding only videos from RSS feed subscriptions to a user playlist.)
  4. Check a playlist where at least one of the videos do not have a duration
  5. Check that the localization of the duration works as expected

Desktop

  • OS: Fedora Linux w/ hyprland
  • OS Version: 41 (Workstation Edition) x86_64
  • FreeTube version: b6ba16f

Co-authored-by: Sulliva LUONG <sullivanluong@gmail.com>
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) March 14, 2025 13:19
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Mar 14, 2025
Copy link
Member

Choose a reason for hiding this comment

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

When one short is inside an user playlist i get this error

VirtualBoxVM_regZZWhCaV

User playlist full of shorts throws an error in the console

VirtualBoxVM_V7UIuZ9Psi.mp4

Couldnt find a creator playlist that had shorts without timestamp and normal videos mixed

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Mar 17, 2025
@MarmadileManteater
Copy link
Contributor Author

I'm not able to replicate, but it seems there is a video in that playlist with a non-number for lengthSeconds which is also not undefined. I pushed a change which may address the issue.

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 19, 2025

Errors are gone but when a short is added to a playlist the it wont show total time anymore. To me it seems like a harsh penalty for having shorts without timestamps within the playlist. I totally accept if these are the limitations we cant circumvent though

VirtualBoxVM_PtnY6raIhA.mp4

@MarmadileManteater
Copy link
Contributor Author

That was the behavior I intended, but it can be changed. My logic is that, if there are any number of videos with no duration in a playlist, the total duration will be wrong.

@efb4f5ff-1298-471a-8973-3d47447115dc

Sounds logical. One thing i came up with is to call it approximately total time so we can still show it but maybe that is just bad UX

@PikachuEXE
Copy link
Collaborator

I feel like putting it to another line would make it "less busy"/easier to read
image
image
image
image
image

@MarmadileManteater
Copy link
Contributor Author

@PikachuEXE
Do you have any opinion on whether we should include playlist duration when there are videos without a length in a playlist? (or if that should be displayed as approximate time)

@PikachuEXE
Copy link
Collaborator

Don't show it seems better UX for now
Otherwise got to show how many videos have/have no length which I guess might look weird/confusing to users

@MarmadileManteater MarmadileManteater added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: waiting for review For PRs that are complete, tested, and ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Let the user know how much time it would take to watch an entire playlist
3 participants