CSS DIV class around the embedded Video

Here's a patch for this as I needed this as well so that I could define a max-width attribute for http://drupal.org/project/fitvids. Hope it helps!

Used to fix this issue as well #1874152: Video Filter with max-width

Wrapping the iframe or object in a div seems common sense. I wrote an identical patch with only a different class name before finding this issue. Patch applies against 7.x-3.1. I don't see any risks with the patch.

It would be a nice additional feature to add a code specific class to the wrapper. This way it is possible to style each wrapper a little differentlly.

The patch has been RTBC for more than a year. Please can we have this merged?

This should be added to the next release.

Shouldn't the attribute be added to the standard field wrapper, rather than adding another DIV?

FYI this will probably need to be rerolled.

Here's a re-rolled patch.

I don't think it makes sense to add attributes to the field wrapper, as this wouldn't be relevant to the case of video filter in body text.

I'd also be inclined to include the relevant CSS in the module for responsive videos so that people who aren't using custom themes get the benefit as well.

Should be easy enough for people who do know CSS to override if needed.

Here's a patch that does that.

FYI, I had better success with https://github.com/toddmotto/fluidvids

Here's my patch draft. It could be improved with a fluidvids module using the Libraries API but for now it works for my needs...

@emmonsaz - nice patch, but you should use hook_page_build instead of hook_init to add css or js.

@pawel.traczynski Nice catch - thanks!

I will not add a dependency to fluidvids, doesn't make sense to me.
I'm leaning towards committing #8, and let people decide for themselves if and how to use the wrapping div.

This doesn't add a dependency, it just takes advantage if the library is found.

This patch makes the PHP code a little more readable and adds a note to the README.txt about it.

Is fluidvids the only way to handle this? The best? I'm concerned there will be more if statements for library X & Y & Z if we go down this road. I might be wrong though, so if this is the preferred way, I'm not gonna be stubborn.

I don't think the current implementation is hurting anything, if someone else wants to add support for another library then maybe it could be refactored at a later point but I think it's fine for now.

Are we abandoning the non-JS approach from #1 - #9 in favor of fluidvids.js? I have #9 implemented on several sites without issue. I have no objection to adding fluidvids.js functionality for those who wish to use it, but if I can solve an issue without JS, that's my preferred route. Take a look at this article, which covers both approaches: http://www.creativebloq.com/netmag/how-add-responsive-video-your-website....

I'd definitely prefer to avoid adding more JS to the page - IMO it's always best to keep the page weight down and avoid unnecessary JS wherever possible.

I don't have an objection to supporting fluidvids.js in addition to #9. The issue that I see us running into is that the CSS from #9 will probably interfere with fluidvids.js

Solution #1
In patch #15, the video_filter_page_build() function checks for the fluidvids.js file before loading it. If the fluidvids.js file doesn't exist, we could load the responsive CSS from #9 using drupal_add_css().

Solution #2
We could also set the wrapper class depending on if the fluidvids.js file exists (e.g. w/o fluidvids.js class="video-filter"; with fluidvids.js class="video-filter-fluidvids"). The CSS from #9 would only apply to the wrapper with the desired class.

I will be committing the simpler solution (#8), and let the discussion continue.

#8 committed.

Here's is a patch to use Bootstrap's responsive videos containers, This should be a good solution for you if you are using bootstrap.

Both the fluidvids.js approach and the Bootstrap approach can be implemented without patching the Video Filter module. The relevant code is contained in theme functions (theme_video_filter_flash and theme_video_filter_iframe), which means it can be overridden. See Overriding theme functions.

This was committed, any further changes can be handled via theming.

I'm not so sure that this is fixed - as it stands, the wrapper div and the iframe both have a class of 'video-filter', which makes targeting the iframe separately from its wrapper unnecessarily difficult.

Here's a new patch that takes the 'video-filter' class off the video itself.

Instead of applying the patch, I wrote some javascript to remove the video-filter class if the video-youtube class was present. Maybe useful for others who have the issues I was having with video-filter class in both the container and the iframe.

Nothing fancy but it works:

  // remove video-filter class from youtube iframes
  $(window).load(function() {
    $('.video-youtube').removeClass('video-filter');
          });