Repository navigation
fix: load CldVideoPlayer posters when aspectRatio is set - #674
Open
PaulBoye-py wants to merge 1 commit into
Open
PaulBoye-py wants to merge 1 commit into
PaulBoye-py wants to merge 1 commit into
Conversation
The player splits a transformation array into separate URL steps and adds aspectRatio as a step of its own, which Cloudinary rejects for poster images. When aspectRatio is set and the transformation prop isn't an array, pass the player a single transformation object. Players without aspectRatio keep their existing URLs. See cloudinary/cloudinary-video-player#1087
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
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
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.
Description
With Video Player v4 (next-cloudinary 7.0.0), setting
aspectRatioonCldVideoPlayermakes the poster image fail to load (Cloudinary returns a 400). The video itself plays normally. This is listed as a known limitation in the 7.0.0 release notes, withcropMode="fill"as the workaround.Root cause:
getVideoPlayerOptions(url-loader) passes the transformation to the player as an array:[{ quality }, ...transformation].aspectRatioas one more step on its own. The poster URL then ends with anar_16:9step that has no crop, which Cloudinary rejects for images:…/poster?tx=q_auto/ar_16:9/c_limit,w_700(400).…/poster?tx=ar_16:9,c_limit,q_auto,w_700(200).Changes:
getPlayerTransformation(in theCldVideoPlayerfolder, not exported from the package). WhenaspectRatiois set and thetransformationprop isn't an array, it combines the array fromgetVideoPlayerOptionsinto one object. In every other case it returns the transformation unchanged.CldVideoPlayerpasses its options through the helper. The logic lives in a separate file so it can be unit tested without a DOM test setup.getVideoPlayerOptions) are unaffected.What changes for users:
aspectRatio: no change. Poster and video URLs are exactly the same as in 7.0.0.aspectRatio: the poster loads again. Their poster and video URLs change (one step instead of several), so Cloudinary generates them once under the new URLs. In testing, the delivered video was identical.aspectRatiowithcropMode="fill"): posters already worked and still do. Their URLs change too, and the workaround can be removed.aspectRatiowith atransformationarray: not changed by this PR (the poster still fails), because combining the user's own steps could change what they asked for. Tracked in CldVideoPlayer: poster fails with aspectRatio and a transformation array #675, which also has a tested workaround.Issue Ticket Number
Fixes the "Known limitation" in the 7.0.0 release notes. Upstream context: cloudinary-video-player#1087.
Type of change
Checklist
Testing
Unit tests (
tests/components/getPlayerTransformation.spec.js, 5 cases). They use the realgetVideoPlayerOptions, so a change in url-loader's output would show up here:aspectRatio→ transformation array unchangedaspectRatio+ transformation object → one object (withquality)aspectRatiowithout a transformation → one objectaspectRatio+cropMode+quality→ one objectaspectRatio+ transformation array → unchangedWith the fix switched off, the 3
aspectRatiocases fail. Full suite: 21/21 pass.Browser: 14
CldVideoPlayerscenarios in the Next.js 16 test app (Video Player 4.1.2), compared with 7.0.0:aspectRatio="16:9"aspectRatio+ transformation objectaspectRatio+cropMode="fill"(workaround)aspectRatio+ transformation arrayaspectRatio(sizes, poster string/object, transformation object/array, quality, player options, controls, responsive, missing asset)Videos play in all scenarios except the deliberately missing asset. No page errors.
Build: library builds; no type errors in
src/;test:app(Next.js 16 production build) passes. The helper doesn't appear in the published type declarations.