-
Notifications
You must be signed in to change notification settings - Fork 34
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
CldVideoPlayer config prop #196
Comments
Quick question about global config. Are you sure that we want to pass the config from global configuration to the CldVideoPlayer as well? The global config uses the |
good question about the config, but we need to normalize on something and i think it makes sense to normalize on the config options as defined in ConfigOptions of URL Loader already, which is the same shape that the underlaying URL Gen library uses (official Cloudinary SDK)
https://github.com/colbyfayock/cloudinary-util/blob/main/packages/url-loader/src/types/config.ts |
Is your feature request related to a problem? Please describe.
Allow CldVideoPlayer to take a config prop, much like CldImage, which can control advanced Cloudinary settings
This would then ideally inherit module-level configuration that's being set up via: #195
Describe the solution you'd like
The Next.js implementation currently passes through any valid option that's defined in the source types from cloudinary-util/types
https://github.com/cloudinary-community/next-cloudinary/blob/main/next-cloudinary/src/components/CldVideoPlayer/CldVideoPlayer.tsx#L44
https://github.com/colbyfayock/cloudinary-util/blob/main/packages/types/src/types/cloudinary-video-player.d.ts
It doesnt look liek the work was done yet to use the types from the new package: #186
By adding the same thing, this would allow any of the configuration options to "just work"
The text was updated successfully, but these errors were encountered: