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

Added app.beep() to play the default system notification sound. #2018

Merged
merged 5 commits into from
Jul 2, 2023

Conversation

bruno-rino
Copy link
Contributor

@bruno-rino bruno-rino commented Jun 29, 2023

app.beep() is a new method that will play the default system notification sound.

Only tested for macOS

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

API References:

@freakboy3742
Copy link
Member

So... this seems like a reasonable implementation of a fairly straightforward feature, but I have to ask... Why?

@bruno-rino
Copy link
Contributor Author

I'm working on a minimal example for a DocumentApp. It doesn't support opening the same file in separate windows, so if I try to open a file a second time, I want to give some non intrusive feedback. I think beep is the standard way to do that.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

I'm not sure I agree that a beep is the right error output for something like loading a document twice - or, if it is, it's something that should be done in conjunction with other forms of feedback (like a dialog). I would have thought this was something that the operating system would have enforced with it's own APIs, rather than being something that an end-user needed to code explicitly.

However, I can't argue that the API exists on pretty much every platform, and it's a fairly small API, so I guess it can't hurt to add it.

@freakboy3742 freakboy3742 merged commit 9dc268b into beeware:main Jul 2, 2023
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