Review Request: Add keyboard navigation to plasma applet Folder View

Fredrik Höglund fredrik at kde.org
Thu Apr 2 22:56:40 CEST 2009


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


I think in general the code looks good, but there are still numerous coding style issues, especially with the way the code is indented.


/trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.h
<http://reviewboard.kde.org/r/368/#comment486>

    I'd prefer it if this function was in AbstractItemView instead, since the code will work for the ListView class as well.



/trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.cpp
<http://reviewboard.kde.org/r/368/#comment489>

    IconView already sets the focus policy to StrongFocus at the top of the constructor (as of today).
    



/trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.cpp
<http://reviewboard.kde.org/r/368/#comment487>

    This could result in a division by zero.



/trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.cpp
<http://reviewboard.kde.org/r/368/#comment488>

    This code still doesn't take the flow into account.
    The icons can flow from left to right, right to left, top to bottom and so on, as indicated by m_flow.
    


- Fredrik


On 2009-04-02 13:21:02, Shantanu Tushar Jha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/368/
> -----------------------------------------------------------
> 
> (Updated 2009-04-02 13:21:02)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This partly addresses the above bug, adding keyboard navigation and launch using Enter key.
> Please report if the code is too complex, I've tried my best to keep it to the point.
> 
> 
> This addresses bug 187241.
>     https://bugs.kde.org/show_bug.cgi?id=187241
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.h 948236 
>   /trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.cpp 948236 
> 
> Diff: http://reviewboard.kde.org/r/368/diff
> 
> 
> Testing
> -------
> 
> Tested on latest SVN build. Navigation and launch work fine. The problem is with movement of the scrollbar with the keyboard focus, the scrollbar refuses to go to minimum value when m_scrollBar->setValue( m_scrollBar->minimum() ); is used. What am I doing wrong?
> 
> 
> Thanks,
> 
> Shantanu
> 
>



More information about the Plasma-devel mailing list