Review Request: a way for a custom corona to forbid some standard context menu entries
Marco Martin
notmart at gmail.com
Sun Jan 24 18:44:04 CET 2010
> On 2010-01-14 18:18:26, Chani Armitage wrote:
> > :/ this feels wrong.
> >
> > for the screensaver, I just created a separate mouse plugin.
> > I suppose the netbook shares actions like "log out" and "lock screen" though, which makes this a bit trickier...
> > we don't really want to duplicate all that code, but nor do we want a special hack for the contextmenu plugin...
> > really, all those plugins were written with just the desktop in mind, they should be in desktop/ not generic/.
> >
> > *thinks*
> >
> > abusing kauthorized wouldn't be much better, I guess?
>
> Marco Martin wrote:
> yeah, this was just as an experiment to see what options there could be. in the netbook a right mouse button menu makes sense, just that single action shouldn't be here...
>
> so, i see at the moment 2 possibilities:
> -every shell will have its own menu implementation, like DesktopMenu, NetbookMenu ScreensaverMenu etc. quite code duplication but would avoid not too pretty new api
> -there is a single menu plugin, but is the list of qactions that depends from te shell, so a qlist of ContamentContextActions(ugh what an ugly name) will be provided by the implementations, so the ndividua coronas... ugly api less duplication.
>
> hmmm, fscinating problem :)
>
> Aaron Seigo wrote:
> i don't think it's worth adding to the libplasma API for what really amounts to one plugin and a subset of its features.
>
> for the particular case mentioned, it suggest that the action should come from the corona. and i agree with that :) in that case the plugin could check if the corona has a "add panel" action and if so include it, otherwise don't.
>
> at the worst, simply duplicating this one plugin would be a better answer imho than the API proposal, but distributing the actions around sounds even better to me. (that particular plugin's code could also likely end up being a lot simpler than it is right now; there are a number of collection classes that are being kept in sync with each other, for instance)
>
>
yes, this is a muuuuch cleaner way.
done as in revision 1079633
- Marco
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2580/#review3697
-----------------------------------------------------------
On 2010-01-14 11:03:23, Marco Martin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2580/
> -----------------------------------------------------------
>
> (Updated 2010-01-14 11:03:23)
>
>
> Review request for Plasma and Chani Armitage.
>
>
> Summary
> -------
>
> This approach doesn't look that right, but is the only way i could think of:
> in the netbook shell there really shouldn't be the "add panel" context menu entry since it isn't supported (what happens right now is the panel containment being created and no views assigned to it.
> we could also decide that yeah, indeed the netbook should support multiple panels too (was thinking about that for unrelated reasons) but the problem would propose itself again when we do another shell without panels but that still make sense to have context menus (like the screensaver)
> i tried to do a generic mechanism: all actions will be enabled by default and the corona keeps a blacklist of them (corona or containment? some actions make sense to be enabled or disabled only globally, like add panel, others could be containment dependent?)
> setContaimentActionEnabled() adds the action to the blacklist
>
> this is just a stub, all actions should check their availability in the future
>
>
> Diffs
> -----
>
> /trunk/KDE/kdebase/workspace/plasma/generic/containmentactions/contextmenu/menu.cpp 1070354
> /trunk/KDE/kdebase/workspace/plasma/netbook/shell/netcorona.cpp 1070354
> /trunk/KDE/kdelibs/plasma/containment.h 1074119
> /trunk/KDE/kdelibs/plasma/containment.cpp 1074119
> /trunk/KDE/kdelibs/plasma/corona.h 1074119
> /trunk/KDE/kdelibs/plasma/corona.cpp 1074119
>
> Diff: http://reviewboard.kde.org/r/2580/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Marco
>
>
More information about the Plasma-devel
mailing list