Skip to content
This repository has been archived by the owner on Mar 26, 2021. It is now read-only.

Add currentTime to audio ontimeupdate event #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

scottire
Copy link

@scottire scottire commented Mar 7, 2021

I want to display the currentTime alongside an audio element (and I also just wanted to play around with IDOM).

From the now defunct gitter:

Ryan Morshead @rmorshea Mar 06 19:44
@scottire there is no built-in way to query attributes of client-side elements. All data communicated from the client to the server is done in the form of events. With that said, would it be sufficient for onTimeUpdate events to send back the value of currentTime? This would require an addition to IDOM's client-side event serializer that should be pretty straightforward. Let me know if you'd like any help in making that contribution. If you don't have the time, create an issue, and I'll try to get to it when I can.

The first commit is largely to get the ball rolling on this so I can add any changes / tests / docs.

One question so I can test this, what's your typical workflow for developing this IDOM javascript dependency?
Do you locally clone this and have another local idom repo and then npm install ../../../../../idom-client-react from within idom/src/idom/client/app?

@rmorshea
Copy link
Member

rmorshea commented Mar 7, 2021

Thanks for this! I'll try to take a look at this later today.

As for the workflow, I'm honestly now quite sure. I've been flip-flopping back on forth on whether this should even be its own repo or not. If this does remain on its own, then ideally there'd be a test suite that would give you confidence that it'd work without having to do an integration test like that. I just haven't gotten around to creating that though. In the absence of such a test suite the workflow you suggested is probably the only option unfortunately.

@rmorshea
Copy link
Member

rmorshea commented Mar 8, 2021

I think this might actually require something a little less straightforward since currentTime is an attribute on the event.target and might not actually be present for all HTML elements that can produce media events.

The solution would probably be a little similar to what I had to do for event.target.value on elements like <input/> etc. In this case though, I'm not sure the necessary logic belongs in the same place (the logic for value might not either for that matter).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants