Review Request: Make the gridview behave correctly, no matter what icon size is set

[Alessandro Diaferia]

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


I'm not 100% sure we should start with such an enormous size. I feel user preferences should be kept in high consideration, at least trying to find the biggest size among those specified by the user. In addition to this you can, and should IMHO, add the configuration option for this kind of setting inside the applet configuration interface.

I'd like to know others opinions before giving a "Ship It!" :-)


trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/abstractmediaitemview.cpp
<http://reviewboard.kde.org/r/3494/#comment4368>

    please, at least do this: setIconSize(KIconLoader::SizeEnormous);



trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/viewitem.cpp
<http://reviewboard.kde.org/r/3494/#comment4365>

    Please, remove this white space



trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/viewitem.cpp
<http://reviewboard.kde.org/r/3494/#comment4366>

    I think we should have a static const int s_margin to define spacing between items if it is this what you want to achieve


- Alessandro


On 2010-04-05 15:05:32, Shantanu Tushar Jha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3494/
> -----------------------------------------------------------
> 
> (Updated 2010-04-05 15:05:32)
> 
> 
> Review request for Plasma, Marco Martin and Alessandro Diaferia.
> 
> 
> Summary
> -------
> 
> In the future, we need to support different sizes for previews. This patch adds preliminary support for this to happen. The size can be set in setIconSize in abstractmediaitemview.cpp . The desktop icon size was too small, made it 128 right now, will be changed soon.
> 
> 
> Diffs
> -----
> 
>   trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/abstractmediaitemview.cpp 1111046 
>   trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/viewitem.cpp 1111046 
> 
> Diff: http://reviewboard.kde.org/r/3494/diff
> 
> 
> Testing
> -------
> 
> Works fine with the only shortcoming that icon size can't be set at runtime, due to existing structure. This will be fixed soon, but should not block this patch from going in.
> 
> 
> Thanks,
> 
> Shantanu
> 
>