> On 2010-03-30 16:59:21, Aaron Seigo wrote: > > 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?
Wow! This really seems the right-way-of-doing-things of what I suggested above to Christophe about handling modes. Too bad i'm not experienced in QState* but i'd really like seeing it implemented this way. This way the layouting stuff becomes really clean and easily extensible. Please Christophe, go for it. :-) Thanks Aaron for this precious hint! - Alessandro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3396/#review4790 ----------------------------------------------------------- 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
