search

cladera / videojs-offset

VideoJS plugin for play a segment of a video.
MIT License
64 stars 43 forks source link

Fix currentTime function when offset is not used #40

Closed DanielWare closed 5 years ago

DanielWare commented 5 years ago

Bug fix for #39

Description

In cases where videojs-offset is loaded but no used, we should fall back to videojs implementation of currentTime. specifically we check to see if _offsetStart is defined before using videojs-offset logic.

Specific Changes proposed

There are 2 code paths for currentTime. In both cases (seconds argument is defined/undefined) we check if this._offsetStart is defined, and if not, we call the super class version of the function.

DanielWare commented 5 years ago

I also updated the implementation of duration. It may have been where this error began.

cladera commented 5 years ago

@DanielWare can you rebase and push again?

DanielWare commented 5 years ago

@cladera Sorry, I'm a bit new to this whole pull request thing. What does that mean?

cladera commented 5 years ago

Hi @DanielWare I just published a new version which upgrades the videojs-plugin generator because the CI was not passing in any PR.

You need to rebase your branch onto master and then push your branch again.

DanielWare commented 5 years ago

@cladera Thanks! I think I have merged your changes and hopefully should be good now.

DanielWare commented 5 years ago

@cladera looks like we need to add shx to the dev dependencies to fix the build.

cladera commented 5 years ago

@DanielWare No, the build was ok. The problem is your branch did not pass the lint check:

   99:1  error  Expected indentation of 12 spaces but found 10  indent
  106:1  error  Expected indentation of 10 spaces but found 8   indent