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

add error sound #63

Merged
merged 1 commit into from Apr 18, 2019
Merged

add error sound #63

merged 1 commit into from Apr 18, 2019

Conversation

ShizukuIchi
Copy link
Owner

No description provided.

@vercel
Copy link

vercel bot commented Apr 17, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://winxp-git-feature-error-sound.shizukuichi.now.sh

@ShizukuIchi
Copy link
Owner Author

#62

@panmona
Copy link

panmona commented Apr 17, 2019

This is the exact modification I imagined, it fixes the concerns from #62.
I'm pretty sure it won't work in Safari and other browsers (mobile chrome) that restrict playing of sounds to be a result of a user interaction. I think the code modifications needed for that would be a lot bigger though and as this is just an enhancement I think it's good enough. (but I don't know this code base really well)

TL,DR: This is great!

@VocalFan
Copy link
Contributor

Well, don't users have to use the website in order to activate the error sound? So isn't the website interacted with?

@panmona
Copy link

panmona commented Apr 17, 2019

@ghostslayer989 The restriction is that it must be called from an onClick Handler.

@ShizukuIchi ShizukuIchi merged commit 52067ef into master Apr 18, 2019
@ShizukuIchi ShizukuIchi mentioned this pull request Apr 18, 2019
@panmona
Copy link

panmona commented Apr 18, 2019

Small follow up to illustrate what I mean:
Unhandled Promise Rejection: NotAllowedError: The request is not allowed by the user agent or the platform in the current context, possibly because the user denied permission. (Safari)
But as I said before this is fine.

@ShizukuIchi ShizukuIchi deleted the feature/error-sound branch April 19, 2019 15:48
@panmona
Copy link

panmona commented Apr 30, 2019

How difficult would it be though to move the sound handling directly into the onClick handler?

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

Successfully merging this pull request may close these issues.

None yet

3 participants