I use a Responsive Framework like ZURB Foundation 3 on Drupal. This framework use specific CSS div container to make Videos like vimeo or youtube Responsive. http://foundation.zurb.com/docs/elements.php#vidCode

It will be nice to define globally the css class because i need not to define this on every content.

Membership dollars fund testing for the Drupal project. Drupal Association Learn more


deggertsen’s picture

Version: 7.x-3.1 » 7.x-3.x-dev
Priority: Minor » Normal
Status: Active » Needs review
1.38 KB

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

Chris Burge’s picture

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.

Chris Burge’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
mnlund’s picture

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.

Pere Orga’s picture

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

DamienMcKenna’s picture

This should be added to the next release.

DamienMcKenna’s picture

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

FYI this will probably need to be rerolled.

malcomio’s picture

Status: Reviewed & tested by the community » Needs review
1.47 KB

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.

malcomio’s picture

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.

emmonsaz’s picture

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

emmonsaz’s picture

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...

pawel.traczynski’s picture

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

emmonsaz’s picture

@pawel.traczynski Nice catch - thanks!

blackdog’s picture

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.

DamienMcKenna’s picture

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.

blackdog’s picture

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.

DamienMcKenna’s picture

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.

Chris Burge’s picture

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....

malcomio’s picture

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.

Chris Burge’s picture

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.

blackdog’s picture

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

blackdog’s picture

Status: Needs review » Active

#8 committed.

  • blackdog committed f9a7151 on 7.x-3.x authored by deggertsen
    Issue #1927318 by emmonsaz, malcomio, DamienMcKenna, deggertsen: CSS DIV...
nedSaf’s picture

2.54 KB

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.

Chris Burge’s picture

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.

DamienMcKenna’s picture

Status: Active » Fixed

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

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

malcomio’s picture

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.