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

Christophe Olinger olingerc at binarylooks.com
Fri May 21 09:29:03 CEST 2010



> 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?

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. :-)

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
> 
>



More information about the Plasma-devel mailing list