Make sound opt-in backwards compatible

[nhocki]

Audio was made opt-in in #112 but this breaks the current default by
disabling audio. This commit changes it back to the current default (on)
but allows people to turn it off by adding the data-lock-audio
attribute.

I personally think that audio should be opt-in, but opening this in case you
don't want to introduce the breaking change.

[leastbad]

Thanks for the PR, Nicolas!

I need you to help me unpack this as I'm finding it a bit difficult to parse. (I'm just slow, don't mind me.)

The currently shipping version 4.5.0 has audio on, period - there's actually no way to shut it off, which is what led to #112 (which was a response to issue #111).

So what I am getting from this PR is that you actually prefer the current behaviour but you're such a pragmatist that you'd rather we maintain backwards compatibility than do things the way that makes you happiest? If I have that right, I'm impressed.

[nhocki]

Hey @leastbad , I do think audio should be off by default, but by doing that you might want to bump to v5 and that might impact more people (even though the upgrade path would be very easy for anyone that wants to keep audio on by default).

Opened the PR in case you wanted to keep v4 around for longer but feel free to close it!

[leastbad]

Thank-you for clarifying! This really helps.

This could go either way. We have a large changeset coming and it might actually make sense to do a v5 release anyhow.

[braindeaf]

Is this going to be published. in IE11 play() is returning null rather than a Promise so I want to be able to turn this off.

document.audio
  .play()
  .then(() => {})
  .catch(() => {})

[leastbad]

@braindeaf thanks for the heads up on this issue. Technically IE11 isn't a supported browser, but it goes without saying that we don't want to make problems for anyone and the good news is that there is absolutely a viable workaround that is already in master branch and will be part of the next release when it comes out.

All you have to do is upgrade your cable_ready gem and npm packages to point to master, and then don't add the data-unlock-audio attribute (eg. do nothing) and the audio will be disabled. I give instructions for SR here, but just swap library names:

https://docs.stimulusreflex.com/hello-world/setup#running-edge

Now, this PR is proposing that we either make opt-out optional. I'm still deciding how to play this hand, but I promise that if this behaviour changes significantly, I will reach out to you with a heads-up so that you can take whatever steps you need to.

The other possibility is that I am seriously tempted to pull the play_audio from the library and create a gallery of top custom operations in the docs. It's not because I don't think it belongs or is super useful, but the reality is that audio is hard, requires a bit of a hack, and as your IE11 issues demonstrate, this solution is always going to be fussy. Since we want CR to be a power-tool everyone reaches for, there's a strong argument to be made for being pragmatic and simplifying the core library.

At any rate, it's worth-while to "watch" this PR for developments.

[braindeaf]

Thanks for the comprehensive response. And I appreciate your attention on this issue. We've found that ditching IE11 isn't very easy, just turning it off would alienate 90% of customers who have no choice about what browsers are installed on their side. If I try to use IE11, Windows jumps up and down to get me to use Edge, but our clients don't have this and certain industries don't have any incentive to upgrade :(

In the meantime I've pointed our package.json to the master branch, which I hadn't realised was possible and it looks good to me :) All the best.

[leastbad]

Just to keep @nhocki and @braindeaf up to date as promised, here's what's coming:

CableReady 5.0.0 will come out in preview form soon. It's an awesome release.

We're taking play_sound out of core in #124, but introducing a slick and easy way to import it as seen in cableready/audio_operations. We think that this is the most productive course of action, since play_sound appears to be genuinely disruptive if you're supporting older browsers.

The great thing about the plugin approach is that not only does it provide a future-proof solution, but if you don't like my audio implementation you can fork it or make your own.