Review Request: Add keyboard navigation to plasma applet Folder View

[Shantanu Tushar Jha]

> On 2009-04-02 13:56:47, Fredrik Höglund wrote:
> > I think in general the code looks good, but there are still numerous coding style issues, especially with the way the code is indented.
> 
> Shantanu Tushar Jha wrote:
>     Oh, I apologise for that, but I'm unable to figure out where I've messed up with the indentation. Please point few examples where there's a problem, it'll be lots of help.
> 
> Fredrik Höglund wrote:
>     You indent the lines with 2 spaces, while the coding style specifies 4.
>     The coding style is documented in detail here: http://techbase.kde.org/Policies/Kdelibs_Coding_Style

Oops, I didn't pay attention to that in the code, sorry. Will change it now.


> On 2009-04-02 13:56:47, Fredrik Höglund wrote:
> > /trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.cpp, line 1344
> > <http://reviewboard.kde.org/r/368/diff/4/?file=4671#file4671line1344>
> >
> >     This could result in a division by zero.
> 
>  wrote:
>     The only place where m_columns is set to 0 is the constructor, but after the layout is done, there should be at least one column and one row, then how m_column can be zero? m_columns is used to hold the number of columns visible, isn't it?
> 
>  wrote:
>     The layout isn't done until items are actually inserted into the model, so until that happens m_columns will be 0.

So should I just return; the keyPressEvent function in the beginning if m_columns is zero -
if (m_columns == 0)
        return;

?


> On 2009-04-02 13:56:47, Fredrik Höglund wrote:
> > /trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.cpp, line 1372
> > <http://reviewboard.kde.org/r/368/diff/4/?file=4671#file4671line1372>
> >
> >     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.
> >
> 
>  wrote:
>     I can't figure out much what does flow mean here, only that it might mean that the model might be displayed in reverse order for some value of m_flow. What exactly is it doing, is unclear to me as I couldn't find some way to set the flow from the applet config and see the effect. Please hint on this.
>     Thanks a lot for the help :)
> 
>  wrote:
>     The flow can only be configured in the config dialog when the applet is used as a containment, but basically it controls how the icons are laid out in the view, like this: 
>     
>     LeftToRight:
>     [1][2][3]
>     [4][5][6]
>     [7][8][9]
>     
>     RightToLeft:
>     [3][2][1]
>     [6][5][4]
>     [9][8][7]
>     
>     TopToBottom:
>     [1][4][7]
>     [2][5][8]
>     [3][6][9]
>

Thanks, actually I figured this out today morning by manually using setFlow :) I'll make the code according to it.


- Shantanu


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


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
> 
>