Review Request: Add keyboard navigation to plasma applet Folder View

[Fredrik Höglund]

> On 2009-03-20 14:07:32, Fredrik Höglund wrote:
> > /trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.cpp, line 1208
> > <http://reviewboard.kde.org/r/368/diff/2/?file=3392#file3392line1208>
> >
> >     A problem with the way this function is implemented is that it assumes that the view is always sorted and that the icons always flow from left to right.
> >     
> >     When the user has rearranged the icons (m_layoutBroken is true), you have to assume that the icons are no longer arranged in a grid and that the visual order no longer matches the order in the model.
> >     
> >     When this is the case, and the user has pressed the up key for example, you have to iterate over all the icons and find the one that is closest to the current icon while still being above it.
> >
> 
>  wrote:
>     "you have to iterate over all the icons". I'm working on this by iterating all icons and finding the nearest one to the current selection according to the key pressed, but the code is getting really complex in terms of calculations. I was wondering if there is any other way of doing this? If anyone has an idea, please let me know. Till then I'm working on it.
>
> 
>  wrote:
>     I would do something like this (the example is for the up key only):
>     
>     QModelIndex nextIndex = QModelIndex();
>     QPoint currentPos = visualRect(currentIndex).center();
>     int lastDistance = 0;
>     
>     for (int i = 0; i < m_validRows; i++) {
>          const QModelIndex index = m_model->index(i, 0);
>          const QPoint pos = visualRect(index).center();
>          if (pos.y() < currentPos.y()) {
>              int distance = (pos - currentPos).manhattanLength();
>              if (distance < lastDistance || !currentIndex.isValid()) {
>                  nextIndex = index;
>                  lastDistance = distance;
>              }
>          }
>     }
>     
>     If nextIndex is valid when you get here, it's the index you should move to.
>     If it isn't valid there are no icons above the current icon.
>     
>     Thanks for working on this feature :)
>
> 
>  wrote:
>     Ok, thanks for the help, that was less complex then mine ;) Its almost done, just a few minor issues remaining.
>     But, I'm unable to understand why you're using `!currentIndex.isValid()` in if (distance < lastDistance || !currentIndex.isValid()) ?
>     Why do we need to validate the currentIndex ?

Oh right, that should actually be nextIndex, not currentIndex.
I guess should read what I wrote more carefully before pressing "publish" :)


- Fredrik


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


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