Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML <audio> volume attribute #1143

Closed
jacobmischka opened this issue Jan 31, 2018 · 9 comments
Closed

HTML <audio> volume attribute #1143

jacobmischka opened this issue Jan 31, 2018 · 9 comments

Comments

@jacobmischka
Copy link
Contributor

jacobmischka commented Jan 31, 2018

I know that volume is not one of the documented bind properties for <audio>, but I'm surprised that setting the attribute doesn't work at all. This can be worked around with refs, but it would be handy if svelte did that for us when changing volume.

<audio src="{{src}}" volume="{{volume}} />

I imagine this shouldn't be terribly difficult, I'll try to look at this as soon as I find the time.

Thank you!

@Conduitry
Copy link
Member

Conduitry commented Feb 1, 2018

Volume attributes currently compile the same way any regular attribute does:

setAttribute(audio, "volume", state.volume);

Is there some corresponding property that needs to be used instead, that was missed here? If there is, it doesn't look like it's on MDN.

@jacobmischka
Copy link
Contributor Author

I think the element just wants to have the property set directly (audio.volume = 0.66), it doesn't seem like it responds to dom element attribute changes.

@PaulBGD
Copy link
Member

PaulBGD commented Feb 2, 2018

Looks like it should support this?: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/audio#attr-volume

edit: removed some stupid stuff

@jacobmischka
Copy link
Contributor Author

jacobmischka commented Feb 2, 2018

That page doesn't mention anything about whether the volume should update with the attribute updating, merely that you can set the initial state using it.

I tested this REPL in Firefox Nightly and Chrome, both behave the same way: https://svelte.technology/repl?version=1.54.0&gist=718ff93aa528f49030c68a988108a0c2

image

@jacobmischka
Copy link
Contributor Author

Here's an example with audio instead of relying on that property value: https://svelte.technology/repl?version=1.54.0&gist=7cad943bb07c698cb02d5e9efbe74621

@jacobmischka
Copy link
Contributor Author

jacobmischka commented Feb 2, 2018

The fix to this is probably to just implement bind for the volume property for video and audio elements, I'll try to do that soon!

Edit: Or yes, just use the volume property instead of setting the dom attribute, sorry for missing you mentioning that earlier!

@mrkishi
Copy link
Member

mrkishi commented Feb 2, 2018

The <audio> tag has no volume content attribute at all.

I'm surprised the MDN page doesn't show the specific types of the attributes. volume is an IDL attribute (available via js), which is not the same as content attributes (those you can set on html tags). MDN on the difference.

See the specification and the repl without bindings.

@jacobmischka
Copy link
Contributor Author

Oh interesting, I guess I misunderstood the MDN page. Thanks for clearing that up!

@mrkishi
Copy link
Member

mrkishi commented Feb 2, 2018

Not much of a misunderstanding when they completely omitted it from the page! 😄

jacobmischka added a commit to jacobmischka/svelte that referenced this issue Feb 3, 2018
Rich-Harris pushed a commit that referenced this issue Feb 3, 2018
* Add audio/video volume binding

Fixes #1143

* Update test and add volumechange event

* Set volume on initial update

* Update test after setting volume initially

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

No branches or pull requests

5 participants