jazzband / django-embed-video

Django app for easy embedding YouTube and Vimeo videos and music from SoundCloud.
http://django-embed-video.rtfd.org
MIT License
382 stars 135 forks source link

Relative size #19

Closed Calzzetta closed 10 years ago

Calzzetta commented 10 years ago

Now is possible to input relative size.

yetty commented 10 years ago

Hi, thank you for your patch. But before the merge, please do following:

  1. Fix current tests which are failing.
  2. Write own tests for added feature.
  3. Mention your update in documentation. + add examples of usage.

How to run tests: http://django-embed-video.readthedocs.org/en/v0.8/development/testing.html

yetty commented 10 years ago

One more thing - please remove scrolling="no" to have the embed code as simple as possible. If you really need it, you can create custom backend. But I don't think it should be default.

Second reason is that it will be more compatible among version. I can be wrong if I thing it is not really needed and useful - if you think so, please provide examples of usages and reasons why to merge it.

Usage of custom backends: http://django-embed-video.readthedocs.org/en/v0.8/examples.html#custom-backends

Calzzetta commented 10 years ago

Hi Yetty, thanks for the help! I made the changes, but before merge I want to add another thing.

On soundcloud embed is possible to have two layouts. For exemple:

<iframe width="100%" height="450" scrolling="no" frameborder="no" src="https://w.soundcloud.com/player/?url=https%3A//api.soundcloud.com/tracks/135210964&amp;auto_play=false&amp;hide_related=false&amp;visual=true"></iframe>

or

<iframe width="100%" height="166" scrolling="no" frameborder="no" src="https://w.soundcloud.com/player/?url=https%3A//api.soundcloud.com/tracks/135210964&amp;color=ff5500&amp;auto_play=false&amp;hide_related=false&amp;show_artwork=true"></iframe>

The parameter that changes a lot the layout is the parameter 'visual'. How can I set the parameter visual to false?

yetty commented 10 years ago

The preffered way is to use variables (video backend, code etc.) to build custom tag in template.

Example:

{% video object.video as my_video %}
  {% if my_video.backend == "SoundCloudBackend" %}
    {# notice `scrolling` option #}
    <iframe width="100%" height="450" scrolling="no" frameborder="no"
     {# in following line you can set custom url #}
     src="https://w.soundcloud.com/player/?url=http%3A%2F%2Fapi.soundcloud.com%2Ftracks%2F{{ my_video.code }}&show_artwork=true&visual=false...">
     </iframe>
  {% else %}
    {% video my_video "large" %}
  {% endif %}
{% endvideo %}

The reasons why I don't to include this logic into backends:

There is an effort to keep package simple but without limited usages.

yetty commented 10 years ago

I've merged relative sizes.

For modifying of template, you can use custom template in current development version of embed_video:

http://django-embed-video.readthedocs.org/en/latest/api/embed_video.backends.html#embed_video.backends.VideoBackend.template_name