----------------------------------------------------------- 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 > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
