Review Request: Plasma Netbook animated Version 1

Marco Martin notmart at gmail.com
Thu Oct 29 22:01:33 CET 2009



> On 2009-10-29 18:45:38, Artur de Souza (MoRpHeUz) wrote:
> > 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.

i'm also for keeping it internal, even if the temptation to use it elsewere is big, hopefully we will be able to replace it for qt 4.7 time


> On 2009-10-29 18:45:38, Artur de Souza (MoRpHeUz) wrote:
> > trunk/KDE/kdebase/workspace/plasma/netbook/containments/common/proxylayout.cpp, line 26
> > <http://reviewboard.kde.org/r/2008/diff/1/?file=13486#file13486line26>
> >
> >     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.

i do think that the duration should be the same for everybody, not controllable :)
the setwidget could be somewhat inferred from the parentLayoutItem in the ctor


> On 2009-10-29 18:45:38, Artur de Souza (MoRpHeUz) wrote:
> > trunk/KDE/kdebase/workspace/plasma/netbook/containments/newspaper/animatedlinearlayout.cpp, line 34
> > <http://reviewboard.kde.org/r/2008/diff/1/?file=13489#file13489line34>
> >
> >     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".

agree


- Marco


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


On 2009-10-29 18:45:58, igorto wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2008/
> -----------------------------------------------------------
> 
> (Updated 2009-10-29 18:45:58)
> 
> 
> 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
> 
>



More information about the Plasma-devel mailing list