Review Request: Set ChannelChatState as Paused when the user has not typed for the past 5 seconds

David Edmundson kde at davidedmundson.co.uk
Wed Mar 21 14:12:35 UTC 2012


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


Good start. Needs some tiny tweaks.


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

    I know this isn't your code, but:
    
    rename this (now that we have 3 states)
    
    otherwise we have a variable "currentlyTyping" true, when our state is paused. Which is weird.
    
    suggest, "textBoxEmpty"



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

    Remove this, this is what you've just added.



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

    I would consider stopping the timer in this "else" state. Then you don't need that check in setChanelState that the box is empty.



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

    method needs renaming.
    
    It's not clear what it does from the name. In fact "what's a channel state" could be one of many things. (channelChatState might work)
    
    for private slots I tend to go with 
    "on Something Happened"
    
    onChatPausedTimerExpired
    
    as it's not meant to be invoked by anyone else.



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

    if you set it to singleShot you won't need to manually stop the timer.
    
    (
    when you create it call setSingleshot(true); 
    )


- David Edmundson


On March 21, 2012, 1:23 p.m., Rohan Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104354/
> -----------------------------------------------------------
> 
> (Updated March 21, 2012, 1:23 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> When the user is done typing, a QTimer is re/started and the Paused state is set after 5 seconds.
> 
> 
> This addresses bug 296439.
>     http://bugs.kde.org/show_bug.cgi?id=296439
> 
> 
> Diffs
> -----
> 
>   lib/chat-widget.h 3d72798 
>   lib/chat-widget.cpp e852972 
> 
> Diff: http://git.reviewboard.kde.org/r/104354/diff/
> 
> 
> Testing
> -------
> 
> Works for me, except for when typing a message FROM a jabber.org account, when for some reason it sets remote contact's state as 'Composing' causing it to look like the remote party is composing. Works when you compose a message in GTalk and try to send it to a jabber.org account
> 
> 
> Thanks,
> 
> Rohan Garg
> 
>

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


More information about the KDE-Telepathy mailing list