LBRYCommunity/lbry-desktop

Fork 0

Fix video width #457

Merged

btzr-io merged 2 commits from patch-3 into master

2017-08-11 00:49:07 +02:00

btzr-io commented

2017-08-09 03:39:22 +02:00

(Migrated from github.com)

The real problem is not the alignment but the the width of the video element.
~Since it's using a container there is no need for position: absolute,~
setting the width to 100% should do the trick 😄

Fixes

Fix: https://github.com/lbryio/lbry-app/issues/419
Prevent regression: #295

![how_to_fix_video_aligment](https://user-images.githubusercontent.com/14793624/29102948-e2a2330e-7c79-11e7-9dbe-6991f3e2cf50.gif) The real problem is not the alignment but the the width of the `video` element. ~Since it's using a container there is no need for `position: absolute`,~ setting the width to `100% ` should do the trick :smile: ### Fixes Fix: https://github.com/lbryio/lbry-app/issues/419 Prevent regression: #295

👍 2

kauffj (Migrated from github.com) reviewed 2017-08-09 03:39:22 +02:00

btzr-io commented

2017-08-09 04:03:39 +02:00

(Migrated from github.com)

I can't request reviews 😞 ^^

I can't request reviews :disappointed: ^^

jsigwart commented

2017-08-09 04:06:16 +02:00

(Migrated from github.com)

Wouldn't this cause horizontal stretching that would look odd on narrow pictures?

btzr-io commented

2017-08-09 04:11:50 +02:00

(Migrated from github.com)

@jsigwart Nope, because it's using object-fit: contain;

contain: increases or decreases the size of the image to fill the box whilst preserving its aspect-ratio.

https://developer.mozilla.org/en-US/docs/Web/CSS/object-fit

@jsigwart Nope, because it's using `object-fit: contain;` > contain: increases or decreases the size of the image to fill the box whilst preserving its aspect-ratio. https://developer.mozilla.org/en-US/docs/Web/CSS/object-fit

jsigwart commented

2017-08-09 04:13:22 +02:00

(Migrated from github.com)

I think what you want to do is this:

video {		   
   height: 100%;
   margin: 0 auto;
}

I think what you want to do is this: ``` video { height: 100%; margin: 0 auto; } ```

hackrush01 commented

2017-08-09 04:14:05 +02:00

(Migrated from github.com)

@btzr-io Could you give a look here.

@btzr-io Could you give a look [here](https://github.com/lbryio/lbry-app/pull/434#issuecomment-320514720).

👍 1

btzr-io commented

2017-08-09 04:18:09 +02:00

(Migrated from github.com)

@jsigwart you don't need margin: 0 auto

@jsigwart you don't need `margin: 0 auto`

👍 1

jsigwart commented

2017-08-09 04:20:49 +02:00

(Migrated from github.com)

Yup, you're exactly right. And that should fix the case of a video being too short, too.

hackrush01 commented

2017-08-09 04:50:12 +02:00

(Migrated from github.com)

This is further along and with a better explanation, so I'm inclined to keep this. Also I'm linking some important comments from that PR to here so the discussion can continue here.
Comment#1

As for it being rendered inside multiple <div> tags, it could be because of the way react handles components(everything can be returned only in a single wrapper tag).

This is further along and with a better explanation, so I'm inclined to keep this. Also I'm linking some important comments from that PR to here so the discussion can continue here. [Comment#1](https://github.com/lbryio/lbry-app/pull/434#issuecomment-321133778) As for it being rendered inside multiple `<div>` tags, it could be because of the way react handles components(everything can be returned only in a single wrapper tag).

👍 1

btzr-io commented

2017-08-09 04:55:32 +02:00

(Migrated from github.com)

Well this is the most easy fix,
not sure if someone want to go deep inside that react component
to figure out what's casing that behavior and if it's really necessary ^^

Well this is the most easy fix, not sure if someone want to go deep inside that react component to figure out what's casing that behavior and if it's really necessary ^^

btzr-io commented

2017-08-09 05:09:43 +02:00

(Migrated from github.com)

I did a quick look, and didn't find nothing wrong with video component,
so that means css is the right fix ^^

I did a quick look, and didn't find nothing wrong with `video` component, so that means `css` is the right fix ^^