Thanks Aaron. This sounds great. I'll try to focus on that on my next hacking sessions. I might have a lot of questions though.
Shantanu, is it ok for you if i focus on this? Also, i'll forget about a main home screen and focus on a browser showing all available dataengines grouped into the media modes. Cheers. You are all great guys and its a joy working with you. Cheers Chris On 30 Mar 2010, at 18:59, "Aaron Seigo" <ase...@kde.org> wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/3396/#review4790 > ----------------------------------------------------------- > > > First the administrivia ;) Please watch the whitespace; for core > plasma code we use http://techbase.kde.org/Policies/Kdelibs_Coding_Style > ... that means things such as 4 space indents rather than tabs and > "if (foo)" rather than "if(foo)". we are pretty strict about keeping > the code style consistent as it makes things so much tidier and > easier to work on. > > Now for the actually useful and interesting bits: looking at this > patch it occurs to me that this is a state machine. As such, I'd > really recommend revisiting this whole idea as a QStateMachine with > a set of QAbstractStates that define each mode. each of these modes > would define which UI components to show in a slot connected to > their own entered() signal, and queue for hiding these components on > their own exited() signal. > > in fact, what i'd recommend is creating a QAbstractState subclass > that provides unified access to things such as "the volume control". > this could be in the form of an enumeration (let's call it Component > for the purpose of this disucssion) which would then be used to > determine a layout. each mode would then be a subclass of this new > class, and switching between them would be a matter of changing the > state of the QStateMachine inside MediaController. > > what this class ultimately needs to do is allow a layout to be > defined, perhaps something like this: > > static const int VideoModeIcon = MediaCenterState::CustomComponent + > 1; > > BrowseVideoState::BrowseVideoState(QObject *parent) > : MediaCenterState(parent), > m_iconVideosMode(new Plasma::IconWidget(this)) > { > addToNavBar(VolumeComponent); > addToNavBar(VideoModeIcon); > ... etc ... > } > > QGraphicsWidget *BrowseVideoState::customComponent(int component) > { > if (component == VideoModeIcon) { > return m_videoModeWidget; > } > > return 0; > } > > in this example, addToNavBar would add components to an internal > list. MediaCenterState's job would then be to take the list of > current items in the layout, the list of new items in the layout and > remove the items that are no longer there while adding the ones that > are new. > > in this way only unique items will be added or removed, and this > will look rather slick when animated. it also means that adding new > states is just a matter of creating a new MediaCenterState subclass > and adding it to the StateMachine. this lends itself very nicely to > any future plugin system that may be desired. it also opens the door > for more than just "addToNavBar" (or whatever it ends up being > called) but definition of any sort of PMC UI changes that should be > enacted. a "showBrowser(bool)" could be added, for instance, to > control whether or not the file browser panel should pop out > automatically or not. a "homeApplet(const QString &pluginName)" > could be added to define what Plasma::Applet to load when entering > that state. it would also make adding a generic state for any full > screen Plasmoid loading completely trivial. > > i don't think a full QML based system is needed or even desired in > this case, since PMC should be ultimately in control of the actual > presentation and layout. the MediaCenterState classes would just > define an order of items and therefore also the transitions between > them (thanks to QStateMachine). > > thoughts? > > > /trunk/playground/base/plasma/MediaCenterComponents/applets/ > mediacontroller/controller.cpp > <http://reviewboard.kde.org/r/3396/#comment4284> > > more state machine stuff :) > > > > /trunk/playground/base/plasma/MediaCenterComponents/applets/ > mediacontroller/controller.cpp > <http://reviewboard.kde.org/r/3396/#comment4280> > > don't need to check if it's empty; just call clear() > > > > /trunk/playground/base/plasma/MediaCenterComponents/containments/ > mediacontainment/mediacontainment.cpp > <http://reviewboard.kde.org/r/3396/#comment4283> > > this is for all intents and purposes a state machine. we should > probably treat it as such. > > > > /trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/ > mediacenter.h > <http://reviewboard.kde.org/r/3396/#comment4282> > > this will be troublesome if extensibility is ever desired; this > looks like a great use case for QStateMachine, in fact. > > > > /trunk/playground/base/plasma/MediaCenterComponents/shells/ > plasmediacenter/mainwindow.cpp > <http://reviewboard.kde.org/r/3396/#comment4281> > > honestly, this should just go full screen. i don't think there is > a real use case for a windowed mode. > > so this line could probably just be: > > showFullScreen(); > > > - Aaron > > > On 2010-03-27 15:48:14, Christophe Olinger wrote: >> >> ----------------------------------------------------------- >> This is an automatically generated e-mail. To reply, visit: >> http://reviewboard.kde.org/r/3396/ >> ----------------------------------------------------------- >> >> (Updated 2010-03-27 15:48:14) >> >> >> Review request for Plasma. >> >> >> Summary >> ------- >> >> This patch extends the controller applet by having 6 different >> layout modes which are adapted to what the media center is >> currently used for, i.e. browsing pictures, playing videos, etc. It >> sends a signal to the containment with the current mode. The >> containment then relayouts the other applets and configures them >> for the current Mode. These modes are defined as enum in the libs. >> *The browser no longer has any controls. Those are now in the >> controller. >> *The controller also has a show/hide playlist button and a toggle >> autohide button for itself. >> *The different modes do not have sensible functions yet. I also >> need to work on configuring the applets for each mode, like telling >> the browser to hide, or the player to show. >> *The controller is not really beautiful. I want animations for show >> (hide icons. I want the modeswitch button in a "drawer" perhaps. >> The toggle buttons need effects. >> >> >> Diffs >> ----- >> >> /trunk/playground/base/plasma/MediaCenterComponents/applets/ >> mediabrowser/abstractmediaitemview.cpp 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/applets/ >> mediabrowser/mediabrowser.h 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/applets/ >> mediabrowser/mediabrowser.cpp 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/applets/ >> mediacontroller/controller.h 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/applets/ >> mediacontroller/controller.cpp 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/containments/ >> mediacontainment/mediacontainment.h 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/containments/ >> mediacontainment/mediacontainment.cpp 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/containments/ >> mediacontainment/medialayout.h 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/containments/ >> mediacontainment/medialayout.cpp 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/libs/ >> mediacenter/browser.cpp 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/libs/ >> mediacenter/mediacenter.h 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/libs/ >> mediacenter/playbackcontrol.h 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/libs/ >> mediacenter/playbackcontrol.cpp 1108007 >> /trunk/playground/base/plasma/MediaCenterComponents/shells/ >> plasmediacenter/mainwindow.cpp 1108007 >> >> Diff: http://reviewboard.kde.org/r/3396/diff >> >> >> Testing >> ------- >> >> I tested the controller itself. The actual effect on the other >> applets when changing modes still needs work. >> >> >> Thanks, >> >> Christophe >> >> > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
