scrolly-video icon indicating copy to clipboard operation
scrolly-video copied to clipboard

Suggestion: Use objectFit property instead of cover

Open lorenzfischer0 opened this issue 5 months ago • 0 comments

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.

lorenzfischer0 avatar Jan 25 '24 13:01 lorenzfischer0