Repository navigation
Toggle the sound notifications - #1536
Conversation
|
🙌 Thanks for opening this pull request! You're awesome. |
| <Checkbox | ||
| className='control' | ||
| checked={this.props.state.saved.prefs.soundNotifications} | ||
| label={'Activate the notification sounds'} |
There was a problem hiding this comment.
Nitpick: I would prefer 'Enable notification sounds'.
But since it is both interaction sounds and notification sounds, maybe it should simply be 'Enable sounds'?
| {this.setDefaultAppButton()} | ||
| </PreferencesSection> | ||
| {this.setStartupSection()} | ||
| <PreferencesSection title='Miscellaneous'> |
There was a problem hiding this comment.
I think we should group the startup and sound options under a 'General' section instead.
There was a problem hiding this comment.
You're right, looks cleaner to me too 👍
|
@adriantombu Thanks for the changes! I took the liberty to add some migration handling to your branch as well. Based on the previous feedback I also think it's fine to make this an option. |
trujamal
left a comment
trujamal
left a comment
There was a problem hiding this comment.
Changes do not impede user functionality, and pass test.
|
🎉 Congrats on getting your first pull request landed! |
This PR is related to the #1067 issue
As the notificaiton sounds seem a bit controversial, I guess the best way to content everybody would be to allow people to chose what they want, hence this little checkbox to toggle the sounds 😃
Personnally, I'm like @rjmill : sometime I need absolute silence, and sometime I need the notifications.
Anyways, I'm open for feedback 👍