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

FitVid modified for Kaltura dynamic embeds #141

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mdale
Copy link

@mdale mdale commented Oct 14, 2013

Change Summary:

  • no $(target).wrap() calls if not needed ( breaks instance reference of extended DOM objects )
  • apply fill CSS directly to target ( not a CSS rule that breaks with rules against injected same domain iframe )
  • use data attribute for fitvid applied flag, not parent selector
  • select wrap against "parents" in cases embed target has DOM manipulations.

I understand this may change the fundamentals too much, but a client requested compatibility so wanted to give you visibility on these changes.

Kaltura dynamic embed works by embedding a 'a same domain iframe' this give us a lot more more control, ( synchronous api, parent page modification for pseudo fullscreen on iPads, IE, host page analytics etc. ) but fitvids's auto iframe selectors and .wrap calls break things a bit.

Isolated test file here:
http://player.kaltura.com/modules/KalturaSupport/tests/FitVids.html

If using kaltura's cross domain iframe based player things work in the normal way without modification.

Michael Dale added 2 commits October 14, 2013 10:53
* no $(target).wrap() calls if not needed ( breaks instance reference of extended DOM objects )
* apply fill CSS directly to target ( not a CSS rule that breaks with rules against injected same domain iframe )
@davatron5000
Copy link
Owner

Seems like a lot of modifications and if just to make it work with one video vendor. Currently we bail on the existence of .fluid-width-video-wrapper. We could just as easily return on data-fitvids if that solves a problem for you. Maybe you can explain a bit more why you moved all those styles down inline? That seems like things are getting too repetitive. It's fine if there's one video on the page (probably most sites?) but if there's multiple videos per page, it seems inefficient.

@mdale
Copy link
Author

mdale commented Oct 14, 2013

Yea ... I recognize its mostly for the way we handle embeds. The data attribute would help, but the .wrap calls are the main issue, since they break the instance references. ... Its fine if we just link to my fork for when people search for the issue.

@mdale
Copy link
Author

mdale commented Oct 14, 2013

Also to clarify about the styles being moved inline... the kaltura player outputs its own iframe, but it needs to manage it independently.

i.e if fitVid is to work on custom player "div" targets, it should not make assumptions about whats underneath, i.e only modify the rules of the target specified, not any children that may exist per that embed mechanism. Consider other players that may work in-page on a <video> tag, and have some child object to support flash in IE ( for example: video.js, mediaElement.js, etc. )

Efficiently wise, I think other things will break down with many in-page players long before the css style vs class setup costs come into effect ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants