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: web push notification when someone enters the lobby #7525

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

acollign
Copy link
Contributor

This PR implements #7518 which consists of notifying players when someone enters the lobby.

Copy link
Collaborator

@NoahTheDuke NoahTheDuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't correct, I'm sorry to say:

  1. .requestPermission is a promise, so calling it without a .then won't actually wait to get permission.
  2. This doesn't handle if the browser doesn't support notifications, so it'll fail and not play any sounds in that case.
  3. It doesn't support if the user rejects the permissions request.
  4. In Safari and Firefox, .requestPermissions can only be called in a user event, like clicking. So you'd need to move the permissions check to something like the create-game and/or join-game callbacks.
  5. This will pop a notification if someone joins your game or if you join someone else's game because we fire :lobby/notification in both cases. I don't think it's good UX to get a notification when deliberately clicking on a game. You'll need to figure out a way to only alert the user when someone else joins the game.

@acollign
Copy link
Contributor Author

This isn't correct, I'm sorry to say:

No problem @NoahTheDuke. Thank you very much for your feedback. I will work on a different solution.

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