-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4050/
-----------------------------------------------------------

(Updated 2010-05-23 16:42:06.013995)


Review request for Plasma and Alessandro Diaferia.


Changes
-------

Applied all Alessandro's suggestions and corrections


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 (updated)
-----

  
trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/browsingwidget.h
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/browsingwidget.cpp
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.h
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.cpp
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.h
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.h
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/applets/mediainfobar/mediainfobar.cpp
 1129752 
  trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/browser.h 
1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/homestate.cpp
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.h
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.h
 1129752 
  
trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp
 1129752 

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

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to