-
-
Notifications
You must be signed in to change notification settings - Fork 46
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
Attribute and params support for the iframe embed code #5
Conversation
…le to specify customs params and attributes to the iframe tag
'embed-width' => '480', | ||
'embed-height' => '295', | ||
'image-src' => 'https://img.youtube.com/vi/$2/0.jpg', | ||
'iframe-player' => 'https://www.youtube.com/embed/$2', |
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.
Should be tabs
Awesome stuff!!! 👯 |
Sorry. Check it now! |
Looks good to me :) Should be fully BC should it not? |
sorry, BC? what do you mean? |
backwards compatible, so it doesnt break existing stuff :) |
ah, yeah! no problem is fully compatible. By default it shows exactly as you did. You can check the examples. |
} | ||
if (!isset($this->_objectParams[$key])) { | ||
return null; | ||
else { |
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.
In this case you dont need an else, since you return early.
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.
This is something from github, i don't have this "else"
https://github.com/microstudi/MediaEmbed/blob/master/src/MediaEmbed/Object/MediaObject.php#L346
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.
Look at the line number beside it (316).
So you do here, though:
https://github.com/microstudi/MediaEmbed/blob/master/src/MediaEmbed/Object/MediaObject.php#L316
and
https://github.com/microstudi/MediaEmbed/blob/master/src/MediaEmbed/Object/MediaObject.php#L342
It's just a good coding practice to keep the levels to a minimum
returning early and avoiding elses does that.
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.
ok
Attribute and params support for the iframe embed code
Hey, great class here.
When I was using your class I realized than the iframe embed code cloud not be customized so I modified the code in order to do so.
Now you can do something like:
This should return and embed code like:
I updated the main classes, tests, examples and README.md files
I hope you found it usefull.