Review Request: Plasma Mediacenter: Move tabbar and browsinig widgets into Browser

Alessandro Diaferia alediaferia at gmail.com
Sat May 22 10:51:26 CEST 2010


2010/5/21 Christophe Olinger <olingerc at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20100522/356918eb/attachment-0001.htm 


More information about the Plasma-devel mailing list