Skip to content

Add support for Bell messages#349

Merged
gujjwal00 merged 2 commits intogujjwal00:masterfrom
EtienneMiret:bell-msg
Dec 4, 2025
Merged

Add support for Bell messages#349
gujjwal00 merged 2 commits intogujjwal00:masterfrom
EtienneMiret:bell-msg

Conversation

@EtienneMiret
Copy link
Copy Markdown
Contributor

Closes #347.

A new setting allow users to choose a specific tone to play on Bell messages. The new tone is also played when the user changes the selected tone.

As explained in the setting help message, the volume of the played tone is set from the system volume for notifications.

I used ToneGenerator instead of playSoundEffect() because the latter is intended for feedbacks on user gestures, which doesn't seem the case here.

@gujjwal00
Copy link
Copy Markdown
Owner

Thanks @EtienneMiret .

I used ToneGenerator instead of playSoundEffect() because the latter is intended for feedbacks on user gestures, which doesn't seem the case here.

Yeah, makes sense. I didn't know about ToneGenerator.

Only feedback I have is regarding the setting. Do we really need to make the tone configurable? I thought a simple ON/OFF Switch with a default bell sound would suffice.

Comment on lines +88 to +96
override fun onAttach(context: Context) {
super.onAttach(context)
val prefs = AppPreferences(context)
listener = OnSharedPreferenceChangeListener { _, key ->
if (key == "bell") {
prefs.ui.bell?.let { Tones.notify(it) }
}
}
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You could simplify this by using setOnPreferenceChangeListener() inside onCreate(). You wouldn't have to fiddle with OnSharedPreferenceChangeListener at all.

@EtienneMiret
Copy link
Copy Markdown
Contributor Author

Only feedback I have is regarding the setting. Do we really need to make the tone configurable? I thought a simple ON/OFF Switch with a default bell sound would suffice.

I thought it was nice to let user choose, but if you feel it overloads the settings UI, I just pushed a new version with a simpler on/off switch.

You could simplify this by using setOnPreferenceChangeListener() inside onCreate(). You wouldn't have to fiddle with OnSharedPreferenceChangeListener at all.

Indeed. I did so because I wanted a Context in order to access AppPreferences. Anyway, with a simple on/off switch, I believe there is no need to provide that much feedback to the user, so I removed that part.

@gujjwal00 gujjwal00 merged commit 7e15b0e into gujjwal00:master Dec 4, 2025
1 check passed
@gujjwal00
Copy link
Copy Markdown
Owner

Hi @EtienneMiret , an update regarding this:

I have been using it for approximately a month now. More often than not, multiple bell messages are triggered quickly on server (e.g. when holding down Backspace key to erase a command in terminal). And sometimes the bell message generated during UI navigation feels a bit unexpected. In both of these cases, I found the loudness of the tone to be quite irritating/jarring.

So I am reducing the tone volume for bell messages.

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.

New feature: add support for Bell messages

2 participants