Add seekable method to Tech and increase supported version range

[stevendesu]

I initially just needed to resolve #41, but I figured I'd go ahead and tweak the package.json while I was at it. Sorry for the confusing triple-commit (you can squash-merge it if you prefer). Since we were forking the repo I made a few changes for convenience for our sake and reverted them prior to the pull request.

[jthomerson]

Also, you need to run grunt standards && npm test to find errors there. You'll see here that the build failed (https://travis-ci.org/silvermine/videojs-chromecast/jobs/498762630#L501) because of a linting issue.

[stevendesu]

git reset origin/master doesn't do anything (since there are no changes in my working tree), but I can squash the three commits into one which should clean it up. I know the process, but can never remember the specific commands - so I'll have to Google them quickly.

[jthomerson]

@stevendesu I just realized why that won't work for you ... you forked the repo and used the master branch, so your origin/master is your repo. So, reset won't work there because you're resetting to HEAD.

Instead, you could add the official origin and reset to it, e.g.

git remote add silvermine https://github.com/silvermine/videojs-chromecast.git
git fetch silvermine
git reset silvermine/master

Or, you could have used a feature branch (the recommended flow) instead of master.

Anyway, thanks for working with us. Just thought maybe that explanation would be helpful for the future.

[stevendesu]

@jthomerson Using a squash commit I cleaned up the history for you. It may not be the most elegant solution (feature branches would probably be smarter), but there's only a single commit now.

As for grunt standards, there is an issue being thrown by eslint and I may need some direction for the preferred method of solving it. To return a TimeRange in the seekable method I needed access to the VideoJS API. Because this is stored on the global window object I was able to just type videojs.createTimeRange() and it worked when I tested it, but videojs is technically not declared in this scope, nor is it passed in as a parameter anywhere.

Does VideoJS make its API available to all components (perhaps through this.videojs) or is there a preferred way to pass it in?

[stevendesu]

Sure enough, this.videojs is a thing :smile: I just inspected the player in the debugger and checked it out. Since it worked I replaced videojs.createTimeRange with this.videojs.createTimeRange then range:

grunt standards && npm test

No errors seemed to be thrown. One new rebase and force-push and we're looking good.

[jthomerson]

@stevendesu thanks. Not sure if https://github.com/silvermine/videojs-chromecast/blob/52c3b840f5457d28cf2a29e068e7c8e42fa8d04f/src/js/tech/ChromecastTech.js#L38 provides what you need?

@yokuze can you look at the questions @stevendesu asks about the exposure of videojs?

[jthomerson]

@yokuze cancel that.

[jthomerson]

But, assigning the rest of the (actual code) review to @yokuze

[yokuze]

@jthomerson sure thing. Hi @stevendesu, thanks again for the PR! Taking a look now.

[yokuze]

@stevendesu just finished the initial review. Thanks!

[stevendesu]

@yokuze Thank you for the thorough comments. I've made all of the requested changes and updated the PR. I also added one additional "Refs" to the commit message: #32

Reading through the other issues, this one mentioned that the reason seeking failed was the lack of seekable method. Now that a seekable method is implemented, seeking works on VOD.

[stevendesu]

@yokuze Two typos corrected (I noticed since GitHub is a single-page app now, I had to refresh the page to see the changes take effect in the "Files changed" tab) and commit message updated

[jthomerson]

Thanks @stevendesu. I just released @silvermine/videojs-chromecast@1.1.2 with your fix.