tjenkinson / clappr-thumbnails-plugin

A plugin for clappr which will display thumbnails when hovering over the scrub bar. Thumbnails can either be individual images or a sprite sheet.
https://tjenkinson.github.io/clappr-thumbnails-plugin/demo/
MIT License
44 stars 14 forks source link

Is this plugin invalid? #89

Closed fztx closed 5 years ago

fztx commented 5 years ago

Is this plugin invalid? No matter how I debug, the thumbnail will not be displayed on the player. I am sure that there are no errors in my steps. Please check if the plugin fails. Please mark it out.

phloxic commented 5 years ago

If you're using latest clappr v0.3.0: seems this plugin is affected by the potential problems there. Short-term try downgrading to clappr v0.2.x - at least that works for me[tm].

fztx commented 5 years ago

If you're using latest clappr v0.3.0: seems this plugin is affected by the potential problems there. Short-term try downgrading to clappr v0.2.x - at least that works for me[tm].

thank you for your reply! I understand

phloxic commented 5 years ago

@fuzetianxia - imho this is still a bug. Either the plugin should be made to work with v0.3.x core or vice versa ;-)

tjenkinson commented 5 years ago

Ye we should try and fix this :)

What's the error?

phloxic commented 5 years ago

It just fails silently - as far as noise is concerned backwards compatibility works ;-) I played around a bit by replacing some stuff according to the release notes, but so far to no avail. Sorry, not familiar with the code.

brandonkal commented 5 years ago

@tjenkinson Having the browser pause on exceptions, I get this to start, all occuring in clappr.min.js @0.3.1.

TypeError: Argument 1 of Window.getComputedStyle is not an object.
ReferenceError: ActiveXObject is not defined
unknown
unknown
SyntaxError: '*,:x' is not a valid selector (jQuery)

These are all caught exceptions.

phloxic commented 5 years ago

This could be related: https://github.com/clappr/clappr/issues/1754

sapolio commented 5 years ago

This could be related: clappr/clappr#1754

Indeed! My plugin is largely based on clappr-thumbnails-plugin. It works just well with clappr 0.2. The problem is with event handlers when extending the core plugin in clappr 0.3. This extension does not work:

export default class dvr extends DVRControls {
  get something () { return 'something' }
}

(Not sure if other types of plugins are also affected.)

sapolio commented 5 years ago

the problem is solved by adding such a handler:

this.listenTo(this.core, Clappr.Events.CORE_ACTIVE_CONTAINER_CHANGED, () => {
    this.stopListening()
    this.bindEvents()
})
tjenkinson commented 5 years ago

Feel free to open a pr :)

On 30 Jan 2019, at 08:53, Aleksandr Shemetillo notifications@github.com<mailto:notifications@github.com> wrote:

the problem is solved by adding such a handler:

this.listenTo(this.core, Clappr.Events.CORE_ACTIVE_CONTAINER_CHANGED, () => { this.stopListening() this.bindEvents() })

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/tjenkinson/clappr-thumbnails-plugin/issues/89#issuecomment-458845920, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ADG-We1Xs2pUd-Eqql9N3xCikOJ8OL3Eks5vIU9ogaJpZM4YwkX9.

sapolio commented 5 years ago

@tjenkinson Ok! ) As soon as i can.

sapolio commented 5 years ago

90

Don't know if It's needed )

phloxic commented 5 years ago

@sapolio - does the trick for me[tm].

sapolio commented 5 years ago

@blacktrash notice the changes please I fixed a couple of my missteps

I'm new to this, sorry :sweat_smile:

phloxic commented 5 years ago

@sapolio - I was merely focusing on the actual fix at https://github.com/tjenkinson/clappr-thumbnails-plugin/pull/90/commits/4f3517dea9e6117c58ce21b2406368cd78509d7c#diff-1fdf421c05c1140f6d71444ea2b27638

tjenkinson commented 5 years ago

Thanks. I just released a new version with ^