-
Notifications
You must be signed in to change notification settings - Fork 72
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 initial Warpwire integration #75
Conversation
Related PR: h5p/h5p-editor-php-library#152 |
For testing, you can use this video URL: https://support.warpwire.com/w/CUgAAA/ |
scripts/warpwire.js
Outdated
*/ | ||
|
||
var getId = function (url) { | ||
const matches = url.match(/^[^\/]+:\/\/([^\/]*\.[^\/]+)\/w\/(.+)\/.*$/); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a bit generic? I.e: Won't warpwire.com always be part of the url?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a bit broad because Warpwire URLs can be whitelabeled. We can improve this regex slightly by changing the \/w\/(.+)
to \/w\/[-_0-9a-zA-Z]{6,7}
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fnoks this has been fixed.
@fnoks I have merged in the latest master and fixed the one small conflict. Please let me know if there are any other things I need to address or any other questions you have. |
I would love to see this integration and also see it in Interactive Video. @fnoks Any chance of approving this? |
Warpwire is a video management and playback platform that is used mostly by educational institutions. It has been requested that we support H5P. To that end, we have created the necessary script and tested it on Moodle and Drupal 7.