<br><br><div class="gmail_quote">2010/5/21 Christophe Olinger <span dir="ltr"><<a href="mailto:olingerc@binarylooks.com">olingerc@binarylooks.com</a>></span><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im"><br>
<br>
> On 2010-05-20 21:59:58, Alessandro Diaferia wrote:<br>
> > Just little annotations here and there. I like the way we're doing this :-) Don't worry about the bugs you mention, i'll likely investigate them as soon as the patch goes in.<br>
<br>
</div>Shall I commit after applying your comments? </blockquote><div>Just update the diff please. I'd like to do another round of review just in case :) <br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<br>
Thanks a lot for the bitwise OR explanation. Your 2 paragraphs explained it better than reading a whole wiki article about it! I sure wish I could follow one of your c++/qt courses. I am happy that you all convinced me to just get going instead of working alone on a useless project and trying to solve problems alone. Learning by doing TM. :-)<br>
</blockquote><div>Glad about it :)<br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<br>
The next patch will add a few animations here and there and use the new Plasma::Animations system. It will cover the medialyout class.<br>
<font color="#888888"><br>
<br>
- Christophe<br>
</font><div class="im"><br>
<br>
-----------------------------------------------------------<br>
This is an automatically generated e-mail. To reply, visit:<br>
<a href="http://reviewboard.kde.org/r/4050/#review5765" target="_blank">http://reviewboard.kde.org/r/4050/#review5765</a><br>
-----------------------------------------------------------<br>
<br>
<br>
</div><div><div></div><div class="h5">On 2010-05-19 10:50:29, Christophe Olinger wrote:<br>
><br>
> -----------------------------------------------------------<br>
> This is an automatically generated e-mail. To reply, visit:<br>
> <a href="http://reviewboard.kde.org/r/4050/" target="_blank">http://reviewboard.kde.org/r/4050/</a><br>
> -----------------------------------------------------------<br>
><br>
> (Updated 2010-05-19 10:50:29)<br>
><br>
><br>
> Review request for Plasma and Alessandro Diaferia.<br>
><br>
><br>
> Summary<br>
> -------<br>
><br>
> Review request for plasma-mediacenter<br>
> Now that the uberpatch is in, hacking has become easier again :-)<br>
> I decided to put the browsing control button (goPrevious) back into the browser, along with the tabbar. The tabbar can be used to change viewmodes later, e.g. by Artist, by Album, by Tag,...I have added an API that can be used to add pages to the tabbar. The states handle the adding of pages on entry. (Later we can add the connection between the tabbar and the modelpackage/dataengines of the browser).<br>
><br>
> Question: do we need states for playing and browsing? (also for the floating mode?) At the moment there is one state for each, but with differences between playing and browsing<br>
><br>
> Question: I am thinking of playing around with QML as soon as kubuntu has a qt4.7 package. Do you think I should? This would mean a lot of refactoring. I wanted to just use it for the two panels at the beginning. Maybe also the playlist.<br>
><br>
> Current bugs (just so that I do not forget them and maybe one of you wants to look at them ;-)<br>
> Video Mode: When entering the videomode and clicking on play (with a video in the playlist) the videoplayer covers only half of the screen. When the video is stopped and again play is pressed, the videoplayer size is correct. Also, when I ply via the playlist (clicking on the item in the playlist) the videoplayer size is always correct?<br>
> Picture mode: I do not understand how to correctly position the widgets in the bottom bar. They are at the wrong position on state entry but as soon as I change a picture, they jump to the correct positions. Switching to the floating mode makes the widgets be a wrong positions also :-/<br>
><br>
> We need a way to tell the browser to change the modelpackage. I guess via the plugin factory, but that is over my head ATM.<br>
><br>
><br>
> Diffs<br>
> -----<br>
><br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/browsingwidget.h 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/browsingwidget.cpp 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.h 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.cpp 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/modelpackages/localpictures/localpictures.desktop 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/modelpackages/localvideos/localvideos.desktop 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.h 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.h 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.cpp 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/browser.h 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/homestate.cpp 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.h 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.h 1128382<br>
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp 1128382<br>
><br>
> Diff: <a href="http://reviewboard.kde.org/r/4050/diff" target="_blank">http://reviewboard.kde.org/r/4050/diff</a><br>
><br>
><br>
> Testing<br>
> -------<br>
><br>
> All states were thoroughly tested and the above bugs were found (among others) There are lots of TODOs and FIXMEs in the code still. It's a WIP :-)<br>
><br>
><br>
> Thanks,<br>
><br>
> Christophe<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>