Review Request: Add keyboard navigation to plasma applet Folder View

Fredrik Höglund fredrik at kde.org
Thu Apr 9 18:38:09 CEST 2009


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

Ship it!


Aside from a couple of minor nitpicks below I think the patch looks good now,
so please commit it after taking care of those :)


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

    Don't forget to remove this before committing ;)



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

    Too much indentation here.



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

    Please add a comment here saying that the fall-through is intentional. 



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

    This closing brace should be at the same level as the else statement on the next line.



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

    Too much indentation in this block.


- Fredrik


On 2009-04-08 21:33:17, Shantanu Tushar Jha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/368/
> -----------------------------------------------------------
> 
> (Updated 2009-04-08 21:33:17)
> 
> 
> 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/abstractitemview.h 951365 
>   /trunk/KDE/kdebase/apps/plasma/applets/folderview/abstractitemview.cpp 951365 
>   /trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.h 951365 
>   /trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.cpp 951365 
> 
> 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