Review Request: ToolBox bindings for declarative containments

[Marco Martin]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107232/#review21536
-----------------------------------------------------------


to me is starting to look good, just a copuple of remaining issues


plasma/scriptengines/javascript/declarative/toolboxproxy.h
<http://git.reviewboard.kde.org/r/107232/#comment16740>

    is this still needed?



plasma/scriptengines/javascript/plasmoid/appletinterface.h
<http://git.reviewboard.kde.org/r/107232/#comment16742>

    shouldn't this property be CONSTANT without NOTIFY?
    
    are there scenarios where the toolboxproxy pointer actually changes?



plasma/scriptengines/javascript/plasmoid/appletinterface.h
<http://git.reviewboard.kde.org/r/107232/#comment16743>

    see toolbox constant thing



plasma/scriptengines/javascript/plasmoid/appletinterface.cpp
<http://git.reviewboard.kde.org/r/107232/#comment16741>

    this should be made not necessary by containmentinterface::toolBox() ?


- Marco Martin


On Nov. 7, 2012, 12:57 a.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107232/
> -----------------------------------------------------------
> 
> (Updated Nov. 7, 2012, 12:57 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> ToolBox bindings for declarative containments
> 
> This adds the necessary bits, actions handling, showing / hiding of toolbox and a hooks for config interface and add widgets.
> 
> 
> The interesting bits:
> 
> Toolbox separate on the scene
> For declarative containments, we add a declarativewidget on top of the view which loads the "org.kde.toolbox" package. The toolbox can differ per platform, layout of toolbox and containment can not "leak" into each other.
> 
> ToolBox import
> The most important and interesting bit is the list of actions the ToolBox exposes, it's collected from corona, containment. The latter is actually problematic, since we don't get access to the actions internally provided by Containment (ContainmentPrivate::addDefaultActions). Containment::setToolBox(AbstractToolBox) being protected, we cannot register our declarative ToolBoxProxy implementation to the containment, so we have to wire up settings and addwidgets separately. Sorting of the actions is "random", and expected to be done by the QML toolbox implementation, based on objectName strings.
> 
> 
> Diffs
> -----
> 
>   plasma/scriptengines/javascript/CMakeLists.txt fe065822398e94934772bff78db3db1e393718c6 
>   plasma/scriptengines/javascript/declarative/toolboxproxy.h PRE-CREATION 
>   plasma/scriptengines/javascript/declarative/toolboxproxy.cpp PRE-CREATION 
>   plasma/scriptengines/javascript/plasmoid/appletinterface.h 6fc2d74ed602fcdee67f5d7eff9453da72375c66 
>   plasma/scriptengines/javascript/plasmoid/appletinterface.cpp ed5c8ae64b94e08f6fd5611b5caf7b33edf73d77 
>   plasma/scriptengines/javascript/plasmoid/declarativeappletscript.h 57689ba0a5188451d5364358e8a8ed4212307d21 
>   plasma/scriptengines/javascript/plasmoid/declarativeappletscript.cpp 4caceb029f968563b7a24efb9300c87c7eb1c6af 
> 
> Diff: http://git.reviewboard.kde.org/r/107232/diff/
> 
> 
> Testing
> -------
> 
> Using this with QML Desktop Containment, works as advertised above.
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20121107/f5537e1f/attachment.html>