nkoehler / mat-video

:tv: mat-video is an Angular 8/9+ video player using Material!
https://nkoehler.github.io/mat-video/
MIT License
91 stars 47 forks source link

Adding a frame by frame option on the player #21

Closed thibserot closed 5 years ago

thibserot commented 5 years ago

What kind of change does this PR introduce? (check one with "x")

What is the current behavior? (You can also link to an open issue here)

Only Play/Pause button available

What is the new behavior?

You can now enable via showFrameByFrame additional controls that will allow you to do a frame-by-frame play (+1,+5, -1, -5)

Does this PR introduce a breaking change? (check one with "x")

Default values have been chosen so that it doesn't change current behavior

If this PR contains a breaking change, please describe the impact and migration path for existing applications:

N/A

Other information:

nkoehler commented 5 years ago

I have removed the tooltips for consistency with the rest of the controls, and tslinted the code.

I was unsure about the new fps variable, because not many people will know the exact fps of their video. But even if it is skipping 6/7/10 frames, it's not really a big issue.

Other than that the changes look fine. Should be available in the next release. Thank you for the contribution.

thibserot commented 5 years ago

I was hesitating with adding the tooltip for all control...The extra cost of importing this module is very low since you are already relying on material and it helps with readability. but if you prefer a tooltip free module I don't mind. And if you wish me to add an extra contribution to add tooltip on all control I'll do it with pleasure!

Thanks for merging so fast the PR :)

When do you think you'll be performing a release?

nkoehler commented 5 years ago

I'm going to leave tooltips out for now, as on other projects I've noticed that with 10+ tooltips there is a performance hit with all the event listeners that get registered.

Not a problem, I was working on the project today anyways. I am hoping to cut a new release (with Angular 8 support as well) later today if all goes well.