Review Request: Folderview popups should inherit file preview settings

Fredrik Höglund fredrik at kde.org
Wed Dec 2 00:30:08 CET 2009


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



More information about the Plasma-devel mailing list