Review Request: Initial implementation of telepathy-approver

[George Kiagiadakis]

> On 2010-12-11 13:14:32, Dario Freddi wrote:
> > src/textchannelapprover.cpp, lines 99-113
> > <http://git.reviewboard.kde.org/r/100199/diff/1/?file=4967#file4967line99>
> >
> >     I'm kinda concerned about this approach: could you please elaborate on why you need such complex logic for handling the notifier item? Ideally, with the notifier item approach, you would always have a notifier item object, and you require attention/show/hide depending on the needs of the application.

Well, I don't want to always have an icon there, it is useless if there are no channels to approve. What I do want though, is a means to approve even if the popups are hidden or disabled. So, my intention is to show a flashing icon when there is one or more channels to approve and clicking on the icon will approve them all (for now; it could have a context menu with options, but that's another topic). The approach of making it a global static pointer is because I don't want to have one icon for every channel, but one for all of them. Also, I am making this specific to text channels because I imagine having a second icon for calls, a third for file transfers, etc... So, when I have an incoming message and an incoming call, I see two icons.

Other suggestions are welcome of course.


- George


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


On 2010-12-09 20:27:51, George Kiagiadakis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100199/
> -----------------------------------------------------------
> 
> (Updated 2010-12-09 20:27:51)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> This is the initial implementation of the approver. It can approve p2p text chats only, for now. It is build as a kded module. When a new message arrives, it shows a knotify popup, it plays a sound and shows a KStatusNotifierItem flashing in the tray.
> 
> gitweb url:
> http://gitweb.kde.org/clones/telepathy-approver/gkiagia/telepathy-approver.git/shortlog/refs/heads/implementation
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt PRE-CREATION 
>   src/CMakeLists.txt PRE-CREATION 
>   src/approverdaemon.h PRE-CREATION 
>   src/approverdaemon.cpp PRE-CREATION 
>   src/channelapprover.h PRE-CREATION 
>   src/channelapprover.cpp PRE-CREATION 
>   src/dispatchoperation.h PRE-CREATION 
>   src/dispatchoperation.cpp PRE-CREATION 
>   src/handlewithcaller.h PRE-CREATION 
>   src/handlewithcaller.cpp PRE-CREATION 
>   src/telepathy_kde_approver.desktop PRE-CREATION 
>   src/telepathy_kde_approver.notifyrc PRE-CREATION 
>   src/textchannelapprover.h PRE-CREATION 
>   src/textchannelapprover.cpp PRE-CREATION 
>   src/tpkdeapprovermodule.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/100199/diff
> 
> 
> Testing
> -------
> 
> Tested with empathy as the chat handler. Currently the "respond" button on the popup doesn't seem to work, but I blame knotify's crappiness for that. Clicking on the flashing icon works perfectly, though.
> 
> 
> Thanks,
> 
> George
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20101211/44467234/attachment-0001.htm