chrisguttandin / timing-object

An implementation of the timing object specification.
MIT License
37 stars 1 forks source link

timeupdate event not triggering #227

Closed bmsan closed 3 years ago

bmsan commented 3 years ago

Hello

I have tried unsucesfully to use the timeupdate event, when the timingObject has a positive velocity:

timingObject.addEventListener('timeupdate', (e) => console.log('time update : ' + timingObject.query().position)) I never get any hit on that callback.

As a workaround I am using setInterval & querying the timingObject.

timing-object version: "version": "3.1.30", "resolved": "https://registry.npmjs.org/timing-object/-/timing-object-3.1.30.tgz", "integrity": "sha512-3Qyi1otLbbjGKYlsGmcrxEJeuzM7OCNwZ45PqY2OjmgTGudh3knvPTxsPiYTkbbE4cc1Sup05sAaUSoO8r//+w==",

chrisguttandin commented 3 years ago

Hi @bmsan,

the timeupdate event is not implement. I mentioned this in the readme but it can probably be overlooked very easily.

226 was already about the same problem.

I will update the readme to make it easier to spot.

bmsan commented 3 years ago

Hi @chrisguttandin, You're right! I missed it somehow, and it's pretty easy to spot...

One question though. Would it make sense from your point of view to check if the targeted event exists and if not to issue an warning? I am not thinking here necessarily only at timeupdate, but maybe the user misspells an existing event.

Thanks!

chrisguttandin commented 3 years ago

I'm not sure. The event handling is inherited from the EventTarget. It does allow registering events of any type. It also allows dispatching events of any type.

It would be perfectly valid (but maybe not the best idea ever) to do something like this.

timingObject.addEventListener('timeupdate', () => {
    // ...
});

timingObject.dispatchEvent(new Event('timeupdate'));

Since the EventTarget is so flexible it's not possible to know which kind of events it may fire in the future.

chrisguttandin commented 3 years ago

Hi @bmsan, I updated the readme to mention the missing timeupdate event more explicitly. Thanks for making me aware of that flaw.