2010/5/21 Christophe Olinger <oling...@binarylooks.com> > > > > On 2010-05-20 21:59:58, Alessandro Diaferia wrote: > > > 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. > > Shall I commit after applying your comments?
Just update the diff please. I'd like to do another round of review just in case :) > > 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. :-) > Glad about it :) > > The next patch will add a few animations here and there and use the new > Plasma::Animations system. It will cover the medialyout class. > > > - Christophe > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/4050/#review5765 > ----------------------------------------------------------- > > > On 2010-05-19 10:50:29, Christophe Olinger wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > http://reviewboard.kde.org/r/4050/ > > ----------------------------------------------------------- > > > > (Updated 2010-05-19 10:50:29) > > > > > > Review request for Plasma and Alessandro Diaferia. > > > > > > Summary > > ------- > > > > Review request for plasma-mediacenter > > Now that the uberpatch is in, hacking has become easier again :-) > > 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). > > > > 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 > > > > 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. > > > > Current bugs (just so that I do not forget them and maybe one of you > wants to look at them ;-) > > 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? > > 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 :-/ > > > > 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. > > > > > > Diffs > > ----- > > > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/browsingwidget.h > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/browsingwidget.cpp > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.h > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.cpp > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/modelpackages/localpictures/localpictures.desktop > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/modelpackages/localvideos/localvideos.desktop > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.h > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.h > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.cpp > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/browser.h > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/homestate.cpp > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.h > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.h > 1128382 > > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp > 1128382 > > > > Diff: http://reviewboard.kde.org/r/4050/diff > > > > > > Testing > > ------- > > > > 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 :-) > > > > > > Thanks, > > > > Christophe > > > > > > -- Alessandro Diaferia KDE Developer KDE e.V. member
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
