Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: Add methods to add and remove <source> elements #8886

Merged
merged 5 commits into from
Oct 9, 2024

Conversation

alex-barstow
Copy link
Contributor

Description

It is useful to have methods for appending and removing <source> elements to the <video> element, as they are sometimes required to enable certain playback features, for example, using Airplay with MSE.

Specific Changes proposed

Add new methods-- addSourceElement() and removeSourceElement() to the player and tech. The former will take a source object and create and append a new <source> element to the <video> element, and the latter will take a source url and remove any <source> element with a matching src.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chrome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
    • Has no DOM changes which impact accessiblilty or trigger warnings (e.g. Chrome issues tab)
    • Has no changes to JSDoc which cause npm run docs:api to error
  • Reviewed by Two Core Contributors

Copy link

codecov bot commented Oct 8, 2024

Codecov Report

Attention: Patch coverage is 93.10345% with 2 lines in your changes missing coverage. Please review.

Project coverage is 83.25%. Comparing base (077077b) to head (b81d154).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/js/tech/html5.js 90.47% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8886      +/-   ##
==========================================
+ Coverage   83.21%   83.25%   +0.03%     
==========================================
  Files         120      120              
  Lines        8068     8097      +29     
  Branches     1938     1944       +6     
==========================================
+ Hits         6714     6741      +27     
- Misses       1354     1356       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

removeSourceElement(srcUrl) {
if (!srcUrl) {
log.error('Source URL is required to remove the source element.');
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason this function returns a boolean but addSourceElement does not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, both probably should. The reason I didn't have addSourceElement() return a boolean is because originally I hadn't added that early return so a source element would always be added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@alex-barstow alex-barstow force-pushed the feat/addSrcAsSourceElement-tech-method branch from ff4cda9 to b81d154 Compare October 8, 2024 23:31
@alex-barstow alex-barstow merged commit eddda97 into main Oct 9, 2024
14 checks passed
@alex-barstow alex-barstow deleted the feat/addSrcAsSourceElement-tech-method branch October 9, 2024 16:16
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants