dkaoster / scrolly-video

Components for scroll-based (or other externally controlled) playback.
https://scrollyvideo.js.org/
MIT License
965 stars 39 forks source link

Suggestion: Use objectFit property instead of cover #85

Open lorenzfischer0 opened 8 months ago

lorenzfischer0 commented 8 months ago

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
  2. Add the new objectFit option where the other options are saved:

    this.objectFit = objectFit;
  3. 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;
  4. 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;
  5. 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'; }



6. 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.