----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110389/#review32519 -----------------------------------------------------------
Ship it! code looks fine, the feature i think will be welcome by many. my only concern is how usable this will be on touch. (probably not very.) for Plasma Active 4 this is not a problem as it is using 4.10. We'll need to revisit this in later versions. (Probably with some use of the platformcomponents/ folder) - Aaron J. Seigo On May 12, 2013, 2:10 p.m., Kai Uwe Broulik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/110389/ > ----------------------------------------------------------- > > (Updated May 12, 2013, 2:10 p.m.) > > > Review request for Plasma and KDE Usability. > > > Description > ------- > > Depends on Review 110407 > > This patch adds a configure button to notifications that, when clicked, open > the KNotify settings for the app that emitted the notification. > > This makes it much easier to turn off a notification you do not want and you > have no clue where to turn it off (no novice user would know that you can > actually turn these off, not even speaking of where) > > I asked Olivier Goffart for permission to use his notifyrc lookup code that > is licensed GPLv2 in this LGPLv2 code. We need to do this because according > to the specs the application name passed to the notification system is "[…] > the optional name of the application sending the notification. This should be > the application's formal name[…]. An example would be "FredApp E-Mail > Client," rather than "fredapp-email-client." > So KNotify had to be modified to not only expose "KDE Power Management > System" to the dataengine but also its internal name "powerdevil". > > For illustration how it works, see here: > http://666kb.com/i/cdzg50m1huwc1k751.jpg ;-) > > > Diffs > ----- > > plasma/generic/applets/notifications/contents/ui/LastNotificationPopup.qml > df63ae6 > > plasma/generic/applets/notifications/contents/ui/NotificationDelegate/NotificationDelegate.qml > 64d9298 > plasma/generic/applets/notifications/contents/ui/Notifications.qml c5be0a3 > plasma/generic/dataengines/notifications/CMakeLists.txt e8d66e0 > plasma/generic/dataengines/notifications/notificationaction.cpp 390cc16 > plasma/generic/dataengines/notifications/notifications.operations bb5d0c6 > plasma/generic/dataengines/notifications/notificationsengine.h 4644c8e > plasma/generic/dataengines/notifications/notificationsengine.cpp 77f5db2 > > Diff: http://git.reviewboard.kde.org/r/110389/diff/ > > > Testing > ------- > > Works, in both the popup and history. Tested with: > - Amarok: Shows configure button properly > - PowerDevil: Shows configure button properly > - Konsole: Shows configure button properly > - Free Space Notifier: Shows configure button properly > - KDialog: Does not show configure button, as expected > - QDBusViewer to issue a notification using the FDO DBus iface: Does not > show configure button, as expected (it does, when your passed appName happens > to match an application, which is rather nice, though) > > > File Attachments > ---------------- > > A few notifications with and without configure option > > http://git.reviewboard.kde.org/media/uploaded/files/2013/05/12/notificationconfigure1.png > > > Thanks, > > Kai Uwe Broulik > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
