Review Request 119946: Wrap the desktop alternatives UI in a dialog

[Marco Martin]

> On Aug. 26, 2014, 5:49 p.m., David Edmundson wrote:
> > desktoppackage/contents/explorer/AppletAlternatives.qml, line 131
> > <https://git.reviewboard.kde.org/r/119946/diff/1/?file=307712#file307712line131>
> >
> >     Why do we need this?
> >     (and if we do need it, it should go on the Switch button too)
> 
> David Edmundson wrote:
>     Edit: I see why we're using it now. Should have read the other patch first.
>     It does need adding to the other button, but this whole thing isn't ideal. It means we have to rely on the Alternatives.qml having magic undocumented properties.
> 
> Marco Martin wrote:
>     the imperative show() and close() don't seem to be defined anywhere, it should just do visible = true; and visible = false; ?
> 
> David Edmundson wrote:
>     Personally I'd rather use an explicit dialog.destroy() rather than having a property that we monitor in code further away. 
>     
>     the code in the C++ then becomes
>     connect(qmlObj->rootObject(), SIGNAL(destroyed(QObject*)),
>             qmlObj, SLOT(deleteLater()));
>             
>     and we can drop the d->alternativesObjects, and the alternativesVisibilityChanged.
> 
> Marco Martin wrote:
>     ah, show()/hide() are from QWindow, nvm (if visible is not redeclared they shouldn't be required tough)

"Personally I'd rather use an explicit dialog.destroy() rather than having a property that we monitor in code further away."

the problem is that you can't use destroy() from javascript on object that have been declared, you can only for objects that have been dynamically created from js as well.
If you try to destroy from javascript the root object, destroy is only a noop and only gives a warning, so as far I know that one can be killed only from c++ unfortunately


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119946/#review65293
-----------------------------------------------------------


On Aug. 26, 2014, 5:37 p.m., Aaron J. Seigo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119946/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2014, 5:37 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> Following up on the changes to plasmashell, the dialog needs to used from the QML in the shell package.
> 
> 
> Diffs
> -----
> 
>   desktoppackage/contents/explorer/AppletAlternatives.qml de99b1ca370c819687230c1bcd54d2839b08dab9 
> 
> Diff: https://git.reviewboard.kde.org/r/119946/diff/
> 
> 
> Testing
> -------
> 
> Used Plasma Desktop with this shell package.
> 
> 
> Thanks,
> 
> Aaron J. Seigo
> 
>

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