Review Request: This is the patch to enable nextTab and previousTab keyboard shortcuts for telepathy-chat-handler.

[David Edmundson]

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


Well done on making sure to check for accidentally access tab -1, or go off the other end.

Few comments below


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

    I don't see the point of this, you have:
    tabWidget->count() for that.
    Duplicating information is normally bad.



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

    1) member variables should always be in the format
    
    m_someVariableName
    
    2) These lines don't need to be here at all. If you don't need to access the action again, we don't need to store a pointer to it.
    
    i.e you can change the code in the cpp file to be
    
    KAction *nextTabAction = new KAction(...);
    
    and you don't need to add anything into the header file. The action will still exist/work.



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

    if you have an index, just use setCurrentIndex.


- David


On May 13, 2011, 1:54 p.m., Tarun Mall wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101355/
> -----------------------------------------------------------
> 
> (Updated May 13, 2011, 1:54 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> The patch here will enable:
> Ctrl+Tab -> for next tab
> Ctrl+shift+Tab -> for previous tab
> 
> 
> Diffs
> -----
> 
>   app/chat-window.h f9c07da 
>   app/chat-window.cpp 149608d 
> 
> Diff: http://git.reviewboard.kde.org/r/101355/diff
> 
> 
> Testing
> -------
> 
> Tested with 1-2 and more number of tabs open.
> 
> 
> Thanks,
> 
> Tarun
> 
>

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