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

Adding extraction of videoID from youtube videos url which contain '/live/' #5426

Merged
merged 5 commits into from
Nov 27, 2023

Conversation

IshaanDasgupta
Copy link
Contributor

Solved #5416
Added an extra conditional statement for extracting 'videoID' from youtube video urls containing '/live/'

Copy link

netlify bot commented Nov 23, 2023

Deploy Preview for volto canceled.

Name Link
🔨 Latest commit 4e109a8
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/65621929cd2abf0008fd3f7f

Copy link
Member

@ericof ericof left a comment

Choose a reason for hiding this comment

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

Could you, please, add a test for it? I believe we should have this in a function that can be properly tested

@IshaanDasgupta
Copy link
Contributor Author

Sure, I am not familiar with adding tests but I'll look into this.

@IshaanDasgupta
Copy link
Contributor Author

@ericof I have added the tests could you please go through them once as this my first time writing jest tests so if there is something wrong do let me know.

Also I have moved the videoDetails extraction code to a separate function and there was also a issue to video's thumbnails not begin displayed so I also changed the logic to bit to get the correct thumbnail URLs.

@IshaanDasgupta
Copy link
Contributor Author

The first screenshot is from https://demo.plone.org/ where the videos in which the url contains "list" or ".be" dont have their thumbnails displayed, the second one is from my local server. There was a problem with the extraction of the videoID from the urls with "list" and ".be".

Screenshot from 2023-11-25 21-14-00
Screenshot from 2023-11-25 21-17-03

Copy link
Member

@ericof ericof left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this!

@ericof ericof merged commit 5d7b7c1 into plone:main Nov 27, 2023
sneridagh added a commit that referenced this pull request Nov 27, 2023
* main:
  Update yarnhook to 0.6.1 in order to support pnpm 8 (#5444)
  Special breadcrumb in controlpanel (#5292)
  Adding extraction of videoID from youtube videos url which contain '/live/' (#5426)
  Release 18.0.0-alpha.2
  Release generate-volto 8.0.3
  Changelog
  Bring back yeoman install which in v5 is deprecated (#5438)
  Improvements and completeness of the ContentMetadataTags component (#5433)
sneridagh added a commit that referenced this pull request Nov 30, 2023
* main:
  Move dangling news item to the right place
  Better start script in root package.json, enable and better config for husky/lint-staged
  All in with monorepo (#5409)
  changed toNumber to parseFloat   #5447 (#5448)
  Release 18.0.0-alpha.3
  Changelog
  Revert "Improvements and completeness of the ContentMetadataTags component (#5433)" (#5449)
  Update yarnhook to 0.6.1 in order to support pnpm 8 (#5444)
  Special breadcrumb in controlpanel (#5292)
  Adding extraction of videoID from youtube videos url which contain '/live/' (#5426)
  Release 18.0.0-alpha.2
  Release generate-volto 8.0.3
  Changelog
  Bring back yeoman install which in v5 is deprecated (#5438)
  Improvements and completeness of the ContentMetadataTags component (#5433)
  Use container from component registry in sitemap component and also refactor the class to functional component(#5418) (#5420)
  Fix image paths in development mode (#5429)
  Visible focus semantic UI - reset (#5335)
  Remove mention of LTS in Volto (#4905)
  JSX is now in Pygments (#5412)
sneridagh added a commit that referenced this pull request Nov 30, 2023
* main: (21 commits)
  Cleanup and renaming (#5456)
  Move dangling news item to the right place
  Better start script in root package.json, enable and better config for husky/lint-staged
  All in with monorepo (#5409)
  changed toNumber to parseFloat   #5447 (#5448)
  Release 18.0.0-alpha.3
  Changelog
  Revert "Improvements and completeness of the ContentMetadataTags component (#5433)" (#5449)
  Update yarnhook to 0.6.1 in order to support pnpm 8 (#5444)
  Special breadcrumb in controlpanel (#5292)
  Adding extraction of videoID from youtube videos url which contain '/live/' (#5426)
  Release 18.0.0-alpha.2
  Release generate-volto 8.0.3
  Changelog
  Bring back yeoman install which in v5 is deprecated (#5438)
  Improvements and completeness of the ContentMetadataTags component (#5433)
  Use container from component registry in sitemap component and also refactor the class to functional component(#5418) (#5420)
  Fix image paths in development mode (#5429)
  Visible focus semantic UI - reset (#5335)
  Remove mention of LTS in Volto (#4905)
  ...
pranayjoshi pushed a commit to pranayjoshi/volto that referenced this pull request Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants