Review Request: Return 80% functionality to PMC, further evolving of state architecture

Alessandro Diaferia alediaferia at gmail.com
Sun Apr 11 19:36:48 CEST 2010


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


Still haven't tried the patch locally but i'd like the setSlidersPlayback to be changed since as you already noted that is not the way to go.


trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp
<http://reviewboard.kde.org/r/3552/#comment4410>

    i'd rename this to makeConnections() or initConnections()



trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/playbackcontrol.h
<http://reviewboard.kde.org/r/3552/#comment4411>

    Why do we need this? Shouldn't be better to have access functions to the volume slider, the seek slider and the playPause IconWidget? this way we can be sure those components will be in the widget and states and other components will be able to directly control them..


- Alessandro


On 2010-04-11 11:49:19, Christophe Olinger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3552/
> -----------------------------------------------------------
> 
> (Updated 2010-04-11 11:49:19)
> 
> 
> Review request for Plasma and Alessandro Diaferia.
> 
> 
> Summary
> -------
> 
> The state calsses now have less functions. Only one for connections and one for configuration. The first one is only called once at PMC initialization, the second one is called at each state switch. Some connections can conflict between states. Those are connected at entry() and disconnected at exit(). Thanks to Alessandr's work we can now also configure the layout from within the state class (I only had to correct some namespace stuff in the medialayout class, I hope that was correct).
> This patch also gets basic functionality back. The modes are not really useful yet. Video mode is the most complete and can be used to view everything at th moment.
> Next step: Clean this up a bit
> Think about subclassing plasma widgets to get the widget type into the widget. This is necessary to be able to tell the controller to layout into zones.
> Start work on information bar.
> 
> 
> Diffs
> -----
> 
>   trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.h 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/mediacontainment.h 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/mediacontainment.cpp 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.h 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/medialayout.cpp 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.h 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/playbackcontrol.h 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.h 1113645 
>   trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp 1113645 
> 
> Diff: http://reviewboard.kde.org/r/3552/diff
> 
> 
> Testing
> -------
> 
> Lots and lots. Seems to be quite slow, but I think that is a problem in the player. It always iterates over all the queue even if it should only show a picture.
> 
> 
> Thanks,
> 
> Christophe
> 
>



More information about the Plasma-devel mailing list