----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2286/#review3358 -----------------------------------------------------------
Ship it! Aside from the comment below, it looks good to me. /trunk/KDE/kdebase/apps/plasma/applets/folderview/folderview.cpp <http://reviewboard.kde.org/r/2286/#comment2696> If you move this call into updateIconViewState(), you only need to do it in one place. That function is called from both configAccepted() and setupIconView(). - Fredrik On 2009-11-28 09:55:39, Yuen Hoe Lim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/2286/ > ----------------------------------------------------------- > > (Updated 2009-11-28 09:55:39) > > > Review request for Plasma and Fredrik Höglund. > > > Summary > ------- > > Currently the folderview on-hover popups always previews (only) images no > matter what the preview settings of the parent folderview applet is. I saw a > 'TODO' comment in the code that says popups should inherit file preview > settings from the parent, so I assumed this is the right / desired behavior > and implemented it :) > > I think my approach is sensible, and it would also accommodate allowing the > folderview and the popup to have different file preview settings if we ever > need that in future. Still, this is my first time ever staring at folderview > code, so if I'm doing something unspeakably wrong, or if there's a better way > to do this, please let me know :) > > (I know it's feature/string freeze now, but this is rectification of faulty > behaviour - ie bugfix, so it can be accepted right? This itch happens to > bother me somewhat :) > > > Diffs > ----- > > /trunk/KDE/kdebase/apps/plasma/applets/folderview/folderview.cpp 1055216 > /trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.h 1055216 > /trunk/KDE/kdebase/apps/plasma/applets/folderview/iconview.cpp 1055216 > /trunk/KDE/kdebase/apps/plasma/applets/folderview/popupview.h 1055216 > /trunk/KDE/kdebase/apps/plasma/applets/folderview/popupview.cpp 1055216 > > Diff: http://reviewboard.kde.org/r/2286/diff > > > Testing > ------- > > Tested on trunk. Works AFAIK. > > > Thanks, > > Yuen Hoe > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
