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

fix(ustream): corrects the url for ustream #215

Merged
merged 7 commits into from
Mar 8, 2017

Conversation

SnehaMidha
Copy link

This issue was not mentioned but the rendered link was not valid for ustream .This commit fixes that issue.

@ritz078
Copy link
Owner

ritz078 commented Mar 6, 2017

I think it works good even now https://codepen.io/ritz078/pen/QpKQQy . Can you fork this and give an example where this doesn't work ? I am using v4.1.16.

Edit: Ustream is broken because the recorded term is not removed from the URL.

@SnehaMidha
Copy link
Author

Everything woks fine on codepen but on my system, using the file demo/index.html, this appears with old code:
image
It says that we are trying to access the link as http://www.http//www.ustream.tv/embed/1524, and it works fine only after changing that line.

@ritz078
Copy link
Owner

ritz078 commented Mar 7, 2017

In that case can't add //www blindly in front. There should be a check whether its already present.

@@ -93,7 +93,14 @@ export default {
if (match.indexOf('/embed/') < 0) {
id.splice(1, 0, 'embed')
}
return `<div class="ejs-embed ejs-ustream"><iframe src="//www.${id.join('/')}" height="${options.videoHeight}" width="${options.videoWidth}"></iframe></div>`
console.log("ustream");
Copy link
Owner

Choose a reason for hiding this comment

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

remove this

console.log(id);
//check if '//www.' is there in the link
if(match.indexOf('/www\.ustream\.tv/') < 0){
return `<div class="ejs-embed ejs-ustream"><iframe src="//www.${id.join('/')}" height="${options.videoHeight}" width="${options.videoWidth}"></iframe></div>`
Copy link
Owner

Choose a reason for hiding this comment

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

prevent multiple returns instead assign the src to a variable and use that in the template.

@ritz078
Copy link
Owner

ritz078 commented Mar 8, 2017

Also I see test is breaking

@ritz078
Copy link
Owner

ritz078 commented Mar 8, 2017

Have you tested with any example ?

@ritz078 ritz078 changed the title corrects the url for ustream fix(ustream): corrects the url for ustream Mar 8, 2017
@ritz078 ritz078 merged commit 6b9e165 into ritz078:master Mar 8, 2017
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