Review Request: supress sending info about user currently typing to contact

[Martin Klapetek]

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


The naming is a bit misleading - "showUserTyping" - does it show the other user typing? Does it show 'me typing' to the user? Where does it show?Since it is used all over the code, I'd like to have that renamed to something more clear - "sendImTypingSignal"?


app/chat-window.h
<http://git.reviewboard.kde.org/r/103385/#comment7409>

    This is a bit misleading naming



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

    You actually don't need this if branch, you can do just 
    
    bool showTyping = behaviourConfig.readEntry("showUserTyping", false);
    
    That will return (bool) 'false' if the value is not set and if it is, it returns whatever it is set to. 
    
    Also this name is rather unclear, "sendImTypingSignal"?



config/behavior-config.cpp
<http://git.reviewboard.kde.org/r/103385/#comment7413>

    Same with the config lines above, use bool directly, not strings.
    
    behaviourConfig.writeEntry("showUserTyping", m_showUserTyping);
    
    ...with improved naming.



config/behavior-config.cpp
<http://git.reviewboard.kde.org/r/103385/#comment7414>

    I know this is based on the previous code, but I think the config changes (all) should only be applied after you press the 'Apply' or 'OK' button in the UI.



config/behavior-config.ui
<http://git.reviewboard.kde.org/r/103385/#comment7412>

    This whole UI needs addressing as stated in previous comments.



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

    Use bool directly, instead of strings



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

    If you intend to keep this debug, please use kDebug()


- Martin Klapetek


On Dec. 11, 2011, 3:11 p.m., Francesco Nwokeka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103385/
> -----------------------------------------------------------
> 
> (Updated Dec. 11, 2011, 3:11 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> This patch is a remake of the general chat config and adds the option to supress sending the "user is typing" info to contact we're currently in a chat with
> 
> 
> This addresses bug 282201.
>     http://bugs.kde.org/show_bug.cgi?id=282201
> 
> 
> Diffs
> -----
> 
>   app/chat-window.h 4afa8a1 
>   app/chat-window.cpp d45cf57 
>   config/behavior-config.h 55d2fea 
>   config/behavior-config.cpp 65ddab9 
>   config/behavior-config.ui 9228db6 
>   config/kcm_telepathy_chat_behavior_config.desktop e0c45f5 
>   lib/chat-widget.h 7ea41eb 
>   lib/chat-widget.cpp e28d52e 
> 
> Diff: http://git.reviewboard.kde.org/r/103385/diff/diff
> 
> 
> Testing
> -------
> 
> spammed martin without him knowing i was typing.
> 
> 
> Screenshots
> -----------
> 
> preview
>   http://git.reviewboard.kde.org/r/103385/s/361/
> 
> 
> Thanks,
> 
> Francesco Nwokeka
> 
>

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