[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request 110389: Add "Configure" button to notifications that opens KNotify settings for t
From:       "Commit Hook" <null () kde ! org>
Date:       2013-05-14 21:34:01
Message-ID: 20130514213401.5630.31073 () vidsolbach ! de
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110389/
-----------------------------------------------------------

(Updated May 14, 2013, 9:33 p.m.)


Status
------

This change has been marked as submitted.


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


[Attachment #5 (text/html)]

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;">  <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/110389/">http://git.reviewboard.kde.org/r/110389/</a>
  </td>
    </tr>
   </table>
   <br />



<table bgcolor="#e0e0e0" width="100%" cellpadding="8" style="border: 1px gray \
solid;">  <tr>
  <td>
   <h1 style="margin-right: 0.2em; padding: 0; font-size: 10pt;">This change has been \
marked as submitted.</h1>  </td>
 </tr>
</table>
<br />


<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;">  <tr>
  <td>

<div>Review request for Plasma and KDE Usability.</div>
<div>By Kai Uwe Broulik.</div>


<p style="color: grey;"><i>Updated May 14, 2013, 9:33 p.m.</i></p>






<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">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 &quot;[…] the optional \
name of the application sending the notification. This should be the \
application&#39;s formal name[…]. An example would be &quot;FredApp E-Mail \
Client,&quot; rather than &quot;fredapp-email-client.&quot; So KNotify had to be \
modified to not only expose &quot;KDE Power Management System&quot; to the dataengine \
but also its internal name &quot;powerdevil&quot;.

For illustration how it works, see here: http://666kb.com/i/cdzg50m1huwc1k751.jpg \
;-)</pre>  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
                break-word;">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)</pre>  </td>
 </tr>
</table>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>plasma/generic/applets/notifications/contents/ui/LastNotificationPopup.qml <span \
style="color: grey">(df63ae6)</span></li>

 <li>plasma/generic/applets/notifications/contents/ui/NotificationDelegate/NotificationDelegate.qml \
<span style="color: grey">(64d9298)</span></li>

 <li>plasma/generic/applets/notifications/contents/ui/Notifications.qml <span \
style="color: grey">(c5be0a3)</span></li>

 <li>plasma/generic/dataengines/notifications/CMakeLists.txt <span style="color: \
grey">(e8d66e0)</span></li>

 <li>plasma/generic/dataengines/notifications/notificationaction.cpp <span \
style="color: grey">(390cc16)</span></li>

 <li>plasma/generic/dataengines/notifications/notifications.operations <span \
style="color: grey">(bb5d0c6)</span></li>

 <li>plasma/generic/dataengines/notifications/notificationsengine.h <span \
style="color: grey">(4644c8e)</span></li>

 <li>plasma/generic/dataengines/notifications/notificationsengine.cpp <span \
style="color: grey">(77f5db2)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/110389/diff/" style="margin-left: \
3em;">View Diff</a></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments \
</h1>

<ul>

 <li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/05/12/notificationconfigure1.png">A \
few notifications with and without configure option</a></li>

</ul>





  </td>
 </tr>
</table>




  </div>
 </body>
</html>



_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic