[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request: Initial work on the flexible controller of the
From: "Alessandro Diaferia" <alediaferia () gmail ! com>
Date: 2010-03-30 21:48:59
Message-ID: 20100330214859.22081.53746 () localhost
[Download RAW message or body]
> 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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic