<br><br><div class="gmail_quote">2010/5/27 Christophe Olinger <span dir="ltr"><<a href="mailto:olingerc@binarylooks.com">olingerc@binarylooks.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Wow, thanks. My 4th accepted patch. Yay! I'll try to commit this<br>
evening or latest tomorrow.<br>
<br>
Concerning the bugs, I can collect them on our techbase page for now.<br></blockquote><div> </div><div>Yeah, great idea! So we give them more visibility among eventual other contributors.</div><div>I'd likely try to figure out which of them can represent junior jobs for new contributors :-) </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
Cheers,<br></blockquote><div><br></div><div>Ciao :-) </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<font color="#888888"><br>
Christophe<br>
</font><div><div></div><div class="h5"><br>
<br>
<br>
<br>
On Wed, May 26, 2010 at 11:15 PM, Alessandro Diaferia<br>
<<a href="mailto:alediaferia@gmail.com">alediaferia@gmail.com</a>> wrote:<br>
><br>
> -----------------------------------------------------------<br>
> This is an automatically generated e-mail. To reply, visit:<br>
> <a href="http://reviewboard.kde.org/r/4152/#review5876" target="_blank">http://reviewboard.kde.org/r/4152/#review5876</a><br>
> -----------------------------------------------------------<br>
><br>
> Ship it!<br>
><br>
><br>
> 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.<br>
><br>
> 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).<br>
><br>
> Here follows little issues i noticed.<br>
><br>
><br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp<br>
> <<a href="http://reviewboard.kde.org/r/4152/#comment5502" target="_blank">http://reviewboard.kde.org/r/4152/#comment5502</a>><br>
><br>
> please add a little comment and specify we're returning to browsing mode as soon as slide-show is finished<br>
><br>
><br>
><br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp<br>
> <<a href="http://reviewboard.kde.org/r/4152/#comment5503" target="_blank">http://reviewboard.kde.org/r/4152/#comment5503</a>><br>
><br>
> please prefer member accessors to array-type ones: m_pictureMedias.first();<br>
><br>
><br>
><br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp<br>
> <<a href="http://reviewboard.kde.org/r/4152/#comment5504" target="_blank">http://reviewboard.kde.org/r/4152/#comment5504</a>><br>
><br>
> you can put this as a static const qreal MARGINFACTOR = 0.2; at the beginning of the code<br>
><br>
><br>
><br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp<br>
> <<a href="http://reviewboard.kde.org/r/4152/#comment5505" target="_blank">http://reviewboard.kde.org/r/4152/#comment5505</a>><br>
><br>
> this line doesn't do anything? you probably want to do m_controlAutohide = !set;<br>
><br>
><br>
> - Alessandro<br>
><br>
><br>
> On 2010-05-26 08:19:03, Christophe Olinger wrote:<br>
>><br>
>> -----------------------------------------------------------<br>
>> This is an automatically generated e-mail. To reply, visit:<br>
>> <a href="http://reviewboard.kde.org/r/4152/" target="_blank">http://reviewboard.kde.org/r/4152/</a><br>
>> -----------------------------------------------------------<br>
>><br>
>> (Updated 2010-05-26 08:19:03)<br>
>><br>
>><br>
>> Review request for Plasma and Alessandro Diaferia.<br>
>><br>
>><br>
>> Summary<br>
>> -------<br>
>><br>
>> This patch covers the following:<br>
>><br>
>> Features:<br>
>> - Use new plasma animation classes<br>
>> - In floating mode, fade player between pictures (avoids painting artefacts)<br>
>> - Return to browser after showing last picture of a slideshow<br>
>> - Start at first picture again after slideshow has been played and is started again<br>
>> - Background states are handled onExit of each state by checking the currentPlayback state<br>
>> - Renamed layoutZone enum members<br>
>><br>
>> Bugfixes:<br>
>> - Fixed bug where video player would be fully expanded horizontally, but not vertically<br>
>> - Slideshow now works correctly<br>
>> - Bottom Panel should be correctly placed now at startup<br>
>><br>
>> Evil bugs:<br>
>> - Video player always remains at 1024 x 600 :-/. Alessandro, could you have a look please?<br>
>> - Qt 4.7 beta 1 does not solve the blue skin bug when playing videos<br>
>> - Layout errors of bottom bar (will fix that in the next days)<br>
>><br>
>><br>
>> Diffs<br>
>> -----<br>
>><br>
>> trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.cpp 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.h 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/homestate.cpp 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenter.h 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.h 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/player.h 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/private/mediahandler.cpp 1130736<br>
>> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp 1130736<br>
>><br>
>> Diff: <a href="http://reviewboard.kde.org/r/4152/diff" target="_blank">http://reviewboard.kde.org/r/4152/diff</a><br>
>><br>
>><br>
>> Testing<br>
>> -------<br>
>><br>
>><br>
>> Thanks,<br>
>><br>
>> Christophe<br>
>><br>
>><br>
><br>
><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>Alessandro Diaferia<br>KDE Developer<br>KDE e.V. member<br><br>