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 copy/open YT video in playlist view to have playlist ID when present #2848

Conversation

PikachuEXE
Copy link
Collaborator

@PikachuEXE PikachuEXE commented Nov 9, 2022

Update copy/open YT video in playlist view to have playlist ID when present

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

Similar to #1258

Description

Same as title

Screenshots

image
image

Testing

A

  • Open a playlist
  • Copy video URL/open video for a video via menu

B

  • Open a playlist
  • Play a video (in FT)
  • Copy video URL/open video for a video in playlist component via menu

Only playlist view and playlist in video playing view are tested by me, please test more views~

Desktop

  • OS: MacOS
  • OS Version: 12.6.1
  • FreeTube version: 2b23381

Additional context

I found that this feature is absent when I try to copy a video URL with playlist for non current playing video

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) November 9, 2022 07:19
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 9, 2022
// `playlistId` can be undefined
if (this.playlistId && this.playlistId.length !== 0) {
// `index` seems can be ignored
return `https://www.youtube.com/watch?v=${this.id}&list=${this.playlistId}`
Copy link
Contributor

Choose a reason for hiding this comment

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

The shortened, https://youtu.be does also support playlists:
https://youtu.be/watch?v=${this.id}&list=${this.playlistId}
e.g.
https://youtu.be/watch?v=NWsMzSe5gLw&list=PL8mG-RkN2uTyfj8UNijuKK6Y38Cz7eQKJ

Whether the slightly shorter url matters is up to you; but at least then you would get the same domain back from this function regardless of playlist or not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for letting me know
Will update soon

I was just copying this from an existing implementation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found out that
https://youtu.be/${this.id}?list=${this.playlistId} also works
https://youtu.be/bStmUUGMA0A?list=PLt6FSN4PoPuSzKNYTjGpv133iaK_i2JyZ
Gonna update to that

Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

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

Tested locally

@absidue
Copy link
Member

absidue commented Dec 5, 2022

@ChunkyProgrammer @efb4f5ff-1298-471a-8973-3d47447115dc we need your reviewing powers 🦸

@PikachuEXE
Copy link
Collaborator Author

Yes please help Pikachu recharge
image

Copy link
Member

Choose a reason for hiding this comment

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

Fast charging enabled!

@FreeTubeBot FreeTubeBot merged commit fa9a992 into FreeTubeApp:development Dec 6, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 6, 2022
@PikachuEXE PikachuEXE deleted the feature/video-url-copy-in-playlist branch December 6, 2022 08:02
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.

6 participants