Review Request 107833: Chat plasmoid fixes (global shortcut, visible property)

[Lasath Fernando]

> On Feb. 13, 2013, 1:11 a.m., David Edmundson wrote:
> > plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ChatWidget.qml, line 207
> > <http://git.reviewboard.kde.org/r/107833/diff/3/?file=101092#file101092line207>
> >
> >     No.

I suspect I was annoyed when I discovered that Item already had a property named 'focus'. ;-)


> On Feb. 13, 2013, 1:11 a.m., David Edmundson wrote:
> > plasmoid/org.kde.ktp-chatplasmoid/contents/ui/main.qml, line 54
> > <http://git.reviewboard.kde.org/r/107833/diff/3/?file=101094#file101094line54>
> >
> >     This means if we have two plasmoids A and B
> >     
> >     and B has an unread message and A doesn't (still not sure if that could come about) and you press the shortcut for plasmoid A,  plasmoid B pops up.

That was partially intentional. I don't know exactly how apol's mechanism for pinning contacts to these plasmoids work but if even if it were possible to have different contacts pinned to different plasmoids, I think it should make sense for the one with the unread message to pop up (Rather than making the user figure out the correct hotkey). 

I don't know if that goes against plasma's user interface guidelines though.


- Lasath


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


On Dec. 25, 2012, 5:26 a.m., Lasath Fernando wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107833/
> -----------------------------------------------------------
> 
> (Updated Dec. 25, 2012, 5:26 a.m.)
> 
> 
> Review request for Telepathy, Aleix Pol Gonzalez and David Edmundson.
> 
> 
> Description
> -------
> 
> This does a few little fixes to the chat plasmoid getting it closer to being keyboard friendly. 
> 
> 1. I redid the implementation of ConversationQueManager using a K_GLOBAL_STATIC it's now significantly shorter and more elegant IMHO.
> 2. I ditched the KAction with a hardcoded shortcut, and instead exposed ConversationQueManager to QML. So that way, the plasmoid can easily invoke dequeueNext() on plasmoid 'activate' shortcut. 
> 
> 3. I fixed the massive klugde that was the visible property of the Dialog. Now, everything is encased in separate Item, which contains one property: isOpen. It isn't bound to anything - instead, there are methods to change its value. This ceases the infinite binding loop, and allows for a clearer logic. 
> I.e. when button is clicked, dialog is closed with ALT+F4 or the model requestsPopout, those methods are called, which update 'isOpen' which in turn propagates the changes to Dialog.visible, ConversationDeleagte.checked and MessagesModel.visibleToUser properties as they are bound to it.
> 
> PS: Sorry if that wall of text didn't make any sense - It's difficult to think straight at 2AM :-)
> 
> 
> This addresses bug 296929.
>     http://bugs.kde.org/show_bug.cgi?id=296929
> 
> 
> Diffs
> -----
> 
>   plasmoid/declarative-plugin/conversation-que-manager.h e8ab426 
>   plasmoid/declarative-plugin/conversation-que-manager.cpp a7b347a 
>   plasmoid/declarative-plugin/messages-model.h 233bbfb 
>   plasmoid/declarative-plugin/qml-plugins.cpp 23a4291 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ChatWidget.qml ea68f41 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ConversationDelegate.qml 8a8d851 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/main.qml feb766b 
> 
> Diff: http://git.reviewboard.kde.org/r/107833/diff/
> 
> 
> Testing
> -------
> 
> Talked to myself. Unfortunately, I didn't talk to multiple people at once though - hopefully everything should still work.
> 
> 
> Thanks,
> 
> Lasath Fernando
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130213/06ef5e00/attachment.html>