----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3396/#review4918 -----------------------------------------------------------
Ok, things are getting a little huge. Let's see together how we can come out of this. I like the idea of the state machine and how it could help us correctly managing the workflow of the mediacenter. Unfortunately i feel something in the current implementation lacks of robustness (does this word exist?). As Aaron pointed out, we need to understend whether we are aproaching this the right way. The mistake with the MediaLayout in this patch well shows that we should probably re-design something. The need for states is due to the fact that we want PMC to know the current media in order to differently behave and interact with the user. This way we could give the best set of actions for each kind of media chosen by the user. The first clear issue was the lack for a way of changing the UI accordingly to the chosen media. I'd go with questions here in order to better summarize what we need in the PMC: Q1. When do we need the UI to react to media type changes? R1. The UI should show different possible actions to the user both while browsing to a personal collection of e.g. videos and when actually reproducing a video. Q2. How the PMC knows which kind of media are we browsing through/reproducing ? R2. The PMC::Browser API should be updated in order to expose this kind of information. In addition to this the PMC::Player should be used in order to retrieve this kind of information while reproducing media. Q3. Are media types mutually exclusive? R3. This (unfortunately?) cannot always be true. The user might want to start his favourite playlist and then run a slideshow of his summer photos. Probably the PMC::Player should inform whether music is currently playing in background. I'd like to know from you whether you think music is the only media type that can be played in background while reproducing something else in foreground. This is an important topic IMHO. Q4. Which UI controls does the PMC need? R4. The PMC needs of course playback controls for the current media. In addition to this the browser should allow the user to navigate through his collection of medias and eventually get back to the welcome page. The welcome page should IMO be inside the browser. Answering the above questions i feel that what we already have is enough in order to make the UI adapting to what is happening inside PMC. The MediaController can be updated in order to support background and foreground media playback control. When MediaBrowser plugins will be ready the welcome screen will be there for free showing the user the available kind of medias. The MediaPlayer should be updated in order to both play music and slideshows that shouldn't be too hard to do. I really like the MediaToolbox by Cristophe and i think that its browsing part could directly go inside the MediaBrowser. Of course i might be wrong about all this stuff and so, please, tell me your opinions. I hope this is constructive enough to reach a valid design together. Regards. trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h <http://reviewboard.kde.org/r/3396/#comment4381> A method of a library cannot rely on a particular implementation. You are asking for a MediaLayout* to be passed in as a parameter but that class is something defined within the MediaContainment, that is within the application level. We, here, are at a library level instead.. This is indicates that probably we have do re-design this. trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h <http://reviewboard.kde.org/r/3396/#comment4382> Same as above. - Alessandro On 2010-04-07 20:59:11, Christophe Olinger wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/3396/ > ----------------------------------------------------------- > > (Updated 2010-04-07 20:59:11) > > > Review request for Plasma. > > > Summary > ------- > > This is the first try to get state machines working. I will of course not > forget the previous comments. It builds and runs and state switching is > possible. No playbackbuttons or other useful stuff yet, but is easy to get > back. > > What I did: > Created a mediacenterstate class which contains info about shared (so called > Main) subcomponents. subcomponents are little widgets like buttons and > sliders which will appear in the control bar. I sublcassed QState for this. > Then I created a picturestate and videostate class, subclasses of the > follwoing which are loaded on state switch. The containment does the state > switch, the state classes do all the rest: connections, conficurations and > hading out subcomponents to the controlbar. > The controlbar now has a class to add widgets to itself. > > > Diffs > ----- > > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/abstractmediaitemview.h > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/abstractmediaitemview.cpp > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.h > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.cpp > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.h > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.h > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/mediacontainment.h > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/mediacontainment.cpp > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/medialayout.h > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/medialayout.cpp > 1112197 > trunk/playground/base/plasma/MediaCenterComponents/libs/CMakeLists.txt > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/CMakeLists.txt > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/browser.h > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/browser.cpp > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenter.h > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h > PRE-CREATION > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp > PRE-CREATION > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.h > PRE-CREATION > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp > PRE-CREATION > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h > PRE-CREATION > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp > PRE-CREATION > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/playbackcontrol.h > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/playbackcontrol.cpp > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/player.h > 1112197 > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.h > PRE-CREATION > > trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp > PRE-CREATION > > trunk/playground/base/plasma/MediaCenterComponents/shells/plasmediacenter/mainwindow.cpp > 1112197 > > Diff: http://reviewboard.kde.org/r/3396/diff > > > Testing > ------- > > State switchting works. > > > Thanks, > > Christophe > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
