VIDCS-3437: Account for portrait and landscape cameras on mobile devices #110
Conversation
| @@ -50,6 +50,9 @@ const VideoTile = forwardRef( | |||
| <div | |||
| className={`relative left-0 top-0 size-full overflow-hidden rounded-xl ${isTalking ? 'outline outline-2 outline-sky-500' : ''} ${!hasVideo ? 'hidden' : ''}`} | |||
| ref={ref} | |||
| style={{ | |||
| backgroundColor: 'rgba(60, 64, 67, 0.55)', | |||
There was a problem hiding this comment.
how come we're using style for everything in this PR?
There was a problem hiding this comment.
I changed most of them back. This one needs to stay, though.
Using style made it easier to test different props with the devTools. If I manually edited it through the .object-contain, object-fill, etc., classes, it would make changes for all of the elements with that class.
|
@cpettet, is this ready for review again? If so, I can help since Mike is on Beer time. |
| @@ -216,7 +216,7 @@ describe('getLayoutBoxes', () => { | |||
| ); | |||
| }); | |||
|
|
|||
| it('should call getLayout with shouldMakeLargeTilesLandscape flag false for single pinned participants with no screenshare', () => { | |||
| it('should call getLayout with shouldMakeLargeTilesLandscape flag true for single pinned participants with no screenshare', () => { | |||
There was a problem hiding this comment.
We want non-screenshare video tiles to be landscape
| box && { | ||
| left: box.left, | ||
| top: box.top, | ||
| // We subtract the margins from width and height | ||
| width: box.width - VIDEO_TILE_MARGIN, | ||
| width: box.width - VIDEO_TILE_MARGIN - (isScreenShare ? 0 : 6), |
There was a problem hiding this comment.
In the non-screenshare publisher and subscriber tiles, the video tile container was peaking out. We didn't see it before because we weren't using object-fit: contain, but we were using object-fit: cover which blew up the image. That's also why mobile publishers looked so strange - we were making a 16:9 image out of a 9:16 image 🤠.
We use 6 since there aren't fractional pixels, but I think it should be 6.22 as 14.22 / 8 is a 16 / 9 ratio.
|
|
looks good! |
What is this PR doing?
Description
Makes the publisher match the corresponding subscriber for a participant by maintaining the aspect ratio of the stream. Current behavior shows a zoomed-in version of the publisher, which is especially strange on mobile. Also, I believe this solves the weird grey edges in certain viewports (or it makes it less apparent, at least).
After Images
Before Image 😂
How should this be manually tested?
Reproing the issue
Verifying the fix
What are the relevant tickets?
A maintainer will add this ticket number.
Resolves VIDCS-3437
Checklist
[✅] Branch is based on
develop(notmain).[ ] Resolves a
Known Issue.[ ] If yes, did you remove the item from the
docs/KNOWN_ISSUES.md?[ ] Resolves an item reported in
Issues.If yes, which issue? Issue Number?