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

Suggestion: Use objectFit property instead of cover #85

Open
lorenzfischer0 opened this issue Jan 25, 2024 · 0 comments
Open

Suggestion: Use objectFit property instead of cover #85

lorenzfischer0 opened this issue Jan 25, 2024 · 0 comments

Comments

@lorenzfischer0
Copy link

lorenzfischer0 commented Jan 25, 2024

Preface

This is a lovely library and I want to start of with a big thanks to the authors and contributors! 🧡


Issue

The cover property doesn't quite work as expected for me. When set to false the video won't cover the container (as expected), but once it's decoded and rendered via the canvas it will cover it.


Proposed Solution

Use the objectFit property for both the video and the canvas and expose it. This way users can set it to "contain", "cover", "fill", etc. without the need to implement custom scaling functions like setCoverStyle.

  1. Remove the cover property (or set it to false) and add an objectFit property in the constructor. Default the object fit to "cover" to replicate the previous default behaviour:
objectFit = "cover", // How the video and canvas should be fit inside the container
  1. Add the new objectFit option where the other options are saved:
this.objectFit = objectFit;
  1. Set the videos heigth, width and objectFit, where it is created:
this.video.style.width = '100%';
this.video.style.height = '100%';
this.video.style.objectFit = this.objectFit;
  1. Do the same for the canvas inside the decodeVideo function:
this.canvas.style.width = '100%';
this.canvas.style.height = '100%';
this.canvas.style.objectFit = this.objectFit;
  1. Remove the folowing resizing logic from paintCanvasFunction
const { width, height } = this.container.getBoundingClientRect();

if (width / height > currFrame.width / currFrame.height) {
	this.canvas.style.width = '100%';
	this.canvas.style.height = 'auto';
} else {
	this.canvas.style.height = '100%';
	this.canvas.style.width = 'auto';
}
  1. Clean up, remove setCoverStyle and all the places here cover was used.

Considerations

Browser support should be good for object-fit according to caniuse.com. I haven't found any negative side effects so far, but I might be overlooking something? You could also keep the cover prop for backwards-compatibility and set the newly introduced objectFit property accordingly. Also should consider to add a objectPosition prop as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant