[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request: Plasma Mediacenter: use new plasma animations,
From:       Christophe Olinger <olingerc () binarylooks ! com>
Date:       2010-05-27 6:31:44
Message-ID: AANLkTimNbiaTBL2aU8XAdFtQZPtG2DlpKnCPvpDrOTla () mail ! gmail ! com
[Download RAW message or body]

Wow, thanks. My 4th accepted patch. Yay! I'll try to commit this
evening or latest tomorrow.

Concerning the bugs, I can collect them on our techbase page for now.

Cheers,

Christophe




On Wed, May 26, 2010 at 11:15 PM, Alessandro Diaferia
<alediaferia@gmail.com> wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4152/#review5876
> -----------------------------------------------------------
> 
> Ship it!
> 
> 
> This is a good patch: the move to new animator API is a really good thing and i \
> wanted this to be done as soon as possible. I still didn't have a look at the video \
> widget size bug as i still don't have videos on my new installation. Anyway i'd say \
> go for this patch and i'll try to fix the bug in the next patch i'm working on. 
> Also i'd like you to take note of such bugs you encounter as we don't have a \
> component on bko still (i probably should ask for one). 
> Here follows little issues i noticed.
> 
> 
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp
>  <http://reviewboard.kde.org/r/4152/#comment5502>
> 
> please add a little comment and specify we're returning to browsing mode as soon as \
> slide-show is finished 
> 
> 
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp
>  <http://reviewboard.kde.org/r/4152/#comment5503>
> 
> please prefer member accessors to array-type ones: m_pictureMedias.first();
> 
> 
> 
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp
> <http://reviewboard.kde.org/r/4152/#comment5504>
> 
> you can put this as a static const qreal MARGINFACTOR = 0.2; at the beginning of \
> the code 
> 
> 
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp
> <http://reviewboard.kde.org/r/4152/#comment5505>
> 
> this line doesn't do anything? you probably want to do m_controlAutohide = !set;
> 
> 
> - Alessandro
> 
> 
> On 2010-05-26 08:19:03, Christophe Olinger wrote:
> > 
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > http://reviewboard.kde.org/r/4152/
> > -----------------------------------------------------------
> > 
> > (Updated 2010-05-26 08:19:03)
> > 
> > 
> > Review request for Plasma and Alessandro Diaferia.
> > 
> > 
> > Summary
> > -------
> > 
> > This patch covers the following:
> > 
> > Features:
> > - Use new plasma animation classes
> > - In floating mode, fade player between pictures (avoids painting artefacts)
> > - Return to browser after showing last picture of a slideshow
> > - Start at first picture again after slideshow has been played and is started \
> >                 again
> > - Background states are handled onExit of each state by checking the \
> >                 currentPlayback state
> > - Renamed layoutZone enum members
> > 
> > Bugfixes:
> > - Fixed bug where video player would be fully expanded horizontally, but not \
> >                 vertically
> > - Slideshow now works correctly
> > - Bottom Panel should be correctly placed now at startup
> > 
> > Evil bugs:
> > - Video player always remains at 1024 x 600 :-/. Alessandro, could you have a \
> >                 look please?
> > - Qt 4.7 beta 1 does not solve the blue skin bug when playing videos
> > - Layout errors of bottom bar (will fix that in the next days)
> > 
> > 
> > Diffs
> > -----
> > 
> > trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.cpp \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.h \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/homestate.cpp \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenter.h \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.h \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/player.h \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/private/mediahandler.cpp \
> > 1130736 trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp \
> > 1130736 
> > Diff: http://reviewboard.kde.org/r/4152/diff
> > 
> > 
> > Testing
> > -------
> > 
> > 
> > Thanks,
> > 
> > Christophe
> > 
> > 
> 
> 
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


[prev in list] [next in list] [prev in thread] [next in thread]