search

justinmc / react-audio-player

A simple React wrapper on the HTML5 audio tag
MIT License
587 stars 103 forks source link

Added loop prop and spread op for custom props #26

Closed joeyfigaro closed 7 years ago

joeyfigaro commented 7 years ago

Also fixed some minor test issues with boolean prop values.

joeyfigaro commented 7 years ago

@justinmc had a chance to check on this?

justinmc commented 7 years ago

Sorry for the delay! Looks good to me. I intend to support every attribute that the audio tag supports, so we definitely want loop. I also hadn't thought about not being able to add arbitrary attributes like data- attributes, which I think your ...props handles nicely. Merging! 👍

justinmc commented 7 years ago

@joeyfigaro Are you blocked by needing me to release a new version for this change? I'll try to push v0.4.0 tonight including this change, but if you need it sooner I would reference this package with a SHA.

joeyfigaro commented 7 years ago

@justinmc Thanks for merging this, mate. I'll give the SHA approach a shot; we haven't had any luck attempting to depend on snapshots/github repos with yarn or npm.

joeyfigaro commented 7 years ago

@justinmc I can't manage to get it to work. Latest attempt used the following:

package.json

...
"react-audio-player": "git://github.com/justinmc/react-audio-player.git#f732fe12c9f6f0d39094e81c641fc39caf307c8a"
...

Can you ping me when you release 0.4.0?

jkauszler commented 7 years ago

Hey, @justinmc we (the Coffitivity community) are slightly inconvenience by the release, yes. If you could push for a release sooner, it would be greatly appreciated. Thanks, bud!

justinmc commented 7 years ago

@joeyfigaro @jkauszler v0.4.0 published and live on npm! Hope I didn't leave you guys stranded for too long.

joeyfigaro commented 7 years ago

@justinmc You're a legend! 💯

jkauszler commented 7 years ago

@justinmc Thank you, sir!

On Wed, Apr 12, 2017 at 8:33 AM, Joey Figaro notifications@github.com wrote:

@justinmc https://github.com/justinmc You're a legend! 💯

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/justinmc/react-audio-player/pull/26#issuecomment-293561832, or mute the thread https://github.com/notifications/unsubscribe-auth/ADKJnxqVDXebsE2dh_gMUpizF0NRG1Caks5rvMSGgaJpZM4Myyb2 .

--

JUSTIN KAUSZLER | Co-Founder, Head of Product | 757-232-5893

Please excuse my brevity: http://five.sentenc.es/

justinmc commented 7 years ago

@joeyfigaro @jkauszler The {..this.props} part of this PR was removed in the latest release in favor of explicitly supporting all the attributes of the native audio tag (see https://github.com/justinmc/react-audio-player/pull/32). There was a problem with props like onPlay being set on the audio tag and causing warnings. It looks to me like everything you guys needed should still be supported, but let me know if that's not the case and I'll fix it.