Review Request: Plasma Netbook animated Version 1

[Artur de Souza (MoRpHeUz)]

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


The commit itself seems nice. Just worried if we want to start using this all around. *Maybe* we have animated layouts in Qt *4.7* that would be more generic than this. But if we just want something specific it seems fine for me, just take a look at the details below.


trunk/KDE/kdebase/workspace/plasma/netbook/containments/common/proxylayout.cpp
<http://reviewboard.kde.org/r/2008/#comment2219>

    Why setting up the animation when setting the widget ? It would be nice to have a way to tell the layout which animation I want it to make, or give a group of animations.
    
    The way it is now all the animations are of the same kind (geometry) and using the same specs (easing curve and duration).
    
    Not sure it's a problem here as this is very specific. In our case (Animated Layouts in Qt) we had to be much more generic. Not sure what would be the desired result of this commit. Maybe Marco/Aaron can give more details about it.



trunk/KDE/kdebase/workspace/plasma/netbook/containments/newspaper/animatedlinearlayout.h
<http://reviewboard.kde.org/r/2008/#comment2220>

    extra space in the end of line



trunk/KDE/kdebase/workspace/plasma/netbook/containments/newspaper/animatedlinearlayout.cpp
<http://reviewboard.kde.org/r/2008/#comment2221>

    maybe better to implement in the proxy to delete the first (and only) item ? so here you can just delete the layout. You can also setOwnedByLayout(item) in the proxy so you just delete the layout here. it seems that the fact that the item is at position "0" is an implementation detail of the proxy that should not go "outside".


- Artur


On 2009-10-29 18:26:10, igorto wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2008/
> -----------------------------------------------------------
> 
> (Updated 2009-10-29 18:26:10)
> 
> 
> Review request for Plasma, Aaron Seigo, Marco Martin, Artur de Souza (MoRpHeUz), and Adenilson Cavalcanti.
> 
> 
> Summary
> -------
> 
> Add animated layouts to plasma-netbook.
> Animate add/remove widgets in layouts(grid and linear).
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/common/proxylayout.h PRE-CREATION 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/common/proxylayout.cpp PRE-CREATION 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/newspaper/CMakeLists.txt 1042281 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/newspaper/animatedlinearlayout.h PRE-CREATION 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/newspaper/animatedlinearlayout.cpp PRE-CREATION 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/newspaper/appletoverlay.h 1042281 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/newspaper/appletoverlay.cpp 1042281 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/newspaper/newspaper.h 1042281 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/newspaper/newspaper.cpp 1042281 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/sal/CMakeLists.txt 1042281 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/sal/animatedgridlayout.h PRE-CREATION 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/sal/animatedgridlayout.cpp PRE-CREATION 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/sal/itemcontainer.h 1042281 
>   trunk/KDE/kdebase/workspace/plasma/netbook/containments/sal/itemcontainer.cpp 1042281 
> 
> Diff: http://reviewboard.kde.org/r/2008/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> igorto
> 
>