Review Request: Re-establish chat after going offline and back online PART1

[David Edmundson]

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



app/chat-window.cpp
<http://git.reviewboard.kde.org/r/101250/#comment2520>

    I think by comparing targetHandleType we can work for room chats too. 
    I don't think we need to restrict to 1-1 chats.
    
    The bug report also says to not check for dupes in which the new channel targetHandleType is HandleTypeNone and always make a new window for them.



app/chat-window.cpp
<http://git.reviewboard.kde.org/r/101250/#comment2521>

    auxChatTab->textChannel()->targetId() /might/ not work.
    
    at this point, auxChatTab->textChannel() returns an invalidated channel.
    
    Does this still contain a working targetId and targetHandleType, or does it now report nonsense? It may, it may not. I don't know.
    Needs either testing asking oggis or looking in Tp-Qt4 code.



app/chat-window.cpp
<http://git.reviewboard.kde.org/r/101250/#comment2519>

    qDebug -> kDebug 
    
    AND AVOID TYPING ALL IN CAPS :-P



lib/chat-widget.cpp
<http://git.reviewboard.kde.org/r/101250/#comment2517>

    Leaky code!
    
    We can now get duplicates of the contactListModel.
    
    if we switch channel we would call "new" again but we havent' deleted the old one (it gets deleted when 'this' the ChatWidget is deleted.
    
    Maybe: make contactList an instance variable (d->contactList) create it on startup and add a "setChannel" method to it.



lib/chat-widget.cpp
<http://git.reviewboard.kde.org/r/101250/#comment2518>

    You don't want to call this again! It will wipe the entire view.



lib/chat-widget.cpp
<http://git.reviewboard.kde.org/r/101250/#comment2522>

    Tbh, I think we want a method that only does "connect channel signals" and sorts out the ChannelContactsModel.
    
    
    Given we want a public method called setChannel(Tp::Channel) anyway we could put this code in here, and call that on setup.


- David


On April 28, 2011, 3:32 p.m., Francesco Nwokeka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101250/
> -----------------------------------------------------------
> 
> (Updated April 28, 2011, 3:32 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> Part1 of resolving bug 270725. I made two setup methods that have the task to setup the connections of the chatWidget's Tp::textChannelPtr. This was done to facilitate my work later on when substituting the old textptr with a new one when the user comes back online and he/she had an ongoing chat.
> Plus you can see that I changed the mathing criteria. I no longer match the textChannelPtr, but the targetId and it's handler.
> 
> 
> Diffs
> -----
> 
>   app/chat-window.h 8a55655 
>   app/chat-window.cpp ca1778a 
>   lib/chat-widget.h f357aea 
>   lib/chat-widget.cpp b483436 
> 
> Diff: http://git.reviewboard.kde.org/r/101250/diff
> 
> 
> Testing
> -------
> 
> Opened chats with contacts and the behaviour was like before
> 
> 
> Thanks,
> 
> Francesco
> 
>

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