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

feat: MSN wizz on nudge #655

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

jdesnoes
Copy link
Contributor

Description

This pull request introduces a fun and nostalgic feature by adding an MSN Wizz animation and sound effect when a user is nudged during a poker game. The changes include:

  • Playing a sound effect when a user is nudged.
  • Adding a shake animation to the body element to mimic the classic MSN Wizz effect.
  • Ensuring the sound and animation are triggered only if the nudged user is the current user.

Many thanks to @kesslerdev for the suggestion.

What type of PR is this? (check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation Update
  • 🎨 Style
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build
  • 🔁 CI
  • 📦 Chore (Release)
  • ⏩ Revert

Related Tickets & Documents

Screenshots/Recordings

Animation

Steps to QA

@StevenWeathers
Copy link
Owner

I'm willing to add a CSS animation however I don't think adding an mp3 sound is ideal for numerous reasons including security concerns and annoying people. If you want to remove the MP3 but keep the animation I'd be happy to accept the PR, otherwise I would suggest building your own fork with the feature for self hosting.

@jdesnoes
Copy link
Contributor Author

I'm willing to add a CSS animation however I don't think adding an mp3 sound is ideal for numerous reasons including security concerns and annoying people. If you want to remove the MP3 but keep the animation I'd be happy to accept the PR, otherwise I would suggest building your own fork with the feature for self hosting.

Hello @StevenWeathers, thanks for your feedback, it makes sense indeed.
What do you think about adding an application scoped configuration, disabled by default, that allows MP3 sound activation?
Do you think it makes sense? Otherwise, indeed, I could remove the MP3 sound and keep the CSS animation only.

@StevenWeathers
Copy link
Owner

I'm willing to add a CSS animation however I don't think adding an mp3 sound is ideal for numerous reasons including security concerns and annoying people. If you want to remove the MP3 but keep the animation I'd be happy to accept the PR, otherwise I would suggest building your own fork with the feature for self hosting.

Hello @StevenWeathers, thanks for your feedback, it makes sense indeed. What do you think about adding an application scoped configuration, disabled by default, that allows MP3 sound activation? Do you think it makes sense? Otherwise, indeed, I could remove the MP3 sound and keep the CSS animation only.

I think the idea of adding a config option makes sense however at this time I do not want to add sound files to the application build.

@jdesnoes
Copy link
Contributor Author

I'm willing to add a CSS animation however I don't think adding an mp3 sound is ideal for numerous reasons including security concerns and annoying people. If you want to remove the MP3 but keep the animation I'd be happy to accept the PR, otherwise I would suggest building your own fork with the feature for self hosting.

Hello @StevenWeathers, thanks for your feedback, it makes sense indeed. What do you think about adding an application scoped configuration, disabled by default, that allows MP3 sound activation? Do you think it makes sense? Otherwise, indeed, I could remove the MP3 sound and keep the CSS animation only.

I think the idea of adding a config option makes sense however at this time I do not want to add sound files to the application build.

Certainly, I understand.
I have updated the pull request to remove all elements related to the MP3 sound and include only the CSS animation.
Let me know if it looks good to you. Thank you.

@StevenWeathers StevenWeathers merged commit 8f066b1 into StevenWeathers:main Dec 20, 2024
9 checks passed
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.

2 participants