Review Request 127926: KNotifyConfigWidget: add disableAllSounds() method

[David Faure]

> On May 18, 2016, 2:16 p.m., Martin Klapetek wrote:
> > A better approach I think would be to have a simple checkbox, store
> > that in a config and then have KNotification/NotifyByAudio simply
> > do nothing if that config/value is present.
> > 
> > Or even have KNotificationManager::notify(..) skip audio action.

Sounds complementary to this approach, actually.

The approach in this patch works better IMHO for disabling all sounds for -one- event source.
A checkbox wouldn't make sense for that purpose (contradictory with the info in the listview), but a global checkbox "disable all sound notifications" would make sense indeed, for those who prefer that.

The people in the bug report had both use cases.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127926/#review95588
-----------------------------------------------------------


On May 15, 2016, 10:53 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127926/
> -----------------------------------------------------------
> 
> (Updated May 15, 2016, 10:53 a.m.)
> 
> 
> Review request for KDE Frameworks, David Edmundson and Olivier Goffart.
> 
> 
> Bugs: 157272
>     https://bugs.kde.org/show_bug.cgi?id=157272
> 
> 
> Repository: knotifyconfig
> 
> 
> Description
> -------
> 
> This saves a lot of clicking compared to selecting each event
> in the list and unchecking the "Play sound" checkbox, something
> I have to do any time I set up a new computer (for me or around the office).
> 
> CCBUG: 157272
> 
> 
> Diffs
> -----
> 
>   src/knotifyconfigwidget.h bf122bd9e30982f0fa0d022d04aeadee4fb181dc 
>   src/knotifyconfigwidget.cpp 06125ea9da565cbc14c8afa1155c723605b42da6 
>   src/knotifyeventlist.h b6bd43ed40f9b2a18d41300760a6c01dca52907d 
>   src/knotifyeventlist.cpp 6913148869a7be6b267668e44888f1a432b2396b 
> 
> Diff: https://git.reviewboard.kde.org/r/127926/diff/
> 
> 
> Testing
> -------
> 
> I'm calling this method from a button in the KCM: http://www.davidfaure.fr/2016/kcmnotify.diff
> 
> My initial idea was a "disable all sounds for all event sources" button, but the underlying classes don't make this easy to support (I'd have to literally set every item as current in the combo, leading to a strange user-visible automation happening). Disabling all sounds for the current event source (app) is already quite a time saver.
> Plus this way the change isn't saved yet, it can still be cancelled by pressing Cancel, like any other change -except- selecting another app.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160518/3d3945e0/attachment.html>