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

Fix overflowing videos #1183

Merged
merged 1 commit into from
Jun 20, 2024
Merged

Conversation

owi92
Copy link
Member

@owi92 owi92 commented Jun 14, 2024

The video container width is set to unset when there is enough space for the control elements. I know this had a good-ish reason, I just don't remember it.
On that note, I should start writing better commit messages, or at least try to explain things like that so I can later remember it.

So anyway, this unset width can lead to situations were videos with weird aspect ratios like 15/4 or sth would overflow on smaller (but larger than mobile) screens. To fix that without removing or further complicating the unset rule, this commit introduced a maxWidth of 100%. BUT that also needs to be overwritten on mobile screens to preserve current sizing behaviour on these.

Closes #1173

@owi92 owi92 added the changelog:user User facing changes label Jun 14, 2024
@github-actions github-actions bot temporarily deployed to test-deployment-pr1183 June 14, 2024 10:37 Destroyed
The video container width is set to `unset` when there
is enough space for the control elements. I know this
had a good-ish reason, I just don't remember it.
On that note, I should start writing better commit messages,
or at least try to explain things like that.

So anyway, this unset width can lead to situations were
videos with weird aspect ratios like 15/4 or sth would
overflow on smaller (but larger than mobile) screens.
To fix that without removing or complicating the `unset`
rule, this commit introduced a `maxWidth` of 100%.
BUT, that also needs to be overwritten on mobile screens
to preserve current sizing behaviour on these.
Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code seems reasonable and I tested with a few videos (portrait, this super wide one, normal 16:9) and all seemed fine on all screen sizes I tested.

@LukasKalbertodt LukasKalbertodt merged commit 1d71b78 into elan-ev:master Jun 20, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:user User facing changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some less common video Formats may not scale correctly on smaller screens
2 participants