Fix video being cropped/zoomed on mobile#118
Merged
Merged
Conversation
The video is 16:9. With the default object-fit: cover, a narrow/portrait viewport (basically any phone held normally) forces the video to scale up so much to cover the full height that it crops out most of the picture - reported as "zoomed in" and "doesn't fit". Desktop was unaffected since wide viewports don't need much cropping to begin with. Added a max-aspect-ratio: 1/1 media query that switches to object-fit: contain (letterboxed, but the whole frame stays visible) for anything narrower than it is tall, regardless of the OBJECT_FIT setting - cover never looks right in that situation regardless of what's configured. Verified locally: full frame visible and letterboxed at 375x812, still cover (untouched) at 1280x800. Adds a Playwright regression test (test-mobile-fit.js) checking the computed object-fit at both a narrow/portrait and a wide desktop viewport size.
Reported as "the error page shows as loading" - not a div-id mixup, the
correct #site-error div was showing the whole time. The <title> tag is
static HTML though, so it always starts as PRE_TITLE ("Loading..." by
default) regardless of which state actually gets picked, and nothing
was overriding it for the error case - so the tab said "Loading..."
over a page that says it errored. Now sets document.title to "Internal
Server Error" when that state is chosen, matching what's on screen.
test-autoplay.js was actually asserting the bug: it hardcoded "Loading..."
as the expected initial title for both overlay types, which passed
before the fix (title genuinely was always "Loading...") and would have
caught this immediately if it had asserted the right value per type.
Fixed to expect the correct title per overlay.
Also broadened test-mobile-fit.js from two viewport checks to six -
mobile/tablet portrait (expect contain), tablet landscape through 4K
desktop (expect cover) - per request to verify all the way up to actual
4K resolution, not just one arbitrary desktop size.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The video is 16:9. With the default
object-fit: cover, a narrow/portrait viewport (basically any phone held normally) forces the video to scale up so much to cover the full height that it crops out most of the picture - this is the "zoomed in, doesn't fit" behavior reported on mobile. Desktop was unaffected since wide viewports don't need much cropping to cover in the first place.Added a
max-aspect-ratio: 1/1media query that switches toobject-fit: contain(letterboxed, but the whole frame stays visible) for anything narrower than it is tall, regardless of theOBJECT_FITenv var -covernever looks right in that situation no matter what's configured. Desktop/landscape viewing is completely untouched.Test plan
object-fit: cover, unchangedtest-mobile-fit.js- Playwright regression test checking computedobject-fitat both viewport sizestest.yml) passes