FitVids.js icon indicating copy to clipboard operation
FitVids.js copied to clipboard

Adding maxWidth variable that adds wrapper, retains original max width/height

Open andyadams opened this issue 13 years ago • 14 comments

References issue #10. When the maxWidth option is set to true, an extra container is added that limits the size of the video to the originally-specified dimensions.

My text editor also strips out trailing whitespace, so there's a bunch of that removed as well :).

P.S. Chris, at WordCamp SF I totally ditched mid-talk upstairs to come and hear yours. It was awesome.

andyadams avatar Nov 15 '11 23:11 andyadams

The maxWidth functionality is really nice, but in the cases where I've tried to use it - it has proved something I've just had to put back to false. This is because stopping the video being 100% width, means that the video isn't centered within it's parent division (and hence looks completely ugly on the page). I can't really think of any obvious ways to fix this though. Would I be right in saying that it would require some heavier Javascript to center within the parent with a max-width (or am I just being stupid)?

joesavage avatar Dec 01 '11 20:12 joesavage

@joesavage,

If you wanted a video to be centered, you could add some styles to make it that way - maybe auto margins? I'm not a CSS master, but it would seem that this would be best handled by simply styling appropriately.

The use case I wrote this for is when someone really wants an embedded video to be a certain width - without my fix, the video would automatically become 100% width and nothing could be floated next to it, for example.

andyadams avatar Dec 01 '11 21:12 andyadams

@andyadams Unfortunately doing a simple "margin: 0 auto" requires a fixed width (you can't use min or max-width), hence I can't find any easy way to center the video. I agree that the maxWidth addition is very good for floating and stuff - but it's a shame that there is no easy way (that I can see) to center the video, as many people seem to use FitVids.js to put videos in content division (in which you'd want it to be centered).

EDIT: Actually, coming back to this, using automatic margins with a max-width seems to work fine :P

joesavage avatar Dec 01 '11 21:12 joesavage

Hi! Love this plugin. Just curious, has there been any movement on this patch?

mhulse avatar Apr 18 '12 23:04 mhulse

None from me - @joesavage ?

andyadams avatar Apr 21 '12 01:04 andyadams

This is a great idea. It really should be the default behavior, IMO. It's the same way we treat responsive images, particularly when they are floated with text.

dabernathy89 avatar Dec 11 '12 07:12 dabernathy89

:+1: from me. It'd be nice to not expand the video to larger than its natural dimensions. That ends up looking blurry.

helgatheviking avatar Nov 13 '13 09:11 helgatheviking

So I'm here, 2 years later, combing through PRs. :crying_cat_face:

I'd be interested in getting this into (the mythical) FitVids 2.0, but I wonder if could be done with just one setting instead of two:

maxWidth: false // default, do not wrap
maxWidth: true // set max-width to originalWidth
maxWidth: 900 // set max-width to 900px

Thoughts?

davatron5000 avatar Nov 27 '13 23:11 davatron5000

I think it only needs to be a single option.

graygilmore avatar Feb 04 '14 20:02 graygilmore

+1

palewire avatar Mar 03 '14 02:03 palewire

I also think this should be the default behaviour. Other than that, the plugin works very nicely. Great job!

pomartel avatar Jan 16 '15 16:01 pomartel

:+1: This fork worked for me

rchrd2 avatar Jul 07 '15 05:07 rchrd2

@davatron5000 Any movement on this?

shennan avatar Nov 16 '16 09:11 shennan

@shennan looks like it's being included in the v2 attempt: http://codepen.io/davatron5000/pen/9fef11884a883855d8a1a187bac0cbfe

graygilmore avatar Nov 16 '16 14:11 graygilmore