@DevYemi is attempting to deploy a commit to the Cloudinary DevX Team on Vercel.

A member of the Team first needs to authorize it.

The latest updates on your projects. Learn more about Vercel for Git ↗︎

hey @DevYemi looking good so far, two comments with questions

additionally, part of the hope was to update the prop name to autoplay (notice no capital "P") which is what the Cloudinary Video Player prop is and what the standard HTML attribute name is

for backwards compatability, we can still recognize autoPlay, at least until I can remove it in a breaking release

so perhaps you could do autoplay || autoPlay somewhere in there based on the props

you can even add a check for autoPlay like:

if ( autoPlay && process.env.NODE_ENV === 'development' ) {
  console.warn('Prop autoPlay will be removed in future versions, please use autoplay (lowercase "p")')
}

hey @DevYemi looking good so far, two comments with questions

additionally, part of the hope was to update the prop name to autoplay (notice no capital "P") which is what the Cloudinary Video Player prop is and what the standard HTML attribute name is

for backwards compatability, we can still recognize autoPlay, at least until I can remove it in a breaking release

so perhaps you could do autoplay || autoPlay somewhere in there based on the props

you can even add a check for autoPlay like:

if ( autoPlay && process.env.NODE_ENV === 'development' ) {
  console.warn('Prop autoPlay will be removed in future versions, please use autoplay (lowercase "p")')
}

Ohh my bad that's true i will make the corrections now, also you are right about backwards compatability and i will also be adding this checker:

if ( autoPlay && process.env.NODE_ENV === 'development' ) {
  console.warn('Prop autoPlay will be removed in future versions, please use autoplay (lowercase "p")')
}

i think this is looking good, i thin kit just needs 1 fix to the Player types as commented

hey @DevYemi sorry for the hold up but noticing that autoplay doesnt actually work as expected

im playing around with this branch and it appears its because you're defining never as the default option for autoplayMode which overrides the autoplay value

if I change the autoplayModeValue variable to:

let autoplayModeValue: string | undefined = undefined;

it works as expected

in addition to that, we can optionally set the value, but i would recommend just leaving it undefined unless someone passes in a string as the rest of the logic has it

thoughts?

hey @DevYemi sorry for the hold up but noticing that autoplay doesnt actually work as expected

im playing around with this branch and it appears its because you're defining never as the default option for autoplayMode which overrides the autoplay value

if I change the autoplayModeValue variable to:

let autoplayModeValue: string | undefined = undefined;

it works as expected

in addition to that, we can optionally set the value, but i would recommend just leaving it undefined unless someone passes in a string as the rest of the logic has it

thoughts?

I believe the suggestion you made is very valid, i will make the corrections and push before night fall today.

just a question should i also do the same for autoplay ?, sets the value to undefined if no value is passed. The current implementation falls back to false

current implementation is:

  let autoPlayValue: boolean | 'true' | 'false' = false;

proposed option:

  let autoPlayValue: boolean | 'true' | 'false' | undefined = undefined;

what do you think ?

doesnt hurt to remove the default as well as it realisticly doesnt do anything as both would be a falsey value that wouldnt trigger it. i dont have a strong opinion on it

changes look good, thank you @DevYemi

🎉 This PR is included in version 5.14.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀