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 YouTube URL with Additional Parameters #177

Merged
merged 3 commits into from
Jul 31, 2024

Conversation

untari
Copy link
Contributor

@untari untari commented Jul 26, 2024

This PR is for issues: #170
This PR enhances the YouTube embedding functionality in the media source component by updating the URL construction to include additional parameters. The changes ensure that the YouTube videos are embedded with the following options:

  • Enable JavaScript API (enablejsapi=1)
  • Autoplay (autoplay=1)
  • Modest branding (modestbranding=1)
  • Loop (loop=1)
  • Hide controls (controls=0)
  • Disable keyboard controls (disablekb=1)
  • Hide related videos at the end (rel=0)
  • Hide video info (showinfo=0)
  • Use no-cookies (https://www.youtube-nocookie.com)
  • Playlist set to the video ID to enable looping (playlist=${ytid)

Summary by Sourcery

This pull request enhances the YouTube embedding functionality by updating the URL construction to include parameters such as enabling JavaScript API, autoplay, modest branding, looping, hiding controls, disabling keyboard controls, hiding related videos, hiding video info, and using no-cookies. These changes ensure a more controlled and customized embedding experience.

  • Enhancements:
    • Updated YouTube URL construction in the media source component to include additional parameters for enhanced embedding functionality.

Copy link

sourcery-ai bot commented Jul 26, 2024

Reviewer's Guide by Sourcery

This pull request enhances the YouTube embedding functionality in the MediaSource component by updating the URL construction to include additional parameters such as enablejsapi, autoplay, modestbranding, loop, controls, disablekb, rel, showinfo, and playlist. The changes also switch to using the no-cookie URL for privacy-friendly embedding.

File-Level Changes

Files Changes
webapp/src/components/MediaSource.vue Enhanced the YouTube embedding functionality by updating the URL construction to include additional parameters for better control and privacy.

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @untari - I've reviewed your changes - here's some feedback:

Overall Comments:

  • The changes look good and achieve the desired functionality. For future improvements, consider creating a dedicated function for constructing the YouTube URL with parameters. This could enhance maintainability and reduce code duplication.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

webapp/src/components/MediaSource.vue Outdated Show resolved Hide resolved
@mariobehling
Copy link
Member

Please also implement switch buttons for the features. This will also help users to see what features are available.

In the previous version of this feature there were switch buttons. A switch button or other button would be expected for these features here as well. For example I am not clear how to use the no cookie domain as a URL parameter otherwise.

There are example of a switch button on this page already.

Screenshot from 2024-07-27 18-01-30

@mariobehling mariobehling merged commit 692b47a into fossasia:development Jul 31, 2024
1 of 2 checks passed
@mariobehling
Copy link
Member

I merged this as it solves some issues, but we need the switch buttons as well.

@untari
Copy link
Contributor Author

untari commented Aug 1, 2024

Hi @mariobehling will open new pr for the switch button

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