Skip to content

Conversation

ClearlyClaire
Copy link
Contributor

The video wrapping was forced to have the aspect ratio of the video itself, even when it has left-padding (because it is a reply or an ancestor in a thread), causing extra padding below the video.

@ClearlyClaire ClearlyClaire requested a review from a team March 29, 2025 16:44
@Gargron
Copy link
Member

Gargron commented Mar 29, 2025

There was a reason this wrapper was added, I’m not 100% certain but I believe since the video is not passed width/height anymore without the aspect ratio it would cause a layout jump while it/poster image loads.

@ClearlyClaire
Copy link
Contributor Author

The current behavior is that vertical videos in a thread look like that:
image

I suppose this needs more testing, but the PR preserves the aspectRatio on the inner div, so I don't think this is an issue.

@ClearlyClaire
Copy link
Contributor Author

I have not been able to find a case where removing aspectRatio from the outer div would break anything. Purposefully breaking the video/thumbnail files also did not break the video player sizing in any context I could see.

@Gargron Gargron added this pull request to the merge queue Mar 31, 2025
Merged via the queue into main with commit 19346fd Mar 31, 2025
28 checks passed
@Gargron Gargron deleted the fixes/video-double-indent branch March 31, 2025 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants